From 93f4fbba72d0497d4a738cee510cac2670e882a1 Mon Sep 17 00:00:00 2001 From: Jon Kazama Date: Tue, 16 Jun 2026 20:09:45 +0200 Subject: [PATCH] perf(card): variable-height clipper + layout-fingerprint cache invalidation + clip-plan self-test --- HellionChat/Plugin.cs | 1 + HellionChat/SelfTests/CardClipPlanStep.cs | 99 ++++++++++++++ HellionChat/Ui/Components/MessageList.cs | 156 ++++++++++++++++++---- 3 files changed, 233 insertions(+), 23 deletions(-) create mode 100644 HellionChat/SelfTests/CardClipPlanStep.cs diff --git a/HellionChat/Plugin.cs b/HellionChat/Plugin.cs index b761245..22deb6e 100755 --- a/HellionChat/Plugin.cs +++ b/HellionChat/Plugin.cs @@ -426,6 +426,7 @@ public sealed class Plugin : IAsyncDalamudPlugin new SelfTests.SidebarUnreadDotStep(this), new SelfTests.UnreadDecisionStep(), new SelfTests.CurrentTabGuidedStep(this), + new SelfTests.CardClipPlanStep(this), ]); // Re-surface the wizard for existing users when a major UX diff --git a/HellionChat/SelfTests/CardClipPlanStep.cs b/HellionChat/SelfTests/CardClipPlanStep.cs new file mode 100644 index 0000000..9442e6e --- /dev/null +++ b/HellionChat/SelfTests/CardClipPlanStep.cs @@ -0,0 +1,99 @@ +using System.Collections.Generic; +using Dalamud.Bindings.ImGui; +using Dalamud.Plugin.SelfTest; + +namespace HellionChat.SelfTests; + +// B2: behavioural check that the card path feeds CardClipPlanner AND that a +// layout change clears the height cache — not a non-null check. Pure plan math +// is pinned headless by CardClipPlanTests; this drives the live accessors. +internal sealed class CardClipPlanStep : ISelfTestStep +{ + private readonly Plugin plugin; + + public CardClipPlanStep(Plugin plugin) => this.plugin = plugin; + + public string Name => "Hellion Chat - Card clip plan + cache invalidation"; + + public SelfTestStepResult RunStep() + { + var messages = plugin.MainWindow.GetMessageListForSelfTest(); + if (messages is null) + { + ImGui.Text("MessageList null"); + SelfTestReport.Append(Name, "FAIL", new[] { "MessageList null" }); + return SelfTestStepResult.Fail; + } + + // 5 rows of 20, viewport 50, scroll 45 -> skip rows 0,1 (lead 40), see 2..4. + IReadOnlyList heights = [20f, 20f, 20f, 20f, 20f]; + var plan = messages.PlanCardClipForSelfTest(heights, scrollY: 45f, viewportHeight: 50f); + if ( + plan.FirstVisible != 2 + || plan.LastVisible != 4 + || plan.LeadDummyHeight is < 39.9f or > 40.1f + ) + { + var msg = + $"Plan wrong: first={plan.FirstVisible} last={plan.LastVisible} lead={plan.LeadDummyHeight}"; + ImGui.Text(msg); + SelfTestReport.Append(Name, "FAIL", new[] { msg }); + return SelfTestStepResult.Fail; + } + + var tab = plugin.MainWindow.ActiveTab; + if (tab is null) + { + ImGui.Text("No active tab"); + SelfTestReport.Append(Name, "FAIL", new[] { "No active tab" }); + return SelfTestStepResult.Fail; + } + + // Flip a height-affecting mode -> fingerprint changes -> cache must drop. + // Config restored in finally (live-singleton discipline). + var savedForm = Plugin.Config.NameFormMode; + int remaining; + try + { + messages.RunHeightCacheInvalidationForSelfTest(tab, 400f); + using (var snap = tab.Messages.GetReadOnly(3)) + { + if (snap.Count > 0) + snap[0].Height[tab.Identifier] = 42f; + } + + Plugin.Config.NameFormMode = + savedForm == NameFormMode.Full ? NameFormMode.Initials : NameFormMode.Full; + remaining = messages.RunHeightCacheInvalidationForSelfTest(tab, 400f); + } + finally + { + Plugin.Config.NameFormMode = savedForm; + messages.RunHeightCacheInvalidationForSelfTest(tab, 400f); + } + + if (remaining != 0) + { + var msg = $"Cache not cleared after layout change: {remaining} left"; + ImGui.Text(msg); + SelfTestReport.Append(Name, "FAIL", new[] { msg }); + return SelfTestStepResult.Fail; + } + + SelfTestReport.Append( + Name, + "PASS", + new[] + { + $"Plan range [{plan.FirstVisible}..{plan.LastVisible}], lead={plan.LeadDummyHeight}", + "Height cache cleared after layout-fingerprint change", + } + ); + ImGui.Text( + $"PASS — plan [{plan.FirstVisible}..{plan.LastVisible}], cache cleared on layout change." + ); + return SelfTestStepResult.Pass; + } + + public void CleanUp() { } +} diff --git a/HellionChat/Ui/Components/MessageList.cs b/HellionChat/Ui/Components/MessageList.cs index a9c43ea..80e55c7 100644 --- a/HellionChat/Ui/Components/MessageList.cs +++ b/HellionChat/Ui/Components/MessageList.cs @@ -1,4 +1,5 @@ using System.Globalization; +using System.Linq; using System.Numerics; using Dalamud.Bindings.ImGui; using Dalamud.Interface.Utility; @@ -7,11 +8,10 @@ using HellionChat.Util; namespace HellionChat.Ui.Components; -// Virtualised message list. Compact mode reuses ImGuiListClipper because -// rows have a constant line height; card mode falls back to a linear -// render with a per-message height cache and an IsItemVisible skip path -// so off-screen rows place a Dummy of the cached height rather than -// running the full render again. +// Virtualised message list. Compact mode uses ImGuiListClipper (constant row +// height); card mode rows wrap to arbitrary heights, so it runs its own +// prefix-sum clipper (CardClipPlanner) over a per-message height cache, dropped +// whenever the layout fingerprint (font/density/name-mode/width) changes. internal sealed class MessageList { private const float CompactRowHeight = 18f; @@ -27,6 +27,18 @@ internal sealed class MessageList private bool _scrolledUp; private bool _scrollToBottomRequested; + // B2: the height cache is only valid while these inputs are unchanged. + // FontManager's own fingerprint covers font sizes only, not density / the two + // name-display modes / width — a stale height would misplace the clipper dummies. + private ( + float Global, + float Symbols, + bool Compact, + int NameForm, + int WorldSuffix, + float Width + ) _lastLayoutFingerprint; + // §6.2: setter-injection breaks the PayloadHandler → MainWindow → MessageList → PayloadHandler 3-cycle. // Wired by PayloadHandlerInitHostedService.StartAsync after both singletons exist. internal void AttachPayloadHandler(PayloadHandler handler) @@ -54,6 +66,55 @@ internal sealed class MessageList // post-snap reset can be asserted; without it only the OR branch is testable. internal void RequestScrollToBottomForSelfTest() => _scrollToBottomRequested = true; + // SelfTest hook (B2): runs the real planner against a caller fixture so the + // step asserts the plan without a live scroll child (GetScrollY is garbage headless). + internal CardClipPlan PlanCardClipForSelfTest( + IReadOnlyList heights, + float scrollY, + float viewportHeight + ) => CardClipPlanner.Plan(heights, scrollY, viewportHeight); + + // SelfTest hook (B2): drives the live invalidation, returns the tab's remaining + // cached-height count so the step can assert the drop. + internal int RunHeightCacheInvalidationForSelfTest(Tab tab, float contentWidth) + { + InvalidateHeightCacheIfLayoutChanged(tab, contentWidth); + using var messages = tab.Messages.GetReadOnly(3); + return messages.Count(m => m.Height.ContainsKey(tab.Identifier)); + } + + // Width is passed in (ContentRegionAvail is only valid inside the draw child); + // enum modes widened to int so the tuple stays comparable. + private (float, float, bool, int, int, float) BuildLayoutFingerprint(float contentWidth) + { + var (global, symbols) = _fonts.EffectiveFontFingerprint(); + return ( + global, + symbols, + Plugin.Config.UseCompactDensity, + (int)Plugin.Config.NameFormMode, + (int)Plugin.Config.WorldSuffixMode, + contentWidth + ); + } + + // Drop the tab's cached heights when the layout fingerprint changed — one + // tuple compare per frame, a clear only on a real settings/resize change. + private void InvalidateHeightCacheIfLayoutChanged(Tab tab, float contentWidth) + { + var fingerprint = BuildLayoutFingerprint(contentWidth); + if (fingerprint.Equals(_lastLayoutFingerprint)) + return; + + _lastLayoutFingerprint = fingerprint; + using var messages = tab.Messages.GetReadOnly(3); + foreach (var msg in messages) + { + msg.Height.Remove(tab.Identifier); + msg.IsVisible.Remove(tab.Identifier); + } + } + public void Draw(Tab tab) { if (!_fonts.FontsReady) @@ -65,9 +126,15 @@ internal sealed class MessageList // No own ImRaii.Child here — MainWindow already wraps the message // area in one. Nesting would give the window two stacked scrolls // and a runaway content-height computation. - using var messages = tab.Messages.GetReadOnly(3); var compact = Plugin.Config.UseCompactDensity; + // B2: drop stale cached heights before the snapshot draw (card path only — + // compact rows are constant height). Width read here while it is valid. + if (!compact) + InvalidateHeightCacheIfLayoutChanged(tab, ImGui.GetContentRegionAvail().X); + + using var messages = tab.Messages.GetReadOnly(3); + // Track whether the user was pinned to the bottom before this frame // so newly arriving rows do not yank them up. The check runs against // the parent child's scroll state, which is the one MainWindow owns. @@ -192,29 +259,72 @@ internal sealed class MessageList private void DrawCard(Tab tab, IReadOnlyList messages) { var tabId = tab.Identifier; - for (var i = 0; i < messages.Count; i++) + var count = messages.Count; + if (count == 0) + return; + + // A row with no cached height yet can't be planned (first frame / post- + // invalidation), so draw everything once to fill the cache, plan next frame. + var heights = new float[count]; + var allCached = true; + for (var i = 0; i < count; i++) + { + if (messages[i].Height.TryGetValue(tabId, out var cached) && cached is float h) + heights[i] = h; + else + allCached = false; + } + + if (!allCached) + { + DrawCardLinearAndMeasure(tabId, messages); + return; + } + + var plan = CardClipPlanner.Plan(heights, ImGui.GetScrollY(), ImGui.GetWindowSize().Y); + + // Cached heights already include one trailing ItemSpacing.y; a dummy adds its + // own, which would shove the first visible row down one spacing (jitter). + // Subtract one spacing per dummy so offset AND total content height stay exact. + var spacingY = ImGui.GetStyle().ItemSpacing.Y; + float CompensatedDummy(float planned) => Math.Max(0f, planned - spacingY); + + if (plan.FirstVisible < 0) + { + // Scrolled into a gap: one full-height dummy keeps the scrollbar honest. + var gap = CompensatedDummy(plan.LeadDummyHeight + plan.EndDummyHeight); + ImGui.Dummy(new Vector2(10f, gap)); + return; + } + + if (plan.LeadDummyHeight > 0f) + ImGui.Dummy(new Vector2(10f, CompensatedDummy(plan.LeadDummyHeight))); + + for (var i = plan.FirstVisible; i <= plan.LastVisible; i++) { var msg = messages[i]; - - // Cached row: place a Dummy of the known height and skip the - // full render path if the row is off-screen. Mirrors the - // v1.5.6 Card-Mode pattern in ChatLogWindow.DrawMessages. - msg.Height.TryGetValue(tabId, out var cachedHeight); - if (cachedHeight is float h) - { - var beforeDummy = ImGui.GetCursorPos(); - ImGui.Dummy(new Vector2(10f, h)); - var visible = ImGui.IsItemVisible(); - msg.IsVisible[tabId] = visible; - if (!visible) - continue; - ImGui.SetCursorPos(beforeDummy); - } - var before = ImGui.GetCursorPosY(); DrawCardRow(msg); var after = ImGui.GetCursorPosY(); msg.Height[tabId] = after - before; + msg.IsVisible[tabId] = true; + } + + if (plan.EndDummyHeight > 0f) + ImGui.Dummy(new Vector2(10f, CompensatedDummy(plan.EndDummyHeight))); + } + + // First-frame / post-invalidation fallback: draw + measure every row into the + // cache so the next frame can take the planned path. + private void DrawCardLinearAndMeasure(Guid tabId, IReadOnlyList messages) + { + foreach (var msg in messages) + { + var before = ImGui.GetCursorPosY(); + DrawCardRow(msg); + var after = ImGui.GetCursorPosY(); + msg.Height[tabId] = after - before; + msg.IsVisible[tabId] = ImGui.IsItemVisible(); } }