refactor(messages): plan compact rows like cards instead of assuming a fixed height

Compact mode ran an ImGuiListClipper with CompactRowHeight = 18f. Two things
were wrong with that.

The number: at the default 12.75pt the font is 17px, and ChunkRenderer pushes
ItemSpacing to zero for the whole chunk loop, so a single-line compact row
advances the cursor by 17, not 18. The clipper seeded the cursor one pixel too
low per row, which accumulates into a visible drift against the scrollbar.

The assumption: compact rows are not constant height at all. DrawCompactRow
renders content with wrap: true, and WrapEncodedLine submits one text item per
wrapped line. At 620px and 17px type that kicks in around 70 characters, so
most chat lines are multi-line.

Both densities now share DrawRows/DrawLinearAndMeasure over the existing
CardClipPlanner, with the row painter passed in. The fixed height and the
clipper are gone.

Also corrects the CompensatedDummy comment: the cached heights carry no
trailing ItemSpacing (every row ends inside DrawChunks, where spacing is
zero, and ImGui writes the advance at submission). The compensation is
correct because it cancels the spacing the dummy itself appends.

CardClipPlanStep drives the invalidation hook directly, which now sits behind
the settle gate, so it walks a synthetic clock past the window.
This commit is contained in:
2026-08-17 23:32:04 +02:00
parent 39a8e95581
commit 0ef33934a2
2 changed files with 52 additions and 53 deletions
+15 -3
View File
@@ -1,6 +1,7 @@
using System.Collections.Generic; using System.Collections.Generic;
using Dalamud.Bindings.ImGui; using Dalamud.Bindings.ImGui;
using Dalamud.Plugin.SelfTest; using Dalamud.Plugin.SelfTest;
using HellionChat.Util;
namespace HellionChat.SelfTests; namespace HellionChat.SelfTests;
@@ -55,7 +56,10 @@ internal sealed class CardClipPlanStep : ISelfTestStep
int remaining; int remaining;
try try
{ {
messages.RunHeightCacheInvalidationForSelfTest(tab, 400f); // v1.10.0/A1: the fingerprint gate waits for the value to settle, so
// the step walks a synthetic clock past the window instead of sleeping.
var clock = Environment.TickCount64;
messages.RunHeightCacheInvalidationForSelfTest(tab, 400f, clock);
using (var snap = tab.Messages.GetReadOnly(3)) using (var snap = tab.Messages.GetReadOnly(3))
{ {
if (snap.Count > 0) if (snap.Count > 0)
@@ -64,12 +68,20 @@ internal sealed class CardClipPlanStep : ISelfTestStep
Plugin.Config.NameFormMode = Plugin.Config.NameFormMode =
savedForm == NameFormMode.Full ? NameFormMode.Initials : NameFormMode.Full; savedForm == NameFormMode.Full ? NameFormMode.Initials : NameFormMode.Full;
remaining = messages.RunHeightCacheInvalidationForSelfTest(tab, 400f); messages.RunHeightCacheInvalidationForSelfTest(tab, 400f, clock);
clock += LayoutFingerprintGate.SettleMs;
remaining = messages.RunHeightCacheInvalidationForSelfTest(tab, 400f, clock);
} }
finally finally
{ {
Plugin.Config.NameFormMode = savedForm; Plugin.Config.NameFormMode = savedForm;
messages.RunHeightCacheInvalidationForSelfTest(tab, 400f); var restore = Environment.TickCount64 + LayoutFingerprintGate.SettleMs * 2;
messages.RunHeightCacheInvalidationForSelfTest(tab, 400f, restore);
messages.RunHeightCacheInvalidationForSelfTest(
tab,
400f,
restore + LayoutFingerprintGate.SettleMs
);
} }
if (remaining != 0) if (remaining != 0)
+37 -50
View File
@@ -8,14 +8,11 @@ using HellionChat.Util;
namespace HellionChat.Ui.Components; namespace HellionChat.Ui.Components;
// Virtualised message list. Compact mode uses ImGuiListClipper (constant row // Virtualised message list. Both densities wrap to arbitrary heights, so both
// height); card mode rows wrap to arbitrary heights, so it runs its own // run the same prefix-sum clipper (CardClipPlanner) over a per-message height
// prefix-sum clipper (CardClipPlanner) over a per-message height cache, dropped // cache, dropped whenever the layout fingerprint settles on a new value.
// whenever the layout fingerprint (font/density/name-mode/width) changes.
internal sealed class MessageList internal sealed class MessageList
{ {
private const float CompactRowHeight = 18f;
private readonly FontManager _fonts; private readonly FontManager _fonts;
private readonly ChunkRenderer _chunkRenderer; private readonly ChunkRenderer _chunkRenderer;
@@ -70,10 +67,11 @@ internal sealed class MessageList
) => CardClipPlanner.Plan(heights, scrollY, viewportHeight); ) => CardClipPlanner.Plan(heights, scrollY, viewportHeight);
// SelfTest hook (B2): drives the live invalidation, returns the tab's remaining // SelfTest hook (B2): drives the live invalidation, returns the tab's remaining
// cached-height count so the step can assert the drop. // cached-height count so the step can assert the drop. nowMs is a parameter so
internal int RunHeightCacheInvalidationForSelfTest(Tab tab, float contentWidth) // the step can step past the settle window without sleeping (v1.10.0/A1).
internal int RunHeightCacheInvalidationForSelfTest(Tab tab, float contentWidth, long nowMs)
{ {
InvalidateHeightCacheIfLayoutChanged(tab, contentWidth); InvalidateHeightCacheIfLayoutChanged(tab, contentWidth, nowMs);
using var messages = tab.Messages.GetReadOnly(3); using var messages = tab.Messages.GetReadOnly(3);
return messages.Count(m => m.Height.ContainsKey(tab.Identifier)); return messages.Count(m => m.Height.ContainsKey(tab.Identifier));
} }
@@ -99,7 +97,7 @@ internal sealed class MessageList
// record compare per frame, a clear only after a real settings/resize change // record compare per frame, a clear only after a real settings/resize change
// stopped moving. The gate is what keeps a slider drag from rebuilding the // stopped moving. The gate is what keeps a slider drag from rebuilding the
// whole tab on every frame. // whole tab on every frame.
private void InvalidateHeightCacheIfLayoutChanged(Tab tab, float contentWidth) private void InvalidateHeightCacheIfLayoutChanged(Tab tab, float contentWidth, long nowMs)
{ {
if (!_fingerprintGates.TryGetValue(tab.Identifier, out var gate)) if (!_fingerprintGates.TryGetValue(tab.Identifier, out var gate))
{ {
@@ -107,7 +105,7 @@ internal sealed class MessageList
_fingerprintGates[tab.Identifier] = gate; _fingerprintGates[tab.Identifier] = gate;
} }
if (!gate.ShouldInvalidate(BuildLayoutFingerprint(contentWidth), Environment.TickCount64)) if (!gate.ShouldInvalidate(BuildLayoutFingerprint(contentWidth), nowMs))
return; return;
using var messages = tab.Messages.GetReadOnly(3); using var messages = tab.Messages.GetReadOnly(3);
@@ -131,10 +129,14 @@ internal sealed class MessageList
// and a runaway content-height computation. // and a runaway content-height computation.
var compact = Plugin.Config.UseCompactDensity; var compact = Plugin.Config.UseCompactDensity;
// B2: drop stale cached heights before the snapshot draw (card path only — // B2: drop stale cached heights before the snapshot draw. Both densities
// compact rows are constant height). Width read here while it is valid. // need this now -- compact rows are not constant height either, they wrap.
if (!compact) // Width read here while it is valid.
InvalidateHeightCacheIfLayoutChanged(tab, ImGui.GetContentRegionAvail().X); InvalidateHeightCacheIfLayoutChanged(
tab,
ImGui.GetContentRegionAvail().X,
Environment.TickCount64
);
using var messages = tab.Messages.GetReadOnly(3); using var messages = tab.Messages.GetReadOnly(3);
@@ -143,10 +145,7 @@ internal sealed class MessageList
// the parent child's scroll state, which is the one MainWindow owns. // the parent child's scroll state, which is the one MainWindow owns.
var pinnedToBottom = ImGui.GetScrollY() >= ImGui.GetScrollMaxY() - 1f; var pinnedToBottom = ImGui.GetScrollY() >= ImGui.GetScrollMaxY() - 1f;
if (compact) DrawRows(tab, messages, compact ? DrawCompactRow : DrawCardRow);
DrawCompact(messages);
else
DrawCard(tab, messages);
// B3-5: scroll values are frame-constant inside the child, so this // B3-5: scroll values are frame-constant inside the child, so this
// reflects the current frame's state wherever it runs; kept after the // reflects the current frame's state wherever it runs; kept after the
@@ -213,28 +212,6 @@ internal sealed class MessageList
_scrollToBottomRequested = true; _scrollToBottomRequested = true;
} }
private void DrawCompact(IReadOnlyList<Message> messages)
{
unsafe
{
var clipper = new ImGuiListClipperPtr(ImGuiNative.ImGuiListClipper());
try
{
clipper.Begin(messages.Count, CompactRowHeight);
while (clipper.Step())
{
for (var i = clipper.DisplayStart; i < clipper.DisplayEnd; i++)
DrawCompactRow(messages[i]);
}
clipper.End();
}
finally
{
clipper.Destroy();
}
}
}
private void DrawCompactRow(Message message) private void DrawCompactRow(Message message)
{ {
// B2-1/B2-2: render the sender through DrawChunks (the name-aware path // B2-1/B2-2: render the sender through DrawChunks (the name-aware path
@@ -259,7 +236,9 @@ internal sealed class MessageList
_chunkRenderer.DrawChunks(message.Content, wrap: true, handler: _handler, lineWidth: 0f); _chunkRenderer.DrawChunks(message.Content, wrap: true, handler: _handler, lineWidth: 0f);
} }
private void DrawCard(Tab tab, IReadOnlyList<Message> messages) // 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)
{ {
var tabId = tab.Identifier; var tabId = tab.Identifier;
var count = messages.Count; var count = messages.Count;
@@ -280,15 +259,18 @@ internal sealed class MessageList
if (!allCached) if (!allCached)
{ {
DrawCardLinearAndMeasure(tabId, messages); DrawLinearAndMeasure(tabId, messages, drawRow);
return; return;
} }
var plan = CardClipPlanner.Plan(heights, ImGui.GetScrollY(), ImGui.GetWindowSize().Y); var plan = CardClipPlanner.Plan(heights, ImGui.GetScrollY(), ImGui.GetWindowSize().Y);
// Cached heights already include one trailing ItemSpacing.y; a dummy adds its // A dummy is submitted outside any style push, so it appends its own
// own, which would shove the first visible row down one spacing (jitter). // trailing ItemSpacing.y. Subtracting one spacing per dummy makes the dummy
// Subtract one spacing per dummy so offset AND total content height stay exact. // advance the cursor by exactly the planned height. (The measured row
// heights carry no trailing spacing: every row ends inside DrawChunks,
// which pushes ItemSpacing to zero, and ImGui writes the advance at item
// submission time.)
var spacingY = ImGui.GetStyle().ItemSpacing.Y; var spacingY = ImGui.GetStyle().ItemSpacing.Y;
float CompensatedDummy(float planned) => Math.Max(0f, planned - spacingY); float CompensatedDummy(float planned) => Math.Max(0f, planned - spacingY);
@@ -307,7 +289,7 @@ internal sealed class MessageList
{ {
var msg = messages[i]; var msg = messages[i];
var before = ImGui.GetCursorPosY(); var before = ImGui.GetCursorPosY();
DrawCardRow(msg); drawRow(msg);
var after = ImGui.GetCursorPosY(); var after = ImGui.GetCursorPosY();
msg.Height[tabId] = after - before; msg.Height[tabId] = after - before;
msg.IsVisible[tabId] = true; msg.IsVisible[tabId] = true;
@@ -318,13 +300,18 @@ internal sealed class MessageList
} }
// First-frame / post-invalidation fallback: draw + measure every row into the // First-frame / post-invalidation fallback: draw + measure every row into the
// cache so the next frame can take the planned path. // cache so the next frame can take the planned path. The settle gate on the
private void DrawCardLinearAndMeasure(Guid tabId, IReadOnlyList<Message> messages) // layout fingerprint is what keeps a resize drag from landing here every frame.
private void DrawLinearAndMeasure(
Guid tabId,
IReadOnlyList<Message> messages,
Action<Message> drawRow
)
{ {
foreach (var msg in messages) foreach (var msg in messages)
{ {
var before = ImGui.GetCursorPosY(); var before = ImGui.GetCursorPosY();
DrawCardRow(msg); drawRow(msg);
var after = ImGui.GetCursorPosY(); var after = ImGui.GetCursorPosY();
msg.Height[tabId] = after - before; msg.Height[tabId] = after - before;
msg.IsVisible[tabId] = ImGui.IsItemVisible(); msg.IsVisible[tabId] = ImGui.IsItemVisible();