fix(style): stop the hover registry from churning on idle rows
Review of block B found Query allocating an entry for every element it was
asked about, hovered or not. The cycle: Query creates the entry, the next
BeginFrame steps it to zero, evicts it, and the next Query creates it again.
With fifteen tabs that is fifteen allocations plus fifteen dictionary inserts
and removes per frame, permanently, with the mouse nowhere near the window.
That is exactly the property master spec 7.5 asks for and the one this block
claimed to improve, so it ate the two string allocations 31fa410 had just
saved. An unhovered element with no entry now returns zero without creating
one.
The footprint self-test only ever queried with hovered: true, which is why it
could not see this. It now runs an idle phase as well.
Four smaller items from the same review:
Advance skipped clearing the hover flags when deltaTime was zero, so such a
frame carried the previous frame's state forward.
Metrics reads GlobalScaleSafe now. The unsafe variant throws while the
interface manager is still coming up, and block F pulls Metrics into more call
sites.
Badge.CalcSize returned a full-size box for a count of zero while Draw drew
nothing, so a caller that reserves and then draws left a badge-shaped hole on
every tab without unread messages -- the normal case.
IconButton caches its glyph strings; ToIconString allocates on every call and
keeps no cache of its own. And the widget gallery clamps its own row width,
since asserting on a zero-width button in the window that demonstrates the
clamp would be a poor look.
This commit is contained in:
@@ -59,13 +59,36 @@ internal sealed class HoverStateFootprintStep : ISelfTestStep
|
||||
|
||||
if (HoverState.TrackedCount != 0)
|
||||
{
|
||||
var msg = $"Registry leaked {HoverState.TrackedCount} entries after fade-out";
|
||||
ImGui.Text(msg);
|
||||
SelfTestReport.Append(Name, "FAIL", new[] { msg });
|
||||
var leak = $"Registry leaked {HoverState.TrackedCount} entries after fade-out";
|
||||
ImGui.Text(leak);
|
||||
SelfTestReport.Append(Name, "FAIL", new[] { leak });
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
SelfTestReport.Append(Name, "PASS", new[] { "registry empty after fade-out" });
|
||||
// The case a hovered-only probe cannot see: querying an element that
|
||||
// is NOT hovered must not create an entry. Otherwise every row in
|
||||
// the window allocates one per frame and loses it again in the next
|
||||
// BeginFrame, forever.
|
||||
for (var frame = 0; frame < 10; frame++)
|
||||
{
|
||||
foreach (var id in ids)
|
||||
HoverState.Query(id, hovered: false);
|
||||
HoverState.AdvanceForTest(Frame);
|
||||
|
||||
if (HoverState.TrackedCount == 0)
|
||||
continue;
|
||||
|
||||
var churn = $"Unhovered query created {HoverState.TrackedCount} entries";
|
||||
ImGui.Text(churn);
|
||||
SelfTestReport.Append(Name, "FAIL", new[] { churn });
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
SelfTestReport.Append(
|
||||
Name,
|
||||
"PASS",
|
||||
new[] { "no growth while hovered, empty after fade-out, no churn when idle" }
|
||||
);
|
||||
return SelfTestStepResult.Pass;
|
||||
}
|
||||
finally
|
||||
|
||||
@@ -48,6 +48,12 @@ internal static class HoverState
|
||||
|
||||
if (!Entries.TryGetValue(id, out var entry))
|
||||
{
|
||||
// Nothing to fade from, so nothing to track. Without this a
|
||||
// never-hovered element allocates an entry in Query and loses it
|
||||
// again in the next BeginFrame, every frame, for every row.
|
||||
if (!hovered)
|
||||
return 0f;
|
||||
|
||||
entry = new Entry();
|
||||
Entries[id] = entry;
|
||||
}
|
||||
@@ -58,10 +64,19 @@ internal static class HoverState
|
||||
return entry.Value;
|
||||
}
|
||||
|
||||
// Never re-entrant: BeginFrame runs once from the draw thread, AdvanceForTest
|
||||
// only from a self-test step. Evicted is shared, so overlapping calls would
|
||||
// corrupt it.
|
||||
private static void Advance(float deltaTime)
|
||||
{
|
||||
if (deltaTime <= 0f)
|
||||
{
|
||||
// Still clear the flags: a zero-delta frame otherwise carries the
|
||||
// previous frame's hover state forward.
|
||||
foreach (var entry in Entries.Values)
|
||||
entry.Hovered = false;
|
||||
return;
|
||||
}
|
||||
|
||||
Evicted.Clear();
|
||||
foreach (var (id, entry) in Entries)
|
||||
|
||||
@@ -66,8 +66,10 @@ internal static class Metrics
|
||||
if (frame == _cachedFrame)
|
||||
return _cachedScale;
|
||||
|
||||
// Safe variant: GlobalScale throws while the interface manager is
|
||||
// still coming up, and Block F pulls Metrics into more call sites.
|
||||
_cachedScale = ImGuiHelpers.GlobalScaleSafe;
|
||||
_cachedFrame = frame;
|
||||
_cachedScale = ImGuiHelpers.GlobalScale;
|
||||
return _cachedScale;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,8 +21,14 @@ internal static class Badge
|
||||
internal static string Format(int count, int maxCount) =>
|
||||
count > maxCount ? $"{maxCount}+" : count.ToString();
|
||||
|
||||
// Zero for a count of zero, matching Draw: a caller that reserves space and
|
||||
// then draws would otherwise leave a badge-shaped hole on every tab without
|
||||
// unread messages, which is the normal case.
|
||||
internal static Vector2 CalcSize(int count, BadgeStyle? styleOverride = null)
|
||||
{
|
||||
if (count <= 0)
|
||||
return Vector2.Zero;
|
||||
|
||||
var style = styleOverride ?? new BadgeStyle();
|
||||
var text = Format(count, style.MaxCount);
|
||||
return WidgetGeometry.Badge(
|
||||
|
||||
@@ -20,9 +20,23 @@ internal readonly record struct IconButtonStyle
|
||||
// the hit area plus the glyph.
|
||||
internal static class IconButton
|
||||
{
|
||||
// FontAwesomeExtensions.ToIconString allocates on every call and holds no
|
||||
// cache of its own, so a per-frame glyph would allocate per button per frame.
|
||||
private static readonly Dictionary<FontAwesomeIcon, string> GlyphCache = [];
|
||||
|
||||
internal static Vector2 CalcSize(float width, float height) =>
|
||||
WidgetGeometry.IconButton(width, height);
|
||||
|
||||
private static string Glyph(FontAwesomeIcon icon)
|
||||
{
|
||||
if (GlyphCache.TryGetValue(icon, out var s))
|
||||
return s;
|
||||
|
||||
s = icon.ToIconString();
|
||||
GlyphCache[icon] = s;
|
||||
return s;
|
||||
}
|
||||
|
||||
internal static (bool Clicked, bool Hovered) Draw(
|
||||
uint id,
|
||||
Vector2 size,
|
||||
@@ -59,7 +73,7 @@ internal static class IconButton
|
||||
{
|
||||
using (font.Push())
|
||||
{
|
||||
var text = icon.ToIconString();
|
||||
var text = Glyph(icon);
|
||||
var textSize = ImGui.CalcTextSize(text);
|
||||
dl.AddText(origin + (clamped - textSize) * 0.5f, glyphAbgr, text);
|
||||
}
|
||||
|
||||
@@ -55,8 +55,14 @@ internal sealed class WidgetGalleryWindow : Window
|
||||
ImGui.TextUnformatted("Row");
|
||||
ImGui.Checkbox("active##row", ref _rowActive);
|
||||
|
||||
var width = ImGui.GetContentRegionAvail().X;
|
||||
var height = Metrics.SidebarRowHeight;
|
||||
// Clamped: this window demonstrates the guard, it should not be the one
|
||||
// that asserts when dragged narrow.
|
||||
var rowSize = HellionChat.Util.WidgetGeometry.IconButton(
|
||||
ImGui.GetContentRegionAvail().X,
|
||||
Metrics.SidebarRowHeight
|
||||
);
|
||||
var width = rowSize.X;
|
||||
var height = rowSize.Y;
|
||||
|
||||
for (var i = 0; i < 3; i++)
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user