diff --git a/crates/benilla-ui/src/script/layout.rs b/crates/benilla-ui/src/script/layout.rs index 12986224..6c8af115 100644 --- a/crates/benilla-ui/src/script/layout.rs +++ b/crates/benilla-ui/src/script/layout.rs @@ -164,6 +164,96 @@ pub(super) const FONTSTRING_MIN_SPAN: f32 = 1.0; /// Marks a [`LayoutScope::node_of`] index as one into `regions` rather than `plan`. const REGION_TAG: u32 = 0x8000_0000; +/// Seed `x`'s cached rect for this pass if `x` is clean: a dirty node reads it and the round loop +/// will not recompute it. A dirty `x`, or one with no roster slot, needs nothing — the first is +/// solved and the second constrains nothing (see [`LayoutScope::edge`]). +fn seed_dep(scope: &LayoutScope, x: u32, seed_frames: &mut Vec, seed_regions: &mut Vec) { + if scope.dirty.get(x as usize).copied().unwrap_or(false) { + return; + } + let Some(&m) = scope.node_of.get(x as usize) else { + return; + }; + if m == u32::MAX { + return; + } + if m & REGION_TAG != 0 { + seed_regions.push(m & !REGION_TAG); + } else { + seed_frames.push(m); + } +} + +/// `WOW_LAYOUT_VERIFY`: recompute the four selection lists the way the pass used to — ask **every** +/// id whether one of its dependents is dirty — and require the same sets. +/// +/// `resolve_layout`'s own verify compares *rects* after a full re-derivation, which catches this +/// pass getting the sets wrong but also everything else that can make a rect differ. This isolates +/// the question, so a selection bug cannot hide behind an unrelated red and an unrelated red cannot +/// be blamed on the selection. Sets, not sequences: see the pass's own comment on why order here +/// was never canonical. +fn verify_selection( + scope: &LayoutScope, + solve_frames: &[u32], + sweep_regions: &[u32], + seed_frames: &[u32], + seed_regions: &[u32], +) { + let read_by_dirty = |id: usize| -> bool { + let mut e = scope.dep_head[id]; + while e != u32::MAX { + if scope.dirty[scope.dep_to[e as usize] as usize] { + return true; + } + e = scope.dep_next[e as usize]; + } + false + }; + let (mut want_solve, mut want_sweep) = (Vec::new(), Vec::new()); + let (mut want_sf, mut want_sr) = (Vec::new(), Vec::new()); + for id in 0..scope.node_of.len() { + let n = scope.node_of[id]; + if n == u32::MAX { + continue; + } + let (idx, is_region, dirty) = (n & !REGION_TAG, n & REGION_TAG != 0, scope.dirty[id]); + if is_region { + if dirty { + want_sweep.push(idx); + want_sr.push(idx); + } else if read_by_dirty(id) { + want_sr.push(idx); + } + } else if dirty { + want_solve.push(idx); + } else if read_by_dirty(id) { + want_sf.push(idx); + } + } + let sorted = |v: &[u32]| { + let mut v = v.to_vec(); + v.sort_unstable(); + v.dedup(); + v + }; + for (what, got, want) in [ + ("solve_frames", sorted(solve_frames), sorted(&want_solve)), + ("sweep_regions", sorted(sweep_regions), sorted(&want_sweep)), + ("seed_frames", sorted(seed_frames), sorted(&want_sf)), + ("seed_regions", sorted(seed_regions), sorted(&want_sr)), + ] { + assert!( + got == want, + "WOW_LAYOUT_VERIFY: walking the dirty set chose a different {what} than walking the \ + graph — {} vs {} entries. A node's dependencies are its anchors' targets plus a \ + region's owner; if `LayoutScope::edge` gained a fourth source, the selection pass \ + needs it too.", + got.len(), + want.len(), + ); + } +} + /// Per-node layout scope, so a resolve touches only what moved. Indexed densely by layout id, as /// frames and regions share one counter (`Model::next_id`); nothing allocates on a steady frame. #[derive(Default)] @@ -186,6 +276,10 @@ pub(crate) struct LayoutScope { dep_free: Vec, /// Per id: in the dirty closure. dirty: Vec, + /// The same set as a list, so a pass can walk the dirty nodes instead of every id. Written + /// wherever [`Self::dirty`] turns true, so the two cannot disagree; no duplicates, because + /// every writer goes through the false→true edge. + dirty_ids: Vec, /// The closure's worklist. stack: Vec, /// The cached frame roster, `(handle, id)`, kept across resolves. @@ -219,6 +313,7 @@ impl LayoutScope { self.dep_head.resize(n, u32::MAX); self.dirty.clear(); self.dirty.resize(n, false); + self.dirty_ids.clear(); self.dep_to.clear(); self.dep_next.clear(); self.dep_free.clear(); @@ -232,6 +327,7 @@ impl LayoutScope { fn begin_incremental(&mut self) { self.dirty.clear(); self.dirty.resize(self.node_of.len(), false); + self.dirty_ids.clear(); self.stack.clear(); self.staged.clear(); } @@ -314,6 +410,7 @@ impl LayoutScope { if !*d { *d = true; self.stack.push(id); + self.dirty_ids.push(id); } } } @@ -332,9 +429,12 @@ impl LayoutScope { /// Mark every node dirty, for what a per-node diff cannot see (a death, a screen move). fn dirty_all(&mut self) { + self.dirty_ids.clear(); for (id, &n) in self.node_of.iter().enumerate() { if n != u32::MAX { self.dirty[id] = true; + #[allow(clippy::cast_possible_truncation)] + self.dirty_ids.push(id as u32); } } } @@ -683,10 +783,10 @@ impl UiScript { if full_scope { scope.dirty_all(); } else { - for (id, &n) in scope.node_of.iter().enumerate() { - if n != u32::MAX && scope.now[id] != scope.last[id] { - scope.stack.push(id as u32); - scope.dirty[id] = true; + for id in 0..scope.node_of.len() { + if scope.node_of[id] != u32::MAX && scope.now[id] != scope.last[id] { + #[allow(clippy::cast_possible_truncation)] + scope.seed(id as u32); } } scope.close(); @@ -698,38 +798,83 @@ impl UiScript { let mut sweep_regions: Vec = Vec::new(); let mut seed_frames: Vec = Vec::new(); let mut seed_regions: Vec = Vec::new(); - // Whether a node in the closure reads `id`; a rect nothing dirty reads is not seeded. - let read_by_dirty = |scope: &LayoutScope, id: u32| -> bool { - let mut e = scope.dep_head[id as usize]; - while e != u32::MAX { - if scope.dirty[scope.dep_to[e as usize] as usize] { - return true; + // `[layout-pre]` ends at the gate and `frame_us`/`region_us` start at the round loop, so + // the walk between them — every id in the graph, not every *dirty* id — was in neither + // line. That gap is why an 18 us `GetLeft` looked like a 3 us resolve. + let t_seedsel = layout_prof_enabled().then(std::time::Instant::now); + // Walk out from the dirty set rather than across the graph. + // + // This used to ask **every id** "is one of your dependents dirty?" — 18 203 frames and + // 42 284 regions on the player's interface — for a dirty closure whose steady-state median + // is 2 frames and 4 regions. It also sat between `[layout-pre]`'s last lap and the round + // loop's first, so neither report charged it, which is how a `GetLeft` that measured + // 17.5–18.4 us a call live (`WOW_UI_VERBS`, six a frame through stock `MouseIsOver`) could + // force a resolve the profiler called 3 us. + // + // The four lists come out the same **sets**, by the same relation read the other way + // round: a clean node is seeded iff some dirty node reads it, and a node's dependencies + // are exactly the three `scope.edge` arguments in the roster walk — its anchors' targets, + // plus a region's owner frame, whose scale it reads. `WOW_LAYOUT_VERIFY=1` re-derives the + // whole graph and compares every rect, which is what would catch an enumeration that + // drifted from `edge`'s. + // + // Sets, not sequences: both rosters are built from `HashMap` iteration (`frame_to_id` for + // frames, `region_data` for regions), so no pass here ever had a canonical order, and the + // region sweep is a fixpoint whose round cap covers any chain — order moves `rounds`, not + // rects. Only the seed lists are sorted, to drop the duplicates a shared dependency would + // otherwise re-`set_external` on every round. + // A full-scope pass has no seeds to find: every node is dirty, so every `seed_dep` would + // return early. Skip the walk rather than make 60 000 calls that do nothing. + let seeds_possible = !full_scope; + for &did in &scope.dirty_ids { + let n = match scope.node_of.get(did as usize) { + Some(&n) if n != u32::MAX => n, + _ => continue, + }; + let idx = n & !REGION_TAG; + if n & REGION_TAG != 0 { + sweep_regions.push(idx); + // The frame solve runs before the sweep, so a dirty region is seeded as well. + seed_regions.push(idx); + if !seeds_possible { + continue; } - e = scope.dep_next[e as usize]; - } - false - }; - for id in 0..scope.node_of.len() { - let n = scope.node_of[id]; - if n == u32::MAX { - continue; - } - let (idx, is_region, dirty) = (n & !REGION_TAG, n & REGION_TAG != 0, scope.dirty[id]); - #[allow(clippy::cast_possible_truncation)] - let id32 = id as u32; - if is_region { - if dirty { - sweep_regions.push(idx); - seed_regions.push(idx); - } else if read_by_dirty(scope, id32) { - seed_regions.push(idx); + let row = scope.regions[idx as usize]; + if let Some(d) = region_data.get(&row.rh) { + for a in &d.anchors { + seed_dep(scope, a.relative_to, &mut seed_frames, &mut seed_regions); + } } - } else if dirty { + if let Some(&owner_id) = frame_to_id.get(&row.owner) { + seed_dep(scope, owner_id, &mut seed_frames, &mut seed_regions); + } + } else { solve_frames.push(idx); - } else if read_by_dirty(scope, id32) { - seed_frames.push(idx); + if !seeds_possible { + continue; + } + let (h, _) = scope.plan[idx as usize]; + if let Some(input) = layout_inputs.get(&h) { + for a in &input.anchors { + seed_dep(scope, a.relative_to, &mut seed_frames, &mut seed_regions); + } + } } } + seed_frames.sort_unstable(); + seed_frames.dedup(); + seed_regions.sort_unstable(); + seed_regions.dedup(); + let seedsel_us = t_seedsel.map_or(0, |t| t.elapsed().as_micros()); + if layout_verify_enabled() { + verify_selection( + scope, + &solve_frames, + &sweep_regions, + &seed_frames, + &seed_regions, + ); + } // This solve's width, beside `layout_solves` (how often) and `layout_rounds` (how deep). *layout_last_scope = (solve_frames.len(), sweep_regions.len()); let round_cap = scope.plan.len() + region_data.len() + 2; @@ -892,7 +1037,8 @@ impl UiScript { // `anchored` is the sweep's roster, `solved` and `swept` this pass's scope. eprintln!( "[layout-prof] rounds={} frames={} regions_total={} anchored={} \ - solved={} swept={} scope={} regions_swept={} frame_us={} region_us={}", + solved={} swept={} scope={} regions_swept={} seedsel_us={seedsel_us} \ + frame_us={} region_us={}", round + 1, scope.plan.len(), region_data.len(),