From 31fa410875e54c14caf9aa171238066b19df9d99 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Mon, 17 Aug 2026 23:37:24 +0200 Subject: [PATCH] fix(style): drive the hover sheen from held state, drop the leaking start map The sheen kept its own Dictionary of start timestamps and only cleared an entry in the un-hover branch. A row that disappeared while the pointer was on it left its entry behind until the plugin reloaded, which is exactly what happens to temp tabs under the LRU limit. It now takes the held intensity from HoverState and draws on the rising edge only. The alpha falls off as the value climbs, so the sweep has faded out by the time the surface underneath is fully in. On the way out it simply does not run, which is what stops it from travelling backwards -- the old SheenStarts.Remove prevented that by resetting, and dropping the map without this guard would have reintroduced it. The sidebar call site built "sidebar.tab.{guid}" per row per frame, two allocations each. Master spec 5.3 asks for constant keys, and 7.5 for a stable allocation count. It now uses ImGui.GetID("row"u8), which is allocation-free and seeded from the window's ID stack, so the same literal stays distinct per window and per PushID'd tab. HoverSheenAllocStep pinned the old dictionary contract and is replaced by HoverStateFootprintStep, which pins the same property against the registry: repeated queries add no entries, and an element that stops being queried leaves the map instead of leaking. --- HellionChat/Plugin.cs | 2 +- HellionChat/SelfTests/HoverSheenAllocStep.cs | 61 -------------- .../SelfTests/HoverStateFootprintStep.cs | 79 +++++++++++++++++++ HellionChat/Ui/Components/Sidebar.cs | 7 +- .../Ui/StyleEngine/DrawListExtensions.cs | 40 +++------- 5 files changed, 96 insertions(+), 93 deletions(-) delete mode 100644 HellionChat/SelfTests/HoverSheenAllocStep.cs create mode 100644 HellionChat/SelfTests/HoverStateFootprintStep.cs diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index 97929c6..713b7f0 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -429,7 +429,7 @@ public sealed class Plugin : IAsyncDalamudPlugin new SelfTests.TypingIpcStateStep(this), new SelfTests.ConfigMigrationV23Step(this), new SelfTests.ChannelPopoutBindStep(this), - new SelfTests.HoverSheenAllocStep(this), + new SelfTests.HoverStateFootprintStep(), new SelfTests.HonorificHeaderRenderStep(this), new SelfTests.AboutIntegrationsStatusStep(this), new SelfTests.PerformanceBaselineStep(this), diff --git a/HellionChat/SelfTests/HoverSheenAllocStep.cs b/HellionChat/SelfTests/HoverSheenAllocStep.cs deleted file mode 100644 index b7a557e..0000000 --- a/HellionChat/SelfTests/HoverSheenAllocStep.cs +++ /dev/null @@ -1,61 +0,0 @@ -using Dalamud.Bindings.ImGui; -using Dalamud.Plugin.SelfTest; -using HellionChat.Themes; -using HellionChat.Ui.StyleEngine; - -namespace HellionChat.SelfTests; - -// Master-spec scope note: the hover-sheen key dictionary must not grow -// frame-by-frame on a constant-key call site. This probe drives 100 -// hovered frames against three constant keys and asserts the dictionary -// only holds those three keys at the end — re-hover does not duplicate -// entries, and the un-hover branch clears the stale start timestamp. -internal sealed class HoverSheenAllocStep : ISelfTestStep -{ - private readonly Plugin plugin; - - public HoverSheenAllocStep(Plugin plugin) - { - this.plugin = plugin; - } - - public string Name => "Hellion Chat - HoverSheen dictionary footprint"; - - public SelfTestStepResult RunStep() - { - // Probe runs outside a regular draw frame, so the sheen path - // would normally not have a window draw-list. We pull the - // foreground draw-list directly — it accepts AddRectFilled - // even without an active window scope. - var dl = ImGui.GetForegroundDrawList(); - var theme = plugin.ThemeRegistry.Active; - var resolver = new TokenResolver(); - var accent = resolver.Resolve(Token.AccentPrimary, theme.Colors); - var min = new System.Numerics.Vector2(0, 0); - var max = new System.Numerics.Vector2(10, 10); - - string[] keys = ["selftest.row.a", "selftest.row.b", "selftest.row.c"]; - for (var frame = 0; frame < 100; frame++) - foreach (var key in keys) - dl.DrawHoverSheen(min, max, accent, key, hovered: true); - - // Hovered loop must have registered exactly the three constant keys - // (no per-frame growth/duplication) — assert via observable state, - // not an unconditional pass (K7 false-green fix). - foreach (var key in keys) - if (!DrawListExtensions.IsSheenTracked(key)) - return SelfTestStepResult.Fail; - - // Un-hover sweep must drop every entry through the cleanup branch. - foreach (var key in keys) - dl.DrawHoverSheen(min, max, accent, key, hovered: false); - - foreach (var key in keys) - if (DrawListExtensions.IsSheenTracked(key)) - return SelfTestStepResult.Fail; - - return SelfTestStepResult.Pass; - } - - public void CleanUp() { } -} diff --git a/HellionChat/SelfTests/HoverStateFootprintStep.cs b/HellionChat/SelfTests/HoverStateFootprintStep.cs new file mode 100644 index 0000000..e726155 --- /dev/null +++ b/HellionChat/SelfTests/HoverStateFootprintStep.cs @@ -0,0 +1,79 @@ +using Dalamud.Bindings.ImGui; +using Dalamud.Plugin.SelfTest; +using HellionChat.Ui.StyleEngine; + +namespace HellionChat.SelfTests; + +// Master-spec §7.5 scope note: the hover registry must not grow frame by frame. +// Successor to HoverSheenAllocStep, which pinned the same contract against the +// old sheen start-timestamp dictionary. +// +// Two properties matter. Repeated queries for the same element must not add +// entries, and once an element stops being queried its entry must actually +// leave the map -- the old dictionary only cleared on an explicit un-hover +// call, so a row that vanished while hovered leaked until the plugin reloaded. +internal sealed class HoverStateFootprintStep : ISelfTestStep +{ + public string Name => "Hellion Chat - HoverState registry footprint"; + + public SelfTestStepResult RunStep() + { + // Runs outside a normal draw frame, so the clock is stepped by hand + // rather than by HoverState.BeginFrame. + const float Frame = 1f / 60f; + + var saved = Plugin.Config.ReduceMotion; + try + { + // The short-circuit would skip the map entirely and make this probe + // vacuous. + Plugin.Config.ReduceMotion = false; + HoverState.Reset(); + + uint[] ids = + [ + ImGui.GetID("selftest.row.a"), + ImGui.GetID("selftest.row.b"), + ImGui.GetID("selftest.row.c"), + ]; + + for (var frame = 0; frame < 100; frame++) + { + foreach (var id in ids) + HoverState.Query(id, hovered: true); + HoverState.AdvanceForTest(Frame); + } + + if (HoverState.TrackedCount != ids.Length) + { + var msg = $"Registry holds {HoverState.TrackedCount}, expected {ids.Length}"; + ImGui.Text(msg); + SelfTestReport.Append(Name, "FAIL", new[] { msg }); + return SelfTestStepResult.Fail; + } + + // Stop querying entirely, the way a removed row would. Fade-out runs + // at 8/s, so 1s of frames is comfortably past zero. + for (var frame = 0; frame < 60; frame++) + HoverState.AdvanceForTest(Frame); + + if (HoverState.TrackedCount != 0) + { + var msg = $"Registry leaked {HoverState.TrackedCount} entries after fade-out"; + ImGui.Text(msg); + SelfTestReport.Append(Name, "FAIL", new[] { msg }); + return SelfTestStepResult.Fail; + } + + SelfTestReport.Append(Name, "PASS", new[] { "registry empty after fade-out" }); + return SelfTestStepResult.Pass; + } + finally + { + Plugin.Config.ReduceMotion = saved; + HoverState.Reset(); + } + } + + public void CleanUp() { } +} diff --git a/HellionChat/Ui/Components/Sidebar.cs b/HellionChat/Ui/Components/Sidebar.cs index d8b05f4..fcbd0fe 100644 --- a/HellionChat/Ui/Components/Sidebar.cs +++ b/HellionChat/Ui/Components/Sidebar.cs @@ -251,11 +251,16 @@ internal sealed class Sidebar TabLifecycleHelpers.OnTabActivated(tab, previous); } + // GetID is seeded from the window's ID stack, so the same "row" literal + // stays distinct per window and per PushID'd tab. The old interpolated + // key allocated two strings per row per frame. + var hoverId = ImGui.GetID("row"u8); + var hoverAmount = StyleEngine.HoverState.Query(hoverId, rowHovered); dl.DrawHoverSheen( origin, origin + new Vector2(avail, RowHeight), accentRgba, - $"sidebar.tab.{tab.Identifier}", + hoverAmount, rowHovered ); diff --git a/HellionChat/Ui/StyleEngine/DrawListExtensions.cs b/HellionChat/Ui/StyleEngine/DrawListExtensions.cs index dc47394..c0824a4 100644 --- a/HellionChat/Ui/StyleEngine/DrawListExtensions.cs +++ b/HellionChat/Ui/StyleEngine/DrawListExtensions.cs @@ -9,13 +9,9 @@ namespace HellionChat.Ui.StyleEngine; // Custom-drawing primitives for the v2.x style layer. Callers feed RGBA // uints (typically resolved via TokenResolver) and these methods convert to -// ABGR before delegating to ImDrawList. Hover-sheen state lives in a small -// static dictionary keyed by constant strings — keep keys constant and -// scope to static UI elements so the per-key footprint stays bounded. +// ABGR before delegating to ImDrawList. internal static class DrawListExtensions { - private const float SheenDurationSeconds = 0.65f; - // A1 accent-tint (Variante A): how far the white sweep is pulled toward // the element's accent hue. Kept low so the sheen reads as a tinted // highlight, not a saturated accent flash (effect level "subtle"). @@ -24,41 +20,25 @@ internal static class DrawListExtensions // Peak sheen alpha (low so the highlight stays subtle); DrawHoverSheen applies the falloff. private const byte SheenPeakAlpha = 0x40; - private static readonly Dictionary SheenStarts = new(); - - // Test-observability hook for HoverSheenAllocStep: lets the self-test - // assert the hover/un-hover dictionary contract via real state instead - // of an unconditional pass (K7 false-green fix). Not a runtime path. - internal static bool IsSheenTracked(string elementId) => SheenStarts.ContainsKey(elementId); - + // Rides the held hover intensity instead of its own timer, so it can no + // longer leak an entry when an element disappears while hovered. Drawn on + // the rising edge only: the alpha falls off as the value climbs, so the + // sweep has faded out by the time the surface underneath is fully in. + // On the way out the surface fades and the sweep simply does not run, + // which is what keeps it from travelling backwards. public static void DrawHoverSheen( this ImDrawListPtr dl, Vector2 min, Vector2 max, uint accentRgba, - string elementId, + float intensity, bool hovered ) { - if (!hovered) - { - // Reset so re-hover restarts the sweep instead of catching the - // tail end of a stale animation. - SheenStarts.Remove(elementId); - return; - } - - if (!SheenStarts.TryGetValue(elementId, out var started)) - { - started = DateTime.UtcNow; - SheenStarts[elementId] = started; - } - - var elapsed = (DateTime.UtcNow - started).TotalSeconds; - if (elapsed > SheenDurationSeconds) + if (!hovered || intensity <= 0f || intensity >= 1f) return; - var t = (float)(elapsed / SheenDurationSeconds); + var t = intensity; var alpha = (byte)Math.Round(SheenPeakAlpha * (1f - t)); // Tint the sweep toward the accent hue, then stamp the falloff alpha. // accentRgba is RGBA; convert to ABGR FIRST or the draw-list swaps R/B.