fix(messages): apply density switches immediately, freeze the cache while waiting
Review of block A found two regressions the settle gate introduced. Density and the two name modes are switches, not sliders. They land on a new value in one frame and stay there, but the gate made them wait 200ms like a drag. Meanwhile the row painter switched instantly, so for about twelve frames the planner ran compact rows against card heights: wrong slice, wrong offset, and the visible rows got overwritten with the new density while everything else kept the old one. Before the gate this was correct, so it was a regression, not a pre-existing bug. The fingerprint now separates a discrete axis that applies at once from the continuous one that waits. Second, the cache did not actually stand still during the wait. Measurements were written back unconditionally, so a resize drag mixed heights from many different widths and the lead dummy drifted for the whole drag. The gate now exposes IsPending and the planned path skips the write while it is set. The linear path still measures, because without a filled cache there is nothing to plan against. Three smaller items from the same review: An oscillation slower than the settle window used to bypass the wait entirely, because the pending clock was never reset on the way back to the applied value. A value that changes on every frame could hold the gate shut forever while the applied fingerprint stayed wrong. MaxWaitMs is the deadline, checked before the still-moving branch so a permanently moving value actually reaches it. The row painter was passed as `compact ? DrawCompactRow : DrawCardRow`. Both are instance methods, so the method group captures `this` and Roslyn does not cache it: 64 bytes per frame per window. Bound once in the constructor now. The per-frame height array is also reused rather than reallocated, which at the default MaxLinesToRender was 10 KB per frame on what A2 had just made the default path.
This commit is contained in:
@@ -2,9 +2,10 @@ using System.Collections.Generic;
|
||||
|
||||
namespace HellionChat.Ui.Components;
|
||||
|
||||
// B2 (PERF-B2): variable-height clip plan for card mode. ImGuiListClipper needs
|
||||
// a constant row height, so card mode (rows wrap to arbitrary heights) computes
|
||||
// its own plan from the cached per-row heights: a lead dummy for the rows above
|
||||
// B2 (PERF-B2): variable-height clip plan. ImGuiListClipper needs a constant
|
||||
// row height, and since v1.10.0/A2 neither density has one (compact rows wrap
|
||||
// too), so both compute a plan from the cached per-row heights: a lead dummy
|
||||
// for the rows above
|
||||
// the viewport, the [first..last] index range that overlaps the viewport, and
|
||||
// an end dummy for the rows below. This is the prefix-sum analogue of
|
||||
// OtterGui's GetNecessarySkips, but for non-uniform heights — kept Dalamud-free
|
||||
@@ -19,7 +20,9 @@ internal readonly record struct CardClipPlan(
|
||||
|
||||
internal static class CardClipPlanner
|
||||
{
|
||||
// heights[i] is the cached, spacing-inclusive height of row i in draw order.
|
||||
// heights[i] is the cached height of row i in draw order. It carries no
|
||||
// trailing ItemSpacing: every row ends inside DrawChunks, where spacing is
|
||||
// pushed to zero, and ImGui writes the advance at item submission time.
|
||||
// A row overlaps the viewport iff rowTop < windowBottom && rowBottom >
|
||||
// windowTop. FirstVisible/LastVisible are -1 when nothing overlaps (empty
|
||||
// list); callers then submit a single end dummy of the full content height
|
||||
|
||||
@@ -31,6 +31,17 @@ internal sealed class MessageList
|
||||
// itself applied, so tab B kept measuring against the previous width.
|
||||
private readonly Dictionary<Guid, LayoutFingerprintGate> _fingerprintGates = [];
|
||||
|
||||
// Bound once. A method group off an instance method captures `this` and is
|
||||
// not cached by Roslyn, so `compact ? DrawCompactRow : DrawCardRow` would
|
||||
// allocate a delegate on every frame of every window.
|
||||
private readonly Action<Message> _drawCompactRow;
|
||||
private readonly Action<Message> _drawCardRow;
|
||||
|
||||
// Reused across frames: at the default MaxLinesToRender of 2500 a fresh
|
||||
// array per frame is 10 KB of garbage, and A2 put the default density on
|
||||
// this path.
|
||||
private float[] _heightScratch = [];
|
||||
|
||||
// §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)
|
||||
@@ -40,6 +51,8 @@ internal sealed class MessageList
|
||||
|
||||
public MessageList(FontManager fonts, ChunkRenderer chunkRenderer)
|
||||
{
|
||||
_drawCompactRow = DrawCompactRow;
|
||||
_drawCardRow = DrawCardRow;
|
||||
_fonts = fonts;
|
||||
_chunkRenderer = chunkRenderer;
|
||||
}
|
||||
@@ -145,7 +158,11 @@ internal sealed class MessageList
|
||||
// the parent child's scroll state, which is the one MainWindow owns.
|
||||
var pinnedToBottom = ImGui.GetScrollY() >= ImGui.GetScrollMaxY() - 1f;
|
||||
|
||||
DrawRows(tab, messages, compact ? DrawCompactRow : DrawCardRow);
|
||||
// While the gate waits out a continuous change, measurements must not go
|
||||
// back into the cache: the rows outside the viewport still carry the old
|
||||
// geometry, and mixing the two makes the lead dummy drift every frame.
|
||||
var frozen = _fingerprintGates[tab.Identifier].IsPending;
|
||||
DrawRows(tab, messages, compact ? _drawCompactRow : _drawCardRow, frozen);
|
||||
|
||||
// B3-5: scroll values are frame-constant inside the child, so this
|
||||
// reflects the current frame's state wherever it runs; kept after the
|
||||
@@ -238,7 +255,12 @@ internal sealed class MessageList
|
||||
|
||||
// Shared by both densities: compact rows wrap too, so neither has a constant
|
||||
// height the ImGuiListClipper could work with. drawRow is the only difference.
|
||||
private void DrawRows(Tab tab, IReadOnlyList<Message> messages, Action<Message> drawRow)
|
||||
private void DrawRows(
|
||||
Tab tab,
|
||||
IReadOnlyList<Message> messages,
|
||||
Action<Message> drawRow,
|
||||
bool frozen
|
||||
)
|
||||
{
|
||||
var tabId = tab.Identifier;
|
||||
var count = messages.Count;
|
||||
@@ -247,7 +269,9 @@ internal sealed class MessageList
|
||||
|
||||
// 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];
|
||||
if (_heightScratch.Length < count)
|
||||
_heightScratch = new float[Math.Max(count, 256)];
|
||||
var heights = _heightScratch;
|
||||
var allCached = true;
|
||||
for (var i = 0; i < count; i++)
|
||||
{
|
||||
@@ -259,11 +283,17 @@ internal sealed class MessageList
|
||||
|
||||
if (!allCached)
|
||||
{
|
||||
// Always measures, frozen or not: without a filled cache there is
|
||||
// nothing to plan against at all.
|
||||
DrawLinearAndMeasure(tabId, messages, drawRow);
|
||||
return;
|
||||
}
|
||||
|
||||
var plan = CardClipPlanner.Plan(heights, ImGui.GetScrollY(), ImGui.GetWindowSize().Y);
|
||||
var plan = CardClipPlanner.Plan(
|
||||
new ArraySegment<float>(heights, 0, count),
|
||||
ImGui.GetScrollY(),
|
||||
ImGui.GetWindowSize().Y
|
||||
);
|
||||
|
||||
// A dummy is submitted outside any style push, so it appends its own
|
||||
// trailing ItemSpacing.y. Subtracting one spacing per dummy makes the dummy
|
||||
@@ -290,6 +320,9 @@ internal sealed class MessageList
|
||||
var msg = messages[i];
|
||||
var before = ImGui.GetCursorPosY();
|
||||
drawRow(msg);
|
||||
if (frozen)
|
||||
continue;
|
||||
|
||||
var after = ImGui.GetCursorPosY();
|
||||
msg.Height[tabId] = after - before;
|
||||
msg.IsVisible[tabId] = true;
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
namespace HellionChat.Util;
|
||||
|
||||
// Layout inputs that make a tab's cached row heights stale. Kept as a plain
|
||||
// value type so the build suite can pin the settle logic without an ImGui frame.
|
||||
// value type so the build suite can pin the gate without an ImGui frame.
|
||||
internal readonly record struct LayoutFingerprint(
|
||||
float FontGlobal,
|
||||
float FontSymbols,
|
||||
@@ -10,35 +10,77 @@ internal readonly record struct LayoutFingerprint(
|
||||
int WorldSuffix,
|
||||
float Width,
|
||||
float UiScale
|
||||
);
|
||||
)
|
||||
{
|
||||
// Toggles: they land on a new value in one frame and stay there. Waiting on
|
||||
// them would leave the planner running against the previous density's
|
||||
// heights while the rows are already painted the new way.
|
||||
internal (bool, int, int) Discrete => (Compact, NameForm, WorldSuffix);
|
||||
}
|
||||
|
||||
// Dragging a window edge or the Dalamud UI-scale slider moves the fingerprint on
|
||||
// every single frame. Acting on each one drops the height cache, which sends the
|
||||
// whole tab through the linear measure path (up to Config.MaxLinesToRender rows,
|
||||
// default 2500). Waiting for the value to settle turns that into one rebuild per
|
||||
// drag instead of one per frame.
|
||||
// Dragging a window edge or the Dalamud UI-scale slider moves the continuous
|
||||
// half of the fingerprint on every frame. Acting on each one drops the height
|
||||
// cache and sends the whole tab through the linear measure path (up to
|
||||
// Config.MaxLinesToRender rows). Waiting for those to settle turns a drag into
|
||||
// one rebuild. Discrete changes bypass the wait entirely.
|
||||
internal sealed class LayoutFingerprintGate
|
||||
{
|
||||
internal const long SettleMs = 200;
|
||||
|
||||
// A value that never stops moving would otherwise hold the gate shut
|
||||
// forever while the applied fingerprint stays wrong.
|
||||
internal const long MaxWaitMs = 1000;
|
||||
|
||||
private LayoutFingerprint? _applied;
|
||||
private LayoutFingerprint _pending;
|
||||
private long _pendingSinceMs;
|
||||
private long _divergedSinceMs;
|
||||
|
||||
// True while a continuous change is being waited out. The caller must not
|
||||
// write fresh measurements into the cache during this window, or the cache
|
||||
// becomes a mix of the old and the in-flight geometry.
|
||||
internal bool IsPending { get; private set; }
|
||||
|
||||
internal bool ShouldInvalidate(LayoutFingerprint current, long nowMs)
|
||||
{
|
||||
// First sight of this tab: nothing is cached yet, so there is nothing to
|
||||
// drop and nothing to wait for.
|
||||
if (_applied is null)
|
||||
{
|
||||
_applied = current;
|
||||
_pending = current;
|
||||
_pendingSinceMs = nowMs;
|
||||
Settle(current, nowMs);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (current.Equals(_applied.Value))
|
||||
var applied = _applied.Value;
|
||||
if (current.Equals(applied))
|
||||
{
|
||||
Settle(current, nowMs);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Density and the two name modes are switches, not sliders: apply now.
|
||||
if (!current.Discrete.Equals(applied.Discrete))
|
||||
{
|
||||
Settle(current, nowMs);
|
||||
return true;
|
||||
}
|
||||
|
||||
// First frame of a divergence starts both clocks: the settle window
|
||||
// restarts on every further move, the deadline does not.
|
||||
if (!IsPending)
|
||||
{
|
||||
_divergedSinceMs = nowMs;
|
||||
_pending = current;
|
||||
_pendingSinceMs = nowMs;
|
||||
IsPending = true;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Checked before the still-moving branch below: a value that changes on
|
||||
// every frame would otherwise never reach it.
|
||||
if (nowMs - _divergedSinceMs >= MaxWaitMs)
|
||||
{
|
||||
Settle(current, nowMs);
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!current.Equals(_pending))
|
||||
{
|
||||
@@ -50,7 +92,16 @@ internal sealed class LayoutFingerprintGate
|
||||
if (nowMs - _pendingSinceMs < SettleMs)
|
||||
return false;
|
||||
|
||||
_applied = current;
|
||||
Settle(current, nowMs);
|
||||
return true;
|
||||
}
|
||||
|
||||
private void Settle(LayoutFingerprint current, long nowMs)
|
||||
{
|
||||
_applied = current;
|
||||
_pending = current;
|
||||
_pendingSinceMs = nowMs;
|
||||
_divergedSinceMs = nowMs;
|
||||
IsPending = false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user