From b63d6e3609141094d625b3751cc5ff810adc7476 Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:25:49 +0700 Subject: [PATCH] A raise's level compaction stops at the stratum's visible high-water (#906) A raise's level compaction counted and packed every level any frame of the stratum held, hidden or shown, so a frame given a high level while hidden was pulled down and later showed under a dialog it should cover. In the reference, each stratum keeps a level bound that grows only when a visible frame links into a level, and the compaction examines and moves only the levels below it, then sets the bound to its count, which the raised frame takes. A frame above the bound keeps its level. The arena now keeps that bound per stratum, grown on every visible link and set by the compaction, which packs only below it. Tests cover the hidden frame above the bound and a high show raising the bound for the next raise. Fixes #885 Co-Authored-By: Claude Sonnet 5.5 --- .../benilla-ui/src/script/object/toplevel.rs | 7 +- .../benilla-ui/src/script/tests/toplevel.rs | 68 +++++++++++++++++++ crates/benilla-ui/src/widget/mod.rs | 20 ++++++ crates/benilla-ui/src/widget/propagation.rs | 30 +++++--- crates/benilla-ui/src/widget/tests.rs | 60 ++++++++++++++++ 5 files changed, 171 insertions(+), 14 deletions(-) diff --git a/crates/benilla-ui/src/script/object/toplevel.rs b/crates/benilla-ui/src/script/object/toplevel.rs index 7d58fe3a..77496f60 100644 --- a/crates/benilla-ui/src/script/object/toplevel.rs +++ b/crates/benilla-ui/src/script/object/toplevel.rs @@ -6,8 +6,9 @@ //! 1. Walks up `+0x9c` to the nearest toplevel self-or-ancestor; with none it does nothing. //! 2. With `force`, resolves the frame's layout and tests whether it is overlapped: a visible frame //! of its stratum at its level or above, neither itself nor a descendant, meets its rect. -//! 3. Only then compacts the stratum's levels, hidden frames included (`0x764eb0`), and sets its -//! level to the top plus one, moving same-stratum children by the same delta (`0x76a4f0`). +//! 3. Only then compacts the stratum's levels below its high-water bound, hidden frames among +//! them (`0x764eb0`), and sets its level to the bound, moving same-stratum children by the same +//! delta (`0x76a4f0`). //! //! The stratum never changes. The overlapped bit is recomputed per raise, never stored: only the //! `force = 0` trigger (`0x764a4c`) reads a stored bit, and it is not wired, so neither is the @@ -79,7 +80,7 @@ pub(in crate::script) fn raise(model: &mut Model, frame: FrameHandle) -> bool { if !overlapped(model, t) { return true; } - // 3. The raise: compact the stratum's occupied levels, then land one above the top. + // 3. The raise: compact the occupied levels below the stratum's bound, then land on the bound. let Some(strata) = model.arena.frame(t).map(|f| f.strata) else { return true; }; diff --git a/crates/benilla-ui/src/script/tests/toplevel.rs b/crates/benilla-ui/src/script/tests/toplevel.rs index d303aa0a..cbb6fbe9 100644 --- a/crates/benilla-ui/src/script/tests/toplevel.rs +++ b/crates/benilla-ui/src/script/tests/toplevel.rs @@ -154,6 +154,74 @@ fn a_raise_is_top_occupied_level_plus_one_after_compaction() { ); } +/// `board_and_dialog` plus `Hidden`: overlapping both, hidden before its level is set to 20, and +/// never shown. The stratum's bound `bucket+0x8` grows only on a visible link (`0x764887`), so it +/// is 6 here, from `Board`. +fn board_dialog_and_hidden_twenty() -> UiScript { + let mut s = board_and_dialog(); + s.run( + r#" + Hidden = CreateFrame("Frame", "Hidden") + Hidden:Hide() + Hidden:SetFrameLevel(20) + Hidden:SetPoint("BOTTOMLEFT", 250, 250) + Hidden:SetWidth(300); Hidden:SetHeight(300) + Hidden:CreateTexture(nil, "ARTWORK"):SetTexture("Hidden.blp") + Dialog:SetToplevel(true) + "#, + ) + .unwrap(); + s.resolve(); + s +} + +/// The compaction examines only the levels below the stratum's bound (`0x764f4c`, `0x764fcf`), so +/// a hidden frame at level 20, above the visible high-water, is neither counted nor moved: the +/// raise gives 1, 20, 2, and the frame, shown later, still draws over the dialog. +#[test] +fn a_raise_leaves_a_hidden_frame_above_the_visible_high_water_where_it_is() { + let mut s = board_dialog_and_hidden_twenty(); + s.run("Dialog:Show()").unwrap(); + assert_eq!( + ( + level(&mut s, "Board"), + level(&mut s, "Hidden"), + level(&mut s, "Dialog") + ), + (1, 20, 2) + ); + s.run("Hidden:Show()").unwrap(); + assert_eq!(level(&mut s, "Hidden"), 20, "showing re-levels nothing"); + assert_eq!( + painted(&mut s), + vec!["Board.blp", "Dialog.blp", "Hidden.blp"], + "the shown frame draws over the dialog" + ); +} + +/// A visible link at a high level grows the bound (`0x764882`-`0x764887`), so the next compaction +/// reaches it: `Hidden` shown at 20 is packed with the rest, and the raise lands above it. +#[test] +fn a_frame_shown_at_a_high_level_raises_the_bound_for_the_next_raise() { + let mut s = board_dialog_and_hidden_twenty(); + s.run("Dialog:Show()").unwrap(); + s.run("Hidden:Show()").unwrap(); + s.run("Dialog:Raise()").unwrap(); + assert_eq!( + ( + level(&mut s, "Board"), + level(&mut s, "Hidden"), + level(&mut s, "Dialog") + ), + (0, 2, 3), + "{{1, 2, 20}} pack to [0, 3) and the dialog takes the bound, above Hidden" + ); + assert_eq!( + painted(&mut s), + vec!["Board.blp", "Hidden.blp", "Dialog.blp"] + ); +} + /// The trigger is the effective-visibility transition, not the `Show` call. `Holder` is hidden /// before `Dialog:Show()` so that call is no transition; otherwise `Dialog` raises there and has /// nothing left to raise over. diff --git a/crates/benilla-ui/src/widget/mod.rs b/crates/benilla-ui/src/widget/mod.rs index 588226d1..f7d368ab 100644 --- a/crates/benilla-ui/src/widget/mod.rs +++ b/crates/benilla-ui/src/widget/mod.rs @@ -254,6 +254,10 @@ pub struct WidgetArena { regions: Arena, names: HashMap, next_insertion: u32, + /// Each stratum's level bound, the reference's `bucket+0x8`: one above the highest level a + /// visible frame has linked at since the last compaction set it. It starts at 0 and only a + /// visible link grows it ([`Self::link_bound`]); nothing else lowers it but the compaction. + level_bound: [u32; Strata::ALL.len()], /// The next [`Frame::created`]. next_created: u64, /// How many times the link stamps were renumbered at the cap, so a reader holding stamps can @@ -316,6 +320,7 @@ impl WidgetArena { regions: Arena::new(), names: HashMap::new(), next_insertion: 0, + level_bound: [0; Strata::ALL.len()], next_created: 0, renumbers: 0, link_log: None, @@ -408,13 +413,26 @@ impl WidgetArena { self.renumbers += 1; } let seq = self.next_insertion; + let mut linked = None; if let Some(f) = self.frame_mut(h) { f.insertion_seq = seq; + linked = Some((f.strata, f.level)); self.next_insertion += 1; if let Some(log) = &mut self.link_log { log.push(h); } } + if let Some((strata, level)) = linked { + self.link_bound(strata, level); + } + } + + /// A visible frame linked into `strata` at `level`: the stratum's bound grows to one above it + /// (`0x764882`-`0x764887`, inside the bucket add `0x764780`, which the level setter reaches + /// only for a visible frame, `0x76a541`). A hidden frame's level never reaches it. + fn link_bound(&mut self, strata: Strata, level: u16) { + let bound = &mut self.level_bound[strata.index() as usize]; + *bound = (*bound).max(u32::from(level) + 1); } /// Start recording every link into a bucket, for the update walk (`0x765650`), which must see @@ -558,9 +576,11 @@ impl WidgetArena { | FrameKind::TabardModel ); let visible = frame.effective_visible; + let (strata, level) = (frame.strata, frame.level); let (index, generation) = self.frames.insert(frame); let handle = FrameHandle { index, generation }; if visible { + self.link_bound(strata, level); if let Some(log) = &mut self.link_log { log.push(handle); } diff --git a/crates/benilla-ui/src/widget/propagation.rs b/crates/benilla-ui/src/widget/propagation.rs index e42e5d14..31900f14 100644 --- a/crates/benilla-ui/src/widget/propagation.rs +++ b/crates/benilla-ui/src/widget/propagation.rs @@ -144,27 +144,33 @@ impl WidgetArena { } /// Level compaction (`0x764eb0`), run by `CSimpleTop::Raise` (`0x7650f0`) before it sets the - /// raised frame's level to the returned count: renumber the occupied levels of `strata` in - /// order, hidden frames included (the scan tests only strata and level, `0x764f18`/`0x764f20`). - /// Each frame of a shifted level moves through [`Self::set_frame_level`] without propagating - /// (`0x764faf`/`0x764fb6`), the levels from the bottom and each in creation order - /// (`0x764f80`–`0x764fc9`), so a moved visible frame relinks and a shifted level's visible - /// frames end in creation order. A frame whose level holds is not moved. + /// raised frame's level to the stratum's level bound (`0x7651d0`/`0x7651d3`): + /// renumber the occupied levels below the bound in order, hidden frames included (the scan + /// tests only strata and level, `0x764f18`/`0x764f20`), and set the bound to the count. Only + /// the levels below the bound are examined (`0x764f4c`) and moved (`0x764fcf`), so a frame + /// given a level above it while hidden is neither counted nor moved. Each frame of a shifted + /// level moves through [`Self::set_frame_level`] without propagating (`0x764faf`/`0x764fb6`), + /// the levels from the bottom and each in creation order (`0x764f80`-`0x764fc9`), so a moved + /// visible frame relinks and a shifted level's visible frames end in creation order. A frame + /// whose level holds is not moved. /// /// Deviation: levels pack contiguously, because the reference's resuming cursor - /// (`0x764fd3`/`0x764fd8`) leaves a hole past a wide gap ({0, 4, 6} at count 7 gives {0, 1, 3} - /// at count 4) that no draw or hit order can see. + /// (`0x764fd3`/`0x764fd8`) leaves a hole past a wide gap ({0, 4, 6} at bound 7 gives {0, 1, 3} + /// at bound 4). No draw or hit order sees the hole, but the raised frame takes the bound, so + /// `GetFrameLevel` shows one level more there than here. pub fn compact_levels(&mut self, strata: Strata) -> u16 { + let bound = self.level_bound[strata.index() as usize]; + let below = |f: &super::Frame| f.strata == strata && u32::from(f.level) < bound; let mut occupied: Vec = self .iter_frames() - .filter(|(_, f)| f.strata == strata) + .filter(|(_, f)| below(f)) .map(|(_, f)| f.level) .collect(); occupied.sort_unstable(); occupied.dedup(); let mut moves: Vec<(u16, u64, FrameHandle, u16)> = self .iter_frames() - .filter(|(_, f)| f.strata == strata) + .filter(|(_, f)| below(f)) .filter_map(|(h, f)| { let idx = occupied.binary_search(&f.level).ok()? as u16; (idx != f.level).then_some((f.level, f.created, h, idx)) @@ -174,7 +180,9 @@ impl WidgetArena { for (_, _, h, level) in moves { self.set_frame_level(h, level, false); } - occupied.len() as u16 + let count = occupied.len() as u16; + self.level_bound[strata.index() as usize] = u32::from(count); + count } // ── Toplevel ───────────────────────────────────────────────────────────────────────────────── diff --git a/crates/benilla-ui/src/widget/tests.rs b/crates/benilla-ui/src/widget/tests.rs index 37511cf0..39355551 100644 --- a/crates/benilla-ui/src/widget/tests.rs +++ b/crates/benilla-ui/src/widget/tests.rs @@ -168,6 +168,66 @@ fn compaction_relinks_the_shifted_visible_frames_in_creation_order() { assert_eq!(seq(&a, hidden), hidden_seq, "a hidden frame is in no list"); } +/// Compaction (`0x764eb0`) reads the stratum's bound `bucket+0x8` (`0x764ec2`) and examines only +/// the levels below it (`0x764f4c`, `0x764fcf`). The bound is the highest visible link plus one, +/// so a frame hidden above it keeps its level, and one hidden below it is packed with the rest. +#[test] +fn compaction_examines_only_the_levels_below_the_bound() { + let mut a = arena(); + let low = a.create(FrameKind::Frame, None, None); + let below = a.create(FrameKind::Frame, None, None); + let above = a.create(FrameKind::Frame, None, None); + let panel = a.create(FrameKind::Frame, None, None); + a.set_shown(below, false); + a.set_shown(above, false); + a.set_frame_level(below, 3, false); + a.set_frame_level(above, 6, false); + a.set_frame_level(panel, 5, false); // the only high link: the bound is 6 + + assert_eq!( + a.compact_levels(Strata::Medium), + 3, + "{{0, 3, 5}} pack to [0, 3); level 6 is not below the bound" + ); + let levels = [low, below, panel, above].map(|h| a.frame(h).unwrap().level); + assert_eq!(levels, [0, 1, 2, 6]); + + // The compaction set the bound to its count: the stratum's next raise lands at 3. + assert_eq!(a.compact_levels(Strata::Medium), 3); + assert_eq!(a.frame(above).unwrap().level, 6); +} + +/// The bound grows on a visible link only (`0x764780`): a show, or a level or stratum change of a +/// visible frame. A hidden frame's level never reaches it. +#[test] +fn the_bound_grows_on_a_visible_link_only() { + let mut a = arena(); + let f = a.create(FrameKind::Frame, None, None); + let g = a.create(FrameKind::Frame, None, None); + a.set_shown(f, false); + a.set_frame_level(f, 9, false); + assert_eq!(a.compact_levels(Strata::Medium), 1, "only g's level 0"); + assert_eq!(a.frame(f).unwrap().level, 9); + + a.set_shown(f, true); + assert_eq!( + a.compact_levels(Strata::Medium), + 2, + "f linked at 9: {{0, 9}}" + ); + assert_eq!(a.frame(f).unwrap().level, 1); + + a.set_frame_level(g, 7, false); + a.set_frame_strata(f, Strata::Dialog); + assert_eq!( + a.compact_levels(Strata::Dialog), + 1, + "f's relink grows the new stratum's bound; the old one is unchanged" + ); + assert_eq!(a.compact_levels(Strata::Medium), 1); + assert_eq!(a.frame(g).unwrap().level, 0); +} + // ── Scale ──────────────────────────────────────────────────────────────────────────────────── #[test]