ui: a scroll names the child it moved instead of giving up on the graph
`SetVerticalScroll` and `SetHorizontalScroll` called `Model::touch_layout` on the line before `reanchor_child`. That gave up naming for the whole graph and then *erased* the precise touch that followed: `reanchor_child` re-anchors the scroll child through `write_anchor`, which reads an offsets-only change (same point, same target) and names the child node — but `touch_layout_node` will not un-say an imprecise touch, so the give-up won on every call. Naming the child is the whole truth here. A scroll offset reaches the layout only as that one anchor: anchor targets are ids fixed at `SetPoint`, so no edge moves and the roster is unchanged, and the clip a scroll child draws within is the ScrollFrame's resolved rect rather than the offset — `clip::scroll_clip_sources` rebuilds it per call and caches nothing. Measured on grom's 2026-10-21 17:08 session, which was a map-and-panel run. These two lines were **472 of the run's 806 whole-graph derivations**, the largest single source by a factor of two, and the cost lands on the Lua that reads a frame edge afterwards rather than on the pass's own `resolve` phase: | | n | fps | UI pass | `frame:GetLeft` | |---|---|---|---|---| | windows with >= 10 scroll derivations | 12 | 68.5 (min 53) | 5.93 ms/f | 2.18 ms/f | | windows with no derivations at all | 11 | 92.4 (min 77) | 1.62 ms/f | 0.05 ms/f | `r(scroll derivations, GetLeft us/f) = +0.907` over 57 in-play windows, against `+0.014` for every other derivation site. `[ui-verbs]` has `frame:GetLeft` at 94-376 us a call in those windows and 0.6-1.5 us in the quiet ones, on ~14 calls a frame, because stock FrameXML reads frame edges constantly and each read past the clean epoch re-derived. That is why a scrolling panel — the world map, the quest log, a profession list — was the expensive thing on screen. The test scrolls both axes on the shipped interface and asserts zero derivations, with a positive control so it cannot pass on a dead counter. Put one `touch_layout` back and it reads exactly 10 for 10 scrolls. `WOW_LAYOUT_VERIFY=1` over the real 92-addon interface gives `frames 13379 -> 13379, regions 29597 -> 29645` — identical to the recorded pre-existing baseline, so this narrowing introduced nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -383,6 +383,82 @@ fn a_region_moving_every_frame_costs_no_graph_derivation_on_the_shipped_ui() {
|
||||
);
|
||||
}
|
||||
|
||||
/// Scrolling a panel costs zero derivations: the offset reaches the layout only as the scroll
|
||||
/// child's anchor, which `write_anchor` names, so the two setters owe no conservative touch of
|
||||
/// their own.
|
||||
///
|
||||
/// This is the regression test for the largest single derivation source yet measured on a live
|
||||
/// session — `SetVerticalScroll`/`SetHorizontalScroll` called `Model::touch_layout` on the line
|
||||
/// before `reanchor_child`, which gave up naming the whole graph and then swallowed the precise
|
||||
/// touch that followed. 472 of 806 derivations in grom's 2026-10-21 17:08 run, and the reason a
|
||||
/// scrolling panel dragged `frame:GetLeft` from ~1 us a call to 94-376 us.
|
||||
///
|
||||
/// Both axes, because they were byte-clones of each other and a fix to one is easy to forget.
|
||||
#[test]
|
||||
fn scrolling_a_panel_costs_no_graph_derivation_on_the_shipped_ui() {
|
||||
benilla_formats::wow_data_or_skip!();
|
||||
let mut s = settled_default_ui();
|
||||
s.run(
|
||||
r#"
|
||||
BenchScroll = CreateFrame("ScrollFrame", "BenchScroll", UIParent)
|
||||
BenchScroll:SetPoint("CENTER", 0, 0)
|
||||
BenchScroll:SetWidth(300); BenchScroll:SetHeight(200)
|
||||
BenchScrollChild = CreateFrame("Frame", "BenchScrollChild", BenchScroll)
|
||||
BenchScrollChild:SetWidth(300); BenchScrollChild:SetHeight(900)
|
||||
BenchScroll:SetScrollChild(BenchScrollChild)
|
||||
-- A row anchored inside the child, so the scroll really does move something that resolves.
|
||||
BenchScrollRow = BenchScrollChild:CreateFontString(nil, "OVERLAY", "GameFontNormal")
|
||||
BenchScrollRow:SetPoint("TOPLEFT", BenchScrollChild, "TOPLEFT", 4, -4)
|
||||
BenchScrollRow:SetText("row")
|
||||
bench_scroll_n = 0
|
||||
function bench_scroll_frame()
|
||||
bench_scroll_n = bench_scroll_n + 1
|
||||
BenchScroll:SetVerticalScroll(bench_scroll_n * 7)
|
||||
BenchScroll:SetHorizontalScroll(bench_scroll_n * 2)
|
||||
-- The read stock FrameXML does dozens of times a frame, and what forces the resolve.
|
||||
local _ = BenchScrollRow:GetLeft()
|
||||
end
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Positive control: the births above move the roster, so settling them must derive. Without
|
||||
// this the assertion below passes just as well on a counter that never moves.
|
||||
let born_at = s.layout_derivations();
|
||||
for _ in 0..6 {
|
||||
s.run("bench_scroll_frame()").unwrap();
|
||||
app_frame_ticked(&mut s, 1.0 / 144.0);
|
||||
}
|
||||
assert!(
|
||||
s.layout_derivations() > born_at,
|
||||
"creating the ScrollFrame, its child and a FontString must derive the graph — a birth \
|
||||
moves the roster. Reading zero here means `layout_derivations` never moves and the \
|
||||
assertion below is vacuous."
|
||||
);
|
||||
|
||||
let derives_before = s.layout_derivations();
|
||||
let walks_before = s.layout_gate_walks();
|
||||
for _ in 0..10 {
|
||||
s.run("bench_scroll_frame()").unwrap();
|
||||
app_frame_ticked(&mut s, 1.0 / 144.0);
|
||||
}
|
||||
let derives = s.layout_derivations() - derives_before;
|
||||
let walks = s.layout_gate_walks() - walks_before;
|
||||
assert!(
|
||||
walks >= 10,
|
||||
"the frames must still be reaching the gate at all — {walks} walks over 10 frames means \
|
||||
this test stopped exercising the path it is guarding"
|
||||
);
|
||||
assert_eq!(
|
||||
derives, 0,
|
||||
"10 frames of scrolling derived the layout graph {derives} times, i.e. about one per \
|
||||
scroll. A scroll offset moves exactly one thing — the scroll child's anchor offsets, \
|
||||
same point and same target — so `write_anchor` names the child and nothing else needs \
|
||||
saying. A derivation here means a setter has gone back to the conservative \
|
||||
`Model::touch_layout`, which also erases the precise touch that follows it."
|
||||
);
|
||||
}
|
||||
|
||||
/// A tooltip content change solves a tooltip-sized scope on the full shipped UI, gated on counts,
|
||||
/// not milliseconds, which machine state skews.
|
||||
#[test]
|
||||
|
||||
@@ -152,6 +152,26 @@ fn subtree_font_strings(model: &Model, f: FrameHandle) -> Vec<crate::widget::Reg
|
||||
///
|
||||
/// One point, merged, because `SetPoint` merges: a child authored `TOPLEFT`/`TOPRIGHT` keeps the
|
||||
/// `TOPRIGHT` that gives it its width.
|
||||
/// Place the scroll child at the current offset (`0x787100`), and **name that write** — this is the
|
||||
/// only way a scroll offset reaches the layout, so it is the only invalidation either setter owes.
|
||||
///
|
||||
/// The point and the target are the same on every call (`TopLeft` to the ScrollFrame's `TopLeft`);
|
||||
/// only the two offsets move. [`write_anchor`](crate::script::object::layout_methods::write_anchor)
|
||||
/// reads that as a value-only change and names the child node, which is precisely true: anchor
|
||||
/// targets are ids fixed at `SetPoint`, so no edge of the graph moves and the roster is unchanged.
|
||||
/// The clip the child draws within is the ScrollFrame's *resolved rect*, not the offset, and
|
||||
/// [`super::clip::scroll_clip_sources`] rebuilds it per call — so a scroll invalidates nothing
|
||||
/// there either.
|
||||
///
|
||||
/// Both setters used to call `Model::touch_layout` on the line before this, which gave up naming
|
||||
/// for the whole graph and then **erased** the precise touch below: `touch_layout_node` will not
|
||||
/// un-say an imprecise touch, so the give-up won every time. The cost was a whole-graph derivation
|
||||
/// per scroll. In grom's 2026-10-21 17:08 session these two lines were **472 of the run's 806**
|
||||
/// derivations, the largest source by a factor of two, and they are why a scrolling panel is
|
||||
/// expensive: `[ui-verbs]` has `frame:GetLeft` at **94-376 us a call** in those windows against
|
||||
/// 0.6-1.5 us in the quiet ones, because every forced resolve past the clean epoch re-derives, and
|
||||
/// stock FrameXML reads a frame edge dozens of times a frame. Those windows ran at 53-68 fps where
|
||||
/// the rest of the session held 85.
|
||||
fn reanchor_child(lua: &Lua, this: &Table) -> mlua::Result<()> {
|
||||
let h = frame_handle_of(lua, this)?;
|
||||
let mut model = lua.app_data_mut::<Model>().expect("model app_data");
|
||||
@@ -250,7 +270,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
|
||||
if !changed {
|
||||
return Ok(());
|
||||
}
|
||||
lua.app_data_mut::<Model>().expect("model").touch_layout();
|
||||
// No `touch_layout` here: see [`reanchor_child`], which names the one node that moved.
|
||||
reanchor_child(lua, &this)?;
|
||||
fire_scroll(lua, &this, "OnVerticalScroll", px)
|
||||
})?,
|
||||
@@ -283,7 +303,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
|
||||
if !changed {
|
||||
return Ok(());
|
||||
}
|
||||
lua.app_data_mut::<Model>().expect("model").touch_layout();
|
||||
// No `touch_layout` here either: see [`reanchor_child`].
|
||||
reanchor_child(lua, &this)?;
|
||||
fire_scroll(lua, &this, "OnHorizontalScroll", px)
|
||||
})?,
|
||||
|
||||
Reference in New Issue
Block a user