From cae9bf1d44a26c12d63d136928ef393dd3844c1a Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 14 Aug 2026 19:27:04 +0700 Subject: [PATCH] a width read answers in its own tick, and Trade stays /2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - GetStringWidth answers the moment the text is set — the client's width law is a per-glyph table, no shaping pass — so Bagnon's character dropdown stops collapsing to 40 pixels - a chat channel keeps its number when another channel leaves: Trade stays /2 through a teleport - the login splash, the zone music and the minimap label wait for the world under the player instead of announcing whatever tile happened to be loaded - ending a session empties the chat window and the input box's history Co-Authored-By: Claude Fable 5 --- .../benilla-app/assets/ui/MerchantFrame.xml | 15 +- crates/benilla-app/assets/ui/MoneyFrame.xml | 39 +- crates/benilla-app/src/chat_bubble.rs | 4 +- crates/benilla-app/src/ui_chat.rs | 56 ++- crates/benilla-app/src/ui_chat/channels.rs | 2 +- crates/benilla-app/src/ui_chat/edit.rs | 76 +++- crates/benilla-app/src/ui_chat/feed.rs | 30 +- crates/benilla-app/src/ui_chat/tests.rs | 109 +++++- .../src/ui_script/bagnon_render_tests.rs | 71 +++- .../benilla-app/src/ui_script/extract/mod.rs | 90 ++--- .../benilla-app/src/ui_script/extract/text.rs | 2 +- .../src/ui_script/merchant_tests.rs | 4 +- crates/benilla-app/src/ui_script/mod.rs | 18 + .../src/ui_script/money_frame_tests.rs | 2 +- crates/benilla-app/src/ui_text/atlas.rs | 339 ++++++++++++++++-- crates/benilla-app/src/ui_text/layout/mod.rs | 279 +++++--------- crates/benilla-app/src/ui_text/measurer.rs | 87 +++++ crates/benilla-app/src/ui_text/mod.rs | 11 +- crates/benilla-ui/src/script/channel.rs | 27 +- crates/benilla-ui/src/script/layout.rs | 87 +++-- crates/benilla-ui/src/script/measure.rs | 148 ++++++++ crates/benilla-ui/src/script/mod.rs | 86 +---- crates/benilla-ui/src/script/model.rs | 12 +- crates/benilla-ui/src/script/region.rs | 3 + crates/benilla-ui/src/script/region/text.rs | 5 + crates/benilla-ui/src/script/tests/channel.rs | 2 +- crates/benilla-ui/src/script/tests/measure.rs | 150 ++++++++ crates/benilla-world/src/terrain_stream.rs | 14 + .../src/terrain_stream/queries.rs | 7 + 29 files changed, 1282 insertions(+), 493 deletions(-) create mode 100644 crates/benilla-app/src/ui_text/measurer.rs create mode 100644 crates/benilla-ui/src/script/measure.rs diff --git a/crates/benilla-app/assets/ui/MerchantFrame.xml b/crates/benilla-app/assets/ui/MerchantFrame.xml index 69bfdf18..c89c9fa8 100644 --- a/crates/benilla-app/assets/ui/MerchantFrame.xml +++ b/crates/benilla-app/assets/ui/MerchantFrame.xml @@ -45,7 +45,7 @@ -- visible denominations into slots 1.. in DISPLAY order (highestFirst=true for the -- left-aligned chains) and — the real shrink, MoneyFrame.lua l.202/269 — sizes each slot to -- exactly its number + its 13px coin (`SetWidth(GetTextWidth() + iconWidth)` there; the - -- app's BENILLA_DIGIT_W advance feed here, summed by BenillaNumberWidth). The static ±4px + -- same read here, off the number FontString the slot already owns). The static ±4px -- chain-anchor offsets are the real MONEY_BUTTON_SPACING gap, so a shrunk group packs tight, -- left-flush in the ltr chains and right-flush in the rtl ones. BenillaMoney_Clear hides -- all three slots (the real collapse hides, see the helper). @@ -60,11 +60,6 @@ { 0.5, 0.75 }, -- [3] copper } - -- (BenillaNumberWidth — the digit-sum this kit sizes its slots by — is an ENGINE binding, - -- published beside the BENILLA_DIGIT_W feed it reads. It was Lua in this file until - -- MoneyFrame.xml's ShowCoin became its second caller and no shipped file turned out to be a - -- dependency of both: whichever one owned it, some fixture met it as nil.) - -- Unused slots HIDE (the real collapse — MoneyFrame.lua l.230/247 goldButton:Hide()), never -- SetWidth(0): a 0 width means "derive from opposing anchors" to the layout (the client's -- own G+0x50 convention), not "zero px". Fills are contiguous from Coin1, so a chain never @@ -93,16 +88,18 @@ slot = slot + 1 -- The real shrink: this slot is exactly its number + its 13px coin, and the -- XML anchor chain re-flows off the new width. - local numW = BenillaNumberWidth(vals[d]) + -- Text first, then measure it: the engine answers a width read inside the + -- tick that set the text, so the number IS this number and not the last one. + local num = getglobal(prefix .. "Coin" .. slot .. "Num") + num:SetText(vals[d]) + local numW = num:GetStringWidth() local slotFrame = getglobal(prefix .. "Coin" .. slot) slotFrame:SetWidth(numW + 13) slotFrame:Show() local icon = getglobal(prefix .. "Coin" .. slot .. "Icon") icon:SetTexture("Interface\\MoneyFrame\\UI-MoneyIcons") icon:SetTexCoord(BENILLA_COIN_TC[d][1], BENILLA_COIN_TC[d][2], 0, 1) - local num = getglobal(prefix .. "Coin" .. slot .. "Num") num:SetWidth(numW) - num:SetText(vals[d]) end end for i = slot + 1, 3 do diff --git a/crates/benilla-app/assets/ui/MoneyFrame.xml b/crates/benilla-app/assets/ui/MoneyFrame.xml index e417fccd..eeebadc3 100644 --- a/crates/benilla-app/assets/ui/MoneyFrame.xml +++ b/crates/benilla-app/assets/ui/MoneyFrame.xml @@ -50,12 +50,9 @@ · `message()` (ref l.145, l.173, l.181) — the one-line StaticPopup shim, which belongs to BasicControls.xml, a file family we have not built. Guarded, so a misuse of the API stays silent instead of erroring on the diagnostic. - · `Button:GetTextWidth()` (ref l.202 etc.) — BUILT (it forwards to the label FontString's - `GetStringWidth`; it arrived with Bagnon's character list, which sizes itself by it). We - still do not call it here, and the reason is timing, not absence: both it and - `GetStringWidth` are served from the measure round-trip, which lands a frame after the - tick that set the text, so either one reads 0 on the update that needs it. ShowCoin sums - BENILLA_DIGIT_W instead — see there. + · `Button:GetTextWidth()` (ref l.202 etc.) — BUILT, and called here, exactly as the + specification calls it. It forwards to the label FontString's `GetStringWidth`, which the + engine now answers inside the tick that asked (`script::measure`). · The four `Pickup*`/`Add*` mail verbs the pickup closures name are not in 1.12's `_G` either (they are absent from reference/1.12-globals.tsv) — so those closures are as unreachable here as they are there. They exist because the table's shape is the contract. @@ -279,28 +276,20 @@ -- Lay one denomination out: its digits, and the width those digits plus the coin icon need. -- - -- The specification measures with `Button:GetTextWidth()` (ref l.202 and kin), and the real - -- client answers that SYNCHRONOUSLY — its font engine is inline, so the coin is the right - -- width on the frame that set its text. + -- `Button:GetTextWidth()` right after `SetText`, which is what the specification writes + -- (ref l.202 and kin) and what the real client answers synchronously off its inline font + -- engine, so the coin is the right width on the frame that set its text. -- - -- Ours cannot ask a FontString: `GetStringWidth` is served from the measure round-trip, - -- which lands at extract, a frame AFTER the tick that called SetText. Reading it here - -- returned 0 on every first open and left all three coins at exactly one icon width, with - -- their digits overlapping — the director's "gold numbers are all cramped up when I first - -- open the bags; close and open again and it looks good", the reopen being the cached - -- measure from the first. - -- - -- So this sums BENILLA_DIGIT_W instead, via MerchantFrame.xml's BenillaNumberWidth: the - -- app measures NumberFontNormal's ten digits once per atlas scale and pushes them as data - -- (`UiScript::set_digit_advances`), which makes the answer available IN this tick. That - -- feed was built for this frame — its doc names "the real SmallMoneyFrame calls - -- GetTextWidth mid-update, MoneyFrame.lua l.202" — and only the merchant price ever used - -- it. A number is the one string whose width is a pure sum of per-glyph advances (no - -- kerning enters the client's step law), which is why the table can be exact here and - -- cannot generalise to arbitrary text. + -- This read used to return 0 — our measure landed at extract, a frame after the tick — and + -- left all three coins at exactly one bare icon width with their digits overlapping: the + -- director's "gold numbers are all cramped up when I first open the bags; close and open + -- again and it looks good", the reopen being the first open's cached measure. It was served + -- for a while by a digits-only advance table, since a number's width is a pure sum of + -- per-glyph advances. The engine answers the general question now (`script::measure`), so + -- this asks it the way the specification does. local function ShowCoin(coin, coinName, amount, iconWidth) coin:SetText(amount) - coin:SetWidth(BenillaNumberWidth(amount) + iconWidth) + coin:SetWidth(coin:GetTextWidth() + iconWidth) coin:Show() end diff --git a/crates/benilla-app/src/chat_bubble.rs b/crates/benilla-app/src/chat_bubble.rs index b9e50553..70da8e50 100644 --- a/crates/benilla-app/src/chat_bubble.rs +++ b/crates/benilla-app/src/chat_bubble.rs @@ -467,13 +467,13 @@ fn draw_bubble( // else max(measured, 2·border-unit). All in shaped space, rescaled after. let cap_s = gx_px(WRAP_W, basis) / s; let floor_s = (2.0 * border) / s; - let (line_w, _) = measure_text(atlas, &b.text, None, spec); + let (line_w, _) = measure_text(&atlas.metrics, &b.text, None, spec); let box_w_s = if line_w > cap_s { cap_s } else { line_w.max(floor_s) }; - let (_, box_h_s) = measure_text(atlas, &b.text, Some(box_w_s), spec); + let (_, box_h_s) = measure_text(&atlas.metrics, &b.text, Some(box_w_s), spec); let (text_w, text_h) = ((box_w_s * s).ceil(), (box_h_s * s).ceil()); // The auto-fit body: the frame hugs the text layout ± the flat margin; BOTTOM-center on diff --git a/crates/benilla-app/src/ui_chat.rs b/crates/benilla-app/src/ui_chat.rs index 30a78c06..bf6cd8dc 100644 --- a/crates/benilla-app/src/ui_chat.rs +++ b/crates/benilla-app/src/ui_chat.rs @@ -112,7 +112,61 @@ impl Plugin for UiChatPlugin { ) .add_systems( OnExit(crate::char_select::ClientState::InWorld), - channels::end_session_channels, + (channels::end_session_channels, end_session_chat), ); } } + +/// **The chat module's session end** — what the reference gets for free by destroying its Lua +/// state, and we have to do by hand until that teardown lands (1288). +/// +/// `shutdown_ui_state`'s own doc carves the reference's logout tail: `PLAYER_LEAVING_WORLD` → +/// `PLAYER_LOGOUT` → the saved files → **destroy the Lua state**. That last step is the one +/// [`crate::ui_script::IngameUiLoaded`] exists to stand in for; while it does, every window in the +/// VM keeps its contents across a character switch. The director saw it as the previous +/// character's `Joined Channel:` lines still sitting under the new character's — chat scrollback +/// is simply the most visible tenant of a VM that should have been rebuilt. +/// +/// So this clears what a fresh VM (and a fresh `ui_chat`) would have: both windows' lines, the +/// edit box's whole cross-open memory — sticky type, the `lastTell` ring, `toldTarget` — and the +/// undrained feed, whose queued lines were addressed to a character that is gone. It is the same +/// shape, and the same reasoning, as [`crate::ui_aura`]'s `end_session_aura_state` (0900). +/// +/// **Not on a reconnect**, deliberately, which is why this is not in [`channels`]'s disconnect +/// twin: channel membership is *server* state and dies with the server's `Player` object however +/// the socket ended, but a seamless same-character reconnect (0065) is benilla's own affordance — +/// the reference has no such thing, it goes to the login screen — and inside it the window keeping +/// its scrollback is the whole point. +fn end_session_chat( + script: Option>, + mut windows: ResMut, + mut edit: ResMut, + mut log: ResMut, +) { + end_chat_session( + script.map(NonSendMut::into_inner), + &mut windows, + &mut edit, + &mut log, + ); +} + +/// [`end_session_chat`]'s body, callable without a `World` — the clear is the law, the system is +/// the wiring. +pub(crate) fn end_chat_session( + script: Option<&mut benilla_ui::script::UiScript>, + windows: &mut frames::ChatWindows, + edit: &mut edit::ChatEditState, + log: &mut ChatLog, +) { + *log = ChatLog::default(); + windows.tell_alert_left = 0.0; + // The channel target/number are [`channels::end_session_channels`]'s (1284); everything else + // the box remembers across an open is this session's too. + *edit = edit::ChatEditState::default(); + if let Some(script) = script { + for frame in ["ChatFrame1", "ChatFrame2"] { + crate::ui_script::run_or_warn(script, &format!("{frame}:Clear()")); + } + } +} diff --git a/crates/benilla-app/src/ui_chat/channels.rs b/crates/benilla-app/src/ui_chat/channels.rs index b66390ce..1a25342b 100644 --- a/crates/benilla-app/src/ui_chat/channels.rs +++ b/crates/benilla-app/src/ui_chat/channels.rs @@ -461,7 +461,7 @@ mod tests { // A live session: two zone channels asked for, both confirmed, `/2` targeted. walk.held = vec!["General - Tanaris".into(), "LocalDefense - Tanaris".into()]; walk.at = Some(("Tanaris".into(), false)); - channels.joined = walk.held.clone(); + channels.joined = walk.held.iter().cloned().map(Some).collect(); edit.channel_target = "LocalDefense - Tanaris".into(); edit.channel_number = 2; diff --git a/crates/benilla-app/src/ui_chat/edit.rs b/crates/benilla-app/src/ui_chat/edit.rs index cadbfd9c..263563fa 100644 --- a/crates/benilla-app/src/ui_chat/edit.rs +++ b/crates/benilla-app/src/ui_chat/edit.rs @@ -155,14 +155,33 @@ impl SendType { } } -/// The channels this session has joined, in join order — the CLIENT-side number law -/// (`GetChannelName(n)`): `/1` is the first joined, `/2` the second; the numbered display form -/// ("1. General - Elwynn Forest") and the `[N. Name]` prefixes all derive from this order. -/// Fed by YOU_JOINED / YOU_LEFT notices ([`super::feed`]); the zone AUTO-join walk that fills it -/// at login is [`super::channels`]. +/// How many channels the client can hold at once — its allocator refuses the eleventh +/// (`0x49b9c0: cmp ecx,0xa`), and the ten boot-seeded `CHANNEL1`…`CHANNEL10` color rows are the +/// same ten (wow-re `chat-color-table.md`). +pub(crate) const MAX_CHANNELS: usize = 10; + +/// The channels this session has joined — the CLIENT-side number law (`GetChannelName(n)`): `/1` +/// is slot 1, `/2` slot 2; the numbered display form ("1. General - Elwynn Forest") and the +/// `[N. Name]` prefixes all derive from it. Fed by YOU_JOINED / YOU_LEFT notices +/// ([`super::feed`]); the zone AUTO-join walk that fills it at login is [`super::channels`]. +/// +/// **It is a SLOT ARRAY, and leaving punches a hole rather than closing one** (1286). The client's +/// records live in a fixed array at `[0xb4fe04]`, stride `0xa0`, with the entry's own **number** +/// at `+0x00`; the allocator `0x49b980` scans for an entry whose number is `0` and *reuses* it +/// (`0x49b9b0`: `cmp dword [edx],0` / `jz`), only growing when none is free and the count is under +/// **ten** (`0x49b9c0: cmp ecx,0xa`), and the leave path `0x49bbd0` clears that number in place +/// (`0x49bc1b: mov dword [eax+edx],0`) without shrinking the count. Lookup by index then demands +/// the entry's number equal the index asked for (`0x49bf30: cmp esi,ecx / jnz`), so a hole answers +/// "not joined" while every channel above it keeps its number. +/// +/// A `Vec` cannot express that: `retain` closed the hole and renumbered everything above +/// it, so walking out of a zone renamed *other* channels — the director saw General and +/// LocalDefense trade numbers on one zone change, and a `/2` typed after that went somewhere else. #[derive(Resource, Default)] pub(crate) struct ChannelState { - pub joined: Vec, + /// Slot `i` is channel number `i + 1`; `None` is a freed slot, kept so the numbers above it + /// do not move. Never longer than [`MAX_CHANNELS`]. + pub joined: Vec>, /// `ChatChannels.dbc`, loaded once at Startup ([`super::channels::load_chat_channels`]). /// /// It lives here because both of its consumers are this type's own business: composing the @@ -178,10 +197,47 @@ impl ChannelState { pub(crate) fn number_of(&self, name: &str) -> Option { self.joined .iter() - .position(|c| c.eq_ignore_ascii_case(name)) + .position(|c| c.as_deref().is_some_and(|c| c.eq_ignore_ascii_case(name))) .map(|i| i as u32 + 1) } + /// The channel occupying slot `number` (1-based), if any. + pub(crate) fn name_of(&self, number: u32) -> Option<&str> { + self.joined + .get(usize::try_from(number.checked_sub(1)?).ok()?)? + .as_deref() + } + + /// Give `name` a slot: **the first free one**, else a new one while under [`MAX_CHANNELS`] — + /// the reference's allocator `0x49b980` (see [`ChannelState`]). Already-joined answers its own + /// number rather than taking a second slot. `None` = all ten are taken. + /// + /// The reference also prints a chat error when full (`0x49b9c5: push 0x199` → `0x496720`); we + /// decline the join and warn instead — one line of feedback we cannot quote without the + /// error-string table this build indexes by id, and the structural half is what matters. + pub(crate) fn claim_slot(&mut self, name: &str) -> Option { + if let Some(n) = self.number_of(name) { + return Some(n); + } + if let Some(i) = self.joined.iter().position(Option::is_none) { + self.joined[i] = Some(name.to_string()); + return Some(i as u32 + 1); + } + if self.joined.len() >= MAX_CHANNELS { + return None; + } + self.joined.push(Some(name.to_string())); + Some(self.joined.len() as u32) + } + + /// Free the slot holding `name` — **cleared in place** (`0x49bbd0`), so every other channel + /// keeps its number. Answers the number that just went empty. + pub(crate) fn free_slot(&mut self, name: &str) -> Option { + let n = self.number_of(name)?; + self.joined[n as usize - 1] = None; + Some(n) + } + /// Fill an event's four channel slots (arg4, arg7, arg8, arg9) in place. /// /// **They are one record, not four fields.** In the reference all four are read off the @@ -419,10 +475,10 @@ pub(super) fn channel_switch( args: &str, ) -> Option<(TypeSwitch, String)> { if let Ok(n) = cmd_lower.parse::() { - let name = channels.joined.get(n.checked_sub(1)? as usize)?; + let name = channels.name_of(n)?; return Some(( TypeSwitch::Channel { - name: name.clone(), + name: name.to_string(), number: n, }, args.to_string(), @@ -438,7 +494,7 @@ pub(super) fn channel_switch( } else { channels.number_of(chan)? }; - let name = channels.joined.get((number - 1) as usize)?.clone(); + let name = channels.name_of(number)?.to_string(); return Some((TypeSwitch::Channel { name, number }, remainder.to_string())); } None diff --git a/crates/benilla-app/src/ui_chat/feed.rs b/crates/benilla-app/src/ui_chat/feed.rs index bd43f0be..f65e519c 100644 --- a/crates/benilla-app/src/ui_chat/feed.rs +++ b/crates/benilla-app/src/ui_chat/feed.rs @@ -494,13 +494,23 @@ pub(super) fn deliver( .notice_byte() .filter(|_| event.kind == Some(ChatEventKind::ChannelNotice)); if notice == Some(channel_notice::YOU_JOINED) && channels.number_of(&event.channel).is_none() { - channels.joined.push(event.channel.clone()); - debug!( - "chat: server confirms channel {:?} joined (slot {})", - event.channel, - channels.joined.len() - ); - script.set_joined_channels(channels.joined.clone()); + match channels.claim_slot(&event.channel) { + Some(slot) => { + debug!( + "chat: server confirms channel {:?} joined (slot {slot})", + event.channel + ); + script.set_joined_channels(channels.joined.clone()); + } + // The reference's own ceiling, reached: ten slots, all taken. It answers with a chat + // error and no record, so the channel stays unnumbered here too. + None => warn!( + "chat: server confirms channel {:?} joined but all {} slots are taken — it has no \ + number, so /N cannot reach it", + event.channel, + super::edit::MAX_CHANNELS + ), + } } // The wire name, kept before `stamp_channel` decorates arg4 with the slot number. let leaving = (notice == Some(channel_notice::YOU_LEFT)).then(|| event.channel.clone()); @@ -508,8 +518,10 @@ pub(super) fn deliver( channels.stamp_channel(event); route(script, windows, event); if let Some(name) = leaving { - debug!("chat: server confirms channel {name:?} left"); - channels.joined.retain(|c| !c.eq_ignore_ascii_case(&name)); + // Cleared in place, never compacted: slot 2 going empty must not make slot 3 into 2 + // ([`ChannelState`], 1286). + let freed = channels.free_slot(&name); + debug!("chat: server confirms channel {name:?} left (slot {freed:?} now free)"); script.set_joined_channels(channels.joined.clone()); } } diff --git a/crates/benilla-app/src/ui_chat/tests.rs b/crates/benilla-app/src/ui_chat/tests.rs index 94567f4d..41f6b68a 100644 --- a/crates/benilla-app/src/ui_chat/tests.rs +++ b/crates/benilla-app/src/ui_chat/tests.rs @@ -614,8 +614,8 @@ fn a_channel_we_are_not_in_fires_the_bare_name_and_zeroes() { #[test] fn stamping_a_channel_splits_the_display_form_from_the_base_name() { let mut channels = super::edit::ChannelState::default(); - channels.joined.push("World".into()); - channels.joined.push("General - Elwynn Forest".into()); + channels.claim_slot("World"); + channels.claim_slot("General - Elwynn Forest"); let mut e = ev(K::Channel, "wts boar livers", "Bob"); e.channel = "General - Elwynn Forest".into(); @@ -651,7 +651,7 @@ fn a_channel_notice_renders_in_the_channels_color_not_the_notice_row() { let mut s = chat_vm(); let mut windows = super::frames::ChatWindows::default(); let mut channels = super::edit::ChannelState::default(); - channels.joined.push("General - Elwynn Forest".into()); + channels.claim_slot("General - Elwynn Forest"); let mut e = ChatEvent::text_only(K::ChannelNotice, String::new()); e.channel = "General - Elwynn Forest".into(); @@ -730,8 +730,8 @@ fn a_leave_notice_keeps_its_number_because_the_record_dies_after_the_line() { let mut s = chat_vm(); let mut windows = super::frames::ChatWindows::default(); let mut channels = super::edit::ChannelState::default(); - channels.joined.push("World".into()); - channels.joined.push("General - Elwynn Forest".into()); + channels.claim_slot("World"); + channels.claim_slot("General - Elwynn Forest"); let mut e = ChatEvent::text_only(K::ChannelNotice, String::new()); e.channel = "General - Elwynn Forest".into(); @@ -749,11 +749,106 @@ fn a_leave_notice_keeps_its_number_because_the_record_dies_after_the_line() { ); assert_eq!( channels.joined, - ["World"], - "and only THEN is the record gone" + [Some("World".to_string()), None], + "and only THEN is the record gone — as a HOLE at slot 2, not a shortened list (1286)" ); } +/// **A channel that leaves does not renumber the ones that stay** (1286). +/// +/// The director's teleport tour: `Left Channel: [1. General - Teldrassil]` / +/// `Joined Channel: [2. General - The Barrens]` / `Left Channel: [1. LocalDefense - Teldrassil]`, +/// with Trade shuffling 1 → 2 → 3 across the same few seconds — every number in the window moving +/// because the list closed each hole. The reference frees the slot in place and refills the first +/// free one, so a zone hop *renames* a channel and leaves its number alone. +#[test] +fn a_freed_slot_is_reused_and_the_others_keep_their_numbers() { + let mut c = super::edit::ChannelState::default(); + assert_eq!(c.claim_slot("General - Teldrassil"), Some(1)); + assert_eq!(c.claim_slot("Trade - City"), Some(2)); + assert_eq!(c.claim_slot("LocalDefense - Teldrassil"), Some(3)); + + // Cross a zone border: General and LocalDefense rename, Trade is untouched. + assert_eq!(c.free_slot("General - Teldrassil"), Some(1)); + assert_eq!( + c.claim_slot("General - The Barrens"), + Some(1), + "the freed slot is reused — the client scans for a zeroed record before growing" + ); + assert_eq!(c.free_slot("LocalDefense - Teldrassil"), Some(3)); + assert_eq!(c.claim_slot("LocalDefense - The Barrens"), Some(3)); + assert_eq!( + c.number_of("Trade - City"), + Some(2), + "Trade never moved: /2 still reaches it, which is the whole complaint" + ); + + // Leaving the city drops Trade; the hole it leaves is what the next join takes. + assert_eq!(c.free_slot("Trade - City"), Some(2)); + assert_eq!(c.name_of(2), None, "a hole answers 'not joined'"); + assert_eq!(c.number_of("General - The Barrens"), Some(1), "still 1"); + assert_eq!( + c.claim_slot("Trade - City"), + Some(2), + "and back into slot 2" + ); + + // The ceiling is the reference's ten (`0x49b9c0: cmp ecx,0xa`), counted in SLOTS. + for i in 4..=super::edit::MAX_CHANNELS { + assert_eq!(c.claim_slot(&format!("Custom{i}")), Some(i as u32)); + } + assert_eq!(c.claim_slot("OneTooMany"), None); + assert_eq!(c.free_slot("Custom7"), Some(7)); + assert_eq!( + c.claim_slot("OneTooMany"), + Some(7), + "full means no free slot, not a permanent ceiling" + ); +} + +/// **The next character does not inherit this one's chat window** (1288). +/// +/// The reference ends a session by destroying its Lua state, so the window that comes back is +/// empty. We keep the VM (`ui_script::IngameUiLoaded` is the latch standing in for that teardown), +/// so the director saw the previous character's `Joined Channel:` lines still sitting under the +/// new character's. Everything the module remembers across a box open goes with the lines. +#[test] +fn a_session_end_empties_the_window_and_the_boxs_memory() { + let mut s = chat_vm(); + let mut windows = super::frames::ChatWindows::default(); + let mut edit = super::edit::ChatEditState::default(); + let mut log = super::ChatLog::default(); + + super::frames::route(&mut s, &mut windows, &ev(K::Say, "hi there", "Bob")); + super::frames::route(&mut s, &mut windows, &ev(K::Whisper, "psst", "Ann")); + edit.remember_tell("Ann"); + edit.sticky = super::edit::SendType::Guild; + log.push_event(ChatEvent::text_only(K::System, "queued".into())); + assert_eq!( + lines_in_window(&s), + 2, + "the window has this session's lines" + ); + + super::end_chat_session(Some(&mut s), &mut windows, &mut edit, &mut log); + + assert_eq!( + lines_in_window(&s), + 0, + "and the next character starts clean" + ); + assert!( + edit.last_tell.is_empty(), + "the tell ring was that character's" + ); + assert_eq!( + edit.sticky, + super::edit::SendType::Say, + "sticky back to SAY" + ); + assert_eq!(windows.tell_alert_left, 0.0, "the chime throttle resets"); +} + /// Every notice byte the composer renders has a token to fire, and vice versa — the two tables are /// the same set by assertion rather than by good intentions (they are read off the same /// `CHAT__NOTICE` GlobalStrings keys). diff --git a/crates/benilla-app/src/ui_script/bagnon_render_tests.rs b/crates/benilla-app/src/ui_script/bagnon_render_tests.rs index 1edef647..fb6a34c7 100644 --- a/crates/benilla-app/src/ui_script/bagnon_render_tests.rs +++ b/crates/benilla-app/src/ui_script/bagnon_render_tests.rs @@ -1139,7 +1139,7 @@ fn a_money_frame_is_the_right_width_on_the_first_open() { s.set_money(123_456_789); // The app feeds this once per atlas scale, before any window opens. A flat 8px advance makes // the expected widths exact arithmetic rather than a font-dependent number. - s.set_digit_advances(&[8.0; 10]); + s.set_text_measurer(Box::new(super::FixedWidthFont(8.0))); s.run("ToggleBackpack()").expect("ToggleBackpack"); for _ in 0..3 { @@ -1238,3 +1238,72 @@ fn a_fontheight_with_no_font_attr_beside_it_is_never_read() { attribute the client never reaches" ); } + +/// A stand-in font engine for the fixture: every character `PER_CHAR` wide, one line tall. The +/// *numbers* the real engine produces are pinned by `ui_text::atlas::metrics_tests` against the +/// client's own fonts; what these tests are about is that an answer arrives **in the tick that +/// asked**, which is exactly what a fixture with head-arithmetic widths shows clearly. +struct BlockFont; + +const PER_CHAR: f32 = 7.0; + +impl benilla_ui::script::TextMeasure for BlockFont { + fn measure(&mut self, req: &benilla_ui::script::MeasureRequest) -> (f32, f32, f32) { + let w = req.text.chars().count() as f32 * PER_CHAR; + (w, 12.0, w) + } +} + +/// **Bug 4 — the director's narrow dropdown.** `Bagnon_Forever/database/ui.lua:56-61` sizes its +/// character list by setting each row's text and reading the row back in the same tick: +/// +/// ```lua +/// button:SetText(player) +/// if button:GetTextWidth() + 40 > width then width = button:GetTextWidth() + 40 end +/// ``` +/// +/// With `GetTextWidth` served only by the extract-time round-trip that read **0** for every row, so +/// the frame took the `0 + 40` floor and drew 40px wide while seven full-width character names hung +/// out of its right edge — the director's screenshot. The list must now be as wide as its widest +/// name plus the addon's own 40px of checkbox and padding. +#[test] +fn the_character_dropdown_is_as_wide_as_the_names_in_it() { + let root = corpus_or_skip!(); + let mut s = open_bagnon(&root); + s.set_text_measurer(Box::new(BlockFont)); + // Three saved characters on this realm, the longest 10 letters — Bagnon_Forever's own store. + s.run( + "local realm = GetRealmName() \ + BagnonForeverData[realm][\"Onewarrior\"] = { g = 100 } \ + BagnonForeverData[realm][\"Onerogue\"] = { g = 200 }", + ) + .expect("seed the character store"); + s.run("BagnonDBUI_ShowCharacterList(Bagnon)") + .expect("BagnonDBUI_ShowCharacterList"); + for _ in 0..3 { + s.tick(0.1); + } + s.resolve(); + + let widest = "Onewarrior".len() as f32 * PER_CHAR + 40.0; + assert_eq!( + s.eval::("return BagnonDBUICharacterList:GetWidth()") + .unwrap(), + widest, + "the list sizes itself from its rows' own text width, read in the tick that set it" + ); + // …and the rows are really inside it: the addon's 6px left inset plus a name, not a 140px + // template button overhanging a 40px box. + let (list_r, row_r): (f32, f32) = s + .eval::>( + "local l = BagnonDBUICharacterList \ + return { l:GetLeft() + l:GetWidth(), BagnonDBUICharacterList1:GetLeft() + \ + BagnonDBUICharacterList1:GetTextWidth() + 24 }", + ) + .map(|v| (v[0], v[1])) + .unwrap(); + assert!( + row_r <= list_r, + "a row's text must end inside the list: text right edge {row_r} > list right edge {list_r}" + ); +} diff --git a/crates/benilla-app/src/ui_script/extract/mod.rs b/crates/benilla-app/src/ui_script/extract/mod.rs index dae3aef7..5cf0986d 100644 --- a/crates/benilla-app/src/ui_script/extract/mod.rs +++ b/crates/benilla-app/src/ui_script/extract/mod.rs @@ -74,9 +74,6 @@ pub(super) fn drive_script( // This frame's `` widget slot, parked for `minimap::emit_minimap` (the UiQuadAppend // producer that fills the hole with tile/arrow quads — decision 0203 phase 1). mut minimap_widget: ResMut, - // Whether the digit-advance feed has run (once per raster environment: on the first frame - // the atlas exists, and again after every seam-scale change — `last_seam` below re-arms it). - mut digits_fed: Local, // The seam scale the engine's text-metric caches were answered under. When `s` moves (window // resize / fullscreen toggle / uiScale change), every cached measure is stale — see the // invalidation below. @@ -116,13 +113,30 @@ pub(super) fn drive_script( // percent — enough that a boot-size measure fails the fullscreen fit test and the ellipsis // eats fitting text: the director's "Contr..." rows, reproduced by `WOW_RESIZE`). Declare the // staleness at the seam and let every round-trip re-answer under the new `s`. - if *last_seam != s { + let seam_moved = *last_seam != s; + if seam_moved { if *last_seam != 0.0 { script.invalidate_text_measures(); - *digits_fed = false; } *last_seam = s; } + // …and re-seat the VM's own font engine at the same edge, for the same reason: an + // [`crate::ui_text::AtlasMeasurer`] answers only for the seam it was built under. This is what + // makes a `SetText` → `GetStringWidth` pair *inside one Lua update* return a real number + // instead of 0 — the reference answers that getter inline (`0x79e510` → `0x772890`), and the + // corpus writes it that way (`Bagnon_Forever/database/ui.lua:58-59`). + // + // Seated BEFORE the tick below, so the first update that runs already has it, and rebuilt only + // on the seam edge or the frame the atlas first exists — an `Arc` clone and an `f32`, never a + // per-frame cost. + if let Some(atlas) = font_atlas.as_deref() { + if seam_moved || !script.has_text_measurer() { + script.set_text_measurer(Box::new(crate::ui_text::AtlasMeasurer::new( + atlas.metrics(), + s, + ))); + } + } // Phase spans (visible under `bevy/trace_chrome`): this system is the biggest flat CPU cost // on an idle frame, and the ledger can only rank what has a name — tick (Lua OnUpdate), // resolve (layout), measure (the text round-trips), extract (tree walk + rasterize), diff. @@ -197,37 +211,6 @@ pub(super) fn drive_script( } let us_resolve = lap(); let measure_span = bevy::log::info_span!("ui_script: measure").entered(); - // The digit-advance feed (the synchronous half of the money layout's metrics): measure - // NumberFontNormal's '0'..'9' once per atlas scale and push them as `BENILLA_DIGIT_W`, so - // `BenillaMoney_Set` can size its coin slots to their numbers *inside* one update — the real - // SmallMoneyFrame's GetTextWidth-mid-update shrink (MoneyFrame.lua l.202-269), which the - // frame-late GetStringWidth round-trip below can't serve. The font pairing comes from the - // registered font object (Fonts.xml stays the single source of truth). - if let Some(atlas) = font_atlas.as_deref_mut() { - if !*digits_fed { - if let Some(number_font) = script.font_object("NumberFontNormal") { - let adv = crate::ui_text::digit_advances( - atlas, - crate::ui_text::FontSpec { - path: number_font.font.as_deref(), - // The one-to-one drawn px (cap inert at NumberFontNormal's 14) × the - // virtual scale; the widths divide back to UI units below. - height: crate::ui_text::drawn_px(number_font.height, None, s), - // The TRUE outline (NumberFontNormal is NORMAL-outlined): it selects the - // baked ring cell; only a THICK outline also biases the step law - // (`outline-bake-tint.md`) — either way the fed digit widths match what - // the render pass actually lays. - outline: number_font.outline, - paint_halo: true, - alpha_gradient: None, - }, - ); - let adv = adv.map(|a| a / s); - script.set_digit_advances(&adv); - *digits_fed = true; - } - } - } // The message-line half of the round-trip: chat ring lines ask for their wrapped ROW COUNT at // the frame's resolved width, so the emit pass can stack real content heights (a long line // pushes older lines up instead of overlapping them). Same-frame like the FontString half, but @@ -727,39 +710,14 @@ fn measure_fontstrings( if requests.is_empty() { return false; } + // Every answer goes through the ONE measure body ([`crate::ui_text::measure_request`]) the VM's + // own synchronous measurer calls, over the same `Arc`-shared table — so a string measured here + // and the same string measured mid-tick are not two computations that agree, they are one. let measures: Vec<(u32, f32, f32, f32, u64)> = requests .iter() .map(|r| { - // The full raster scale: seam × the owner frame's effective_scale. Measure at the - // exact drawn size and divide the whole product back out, so the frame-LOCAL answer - // times the layout's scale lands on the drawn glyphs to the pixel (integer-stepped - // advances don't commute with scaling — the request's `scale` doc). - let rs = s * r.scale; - let spec = |()| crate::ui_text::FontSpec { - path: r.font.as_deref(), - // The render pass's exact drawn px (two regimes × the full scale) — - // measure == render, and Lua's GetStringWidth/Height echo the DRAWN size - // (0x772890); results divide back to frame-local UI units below. - height: crate::ui_text::drawn_px(r.height, r.text_height, rs), - // The TRUE outline: THICK biases the client's step law (+1px per glyph — - // GlyphStepBase 0x5ca2b0, THICK-only per outline-bake-tint.md) and any outline - // adds the +2r line pitch, so measure must see it. - outline: r.outline, - paint_halo: true, // measure never paints; irrelevant here - alpha_gradient: None, // alpha never changes metrics - }; - let wrap = r.wrap_width.map(|w| w * rs); - let (w, h) = crate::ui_text::measure_text(atlas, &r.text, wrap, spec(())); - // …and the NATURAL width, which is what `GetStringWidth` answers with (the reference - // measures its getter's string with no wrap constraint — wow-re - // `fontstring-overflow.md`, "The measurement echo"). A second pass only for the - // regions that actually carry a declared width; for the rest the two are one number. - let natural = if wrap.is_some() { - crate::ui_text::measure_text(atlas, &r.text, None, spec(())).0 - } else { - w - }; - (r.id, w / rs, h / rs, natural / rs, r.key) + let (w, h, natural) = crate::ui_text::measure_request(&atlas.metrics, s, r); + (r.id, w, h, natural, r.key) }) .collect(); script.set_measured_text(&measures); diff --git a/crates/benilla-app/src/ui_script/extract/text.rs b/crates/benilla-app/src/ui_script/extract/text.rs index 89c49e44..0c2de571 100644 --- a/crates/benilla-app/src/ui_script/extract/text.rs +++ b/crates/benilla-app/src/ui_script/extract/text.rs @@ -188,7 +188,7 @@ pub(super) fn emit( // (`ink=`). They diverge by the width of the markup when the advance table is measured over // the raw buffer — the caret-out-in-space report (decision 1075) as a pair of numbers. let ebox_geom = ebox.map(|ui| { - let ink = crate::ui_text::measure_text(atlas, draw_text, None, spec).0; + let ink = crate::ui_text::measure_text(&atlas.metrics, draw_text, None, spec).0; format!(" caret={:.1} ink={ink:.1}", ui.caret_x * host.scale) }); info!( diff --git a/crates/benilla-app/src/ui_script/merchant_tests.rs b/crates/benilla-app/src/ui_script/merchant_tests.rs index 1c0d2099..f74c0d7f 100644 --- a/crates/benilla-app/src/ui_script/merchant_tests.rs +++ b/crates/benilla-app/src/ui_script/merchant_tests.rs @@ -783,7 +783,7 @@ fn money_display_shrinks_to_content_and_stays_flush() { load_xml(&s, "MerchantFrame.xml"); // The app's synchronous number metrics (ui_script feeds NumberFontNormal's real advances once // per atlas bake; a uniform 6px here keeps the arithmetic below legible). - s.set_digit_advances(&[6.0; 10]); + s.set_text_measurer(Box::new(super::FixedWidthFont(6.0))); s.set_money(12_345); // 1g 23s 45c → purse digits "1" / "23" / "45" s.set_merchant(Some(MerchantState { @@ -1172,7 +1172,7 @@ fn trade_recipient_money_renders_the_digit_not_ellipsis() { load_xml(&s, "GameTooltip.xml"); load_xml(&s, "MerchantFrame.xml"); // the BenillaMoney_* helpers load_xml(&s, "TradeFrame.xml"); - s.set_digit_advances(&[6.0; 10]); + s.set_text_measurer(Box::new(super::FixedWidthFont(6.0))); let target = benilla_ui::script::TradeSideState { gold: 5, // a partner offering 5 copper → the recipient trio should show "5" diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index 223cdfd7..64c9f82f 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/crates/benilla-app/src/ui_script/mod.rs @@ -889,6 +889,24 @@ fn demo_unit_feed(script: Option>, mut fired: Local) } } +/// A fixed-advance stand-in font engine for tests — every character `.0` wide, one line tall, +/// greedily wrapped. The *numbers* the real engine produces are pinned against the client's own +/// fonts by `ui_text::atlas::metrics_tests`; a fixture wants arithmetic a reader can do in their +/// head, and wants a measure to arrive **in the tick that asked** exactly as the app's does. +#[cfg(test)] +pub(crate) struct FixedWidthFont(pub(crate) f32); + +#[cfg(test)] +impl benilla_ui::script::TextMeasure for FixedWidthFont { + fn measure(&mut self, req: &benilla_ui::script::MeasureRequest) -> (f32, f32, f32) { + let natural = req.text.chars().count() as f32 * self.0; + match req.wrap_width { + Some(w) if w > 0.0 && natural > w => (w, 12.0 * (natural / w).ceil(), natural), + _ => (natural, 12.0, natural), + } + } +} + #[cfg(test)] mod cast_tests; diff --git a/crates/benilla-app/src/ui_script/money_frame_tests.rs b/crates/benilla-app/src/ui_script/money_frame_tests.rs index f9644886..d173502a 100644 --- a/crates/benilla-app/src/ui_script/money_frame_tests.rs +++ b/crates/benilla-app/src/ui_script/money_frame_tests.rs @@ -40,7 +40,7 @@ fn harness(money: u64) -> UiScript { // The digit advances the app feeds once per atlas scale. Fed here so the width arithmetic // below is deliberate rather than an artefact of an unfed VM — a flat 8px per digit makes // every expected number readable as `digits x 8 + icon`. - s.set_digit_advances(&[8.0; 10]); + s.set_text_measurer(Box::new(super::FixedWidthFont(8.0))); load_xml(&s, "Fonts.xml"); load_xml(&s, "MoneyFrame.xml"); diff --git a/crates/benilla-app/src/ui_text/atlas.rs b/crates/benilla-app/src/ui_text/atlas.rs index 4c6f65c7..16b4c3fb 100644 --- a/crates/benilla-app/src/ui_text/atlas.rs +++ b/crates/benilla-app/src/ui_text/atlas.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::sync::Arc; use bevy::asset::RenderAssetUsages; use bevy::image::Image; @@ -518,6 +519,79 @@ fn default_charset() -> Vec { // Integration — the resource, plugin, and Bevy Startup system // ───────────────────────────────────────────────────────────────────────────────────────────── +/// One character's contribution to a line's width, at one (face, size), **before** the outline +/// bias — the pre-summed floor of its glyphs' physical advances plus how many glyphs it shaped to. +/// +/// Two numbers rather than one because the client's step law +/// ([`super::layout::client_step`]) adds its bias **per glyph**, not per character: a character +/// that shapes to two glyphs takes two biases. Every character in the baked charset shapes to one +/// under the client's four faces, so `glyphs` is 1 everywhere today — it is here so the law stays +/// the law if that ever stops being true. +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct CharStep { + /// Σ `advance.floor()` over the character's glyphs, in **physical** px. + pub(super) floor_sum: f32, + /// How many glyphs it shaped to — the multiplier on the outline bias. + pub(super) glyphs: u16, +} + +/// Everything **width measurement** needs, and nothing that needs `&mut` — the pure-data half of +/// [`UiFontAtlas`], split out so it can be handed to the script VM as a +/// [`TextMeasure`](benilla_ui::script::TextMeasure) and answer a `GetStringWidth` inline. +/// +/// **Why a table instead of a shaper.** The client's width law is a sum of per-glyph steps with no +/// kerning term (`ComputeStep 0x5ca2d0` — see [`super::layout::client_step`]), so a string's width +/// is a pure function of its characters, its face, its size and its outline. Nothing about it needs +/// a shaping engine; the shaper was only ever how we looked an advance up. Precomputing that lookup +/// at bake time is therefore not an approximation of the measure path — it **is** the measure path, +/// with the one `&mut`-shaped step hoisted out of the hot loop. +/// +/// Cheap to clone by design ([`Arc`] at every holder), because the VM's copy and the atlas's are +/// the same table and must never be two tables. +pub(crate) struct TextMetrics { + /// `(family, baked size bits) -> char -> step` for the baked charset. A character absent here + /// has no baked glyph, draws nothing, and measures **zero** — see [`Self::char_step`]. + pub(super) steps: HashMap<(String, u32), HashMap>, + /// Blizzard font path (lowercased) → `fontdb` family name. + pub(super) path_to_family: HashMap, + /// The fallback family (Friz Quadrata) for a FontString with no/unknown font path. + pub(super) default_family: String, + /// The default family's baked sizes (the superset), ascending — [`Self::snap_size`]'s list. + pub(super) sizes: Vec, + /// Per-family baked sizes (ascending) — [`Self::snap_for`]'s lookup. + pub(super) sizes_by_family: HashMap>, + /// The physical/logical scale the atlas was baked at; every step above is physical px. + pub(super) scale: f32, +} + +impl TextMetrics { + /// The `fontdb` family a Blizzard font path resolves to, or the default face. + pub(super) fn family_for(&self, path: Option<&str>) -> String { + path.and_then(|p| self.path_to_family.get(&p.to_ascii_lowercase())) + .unwrap_or(&self.default_family) + .clone() + } + + /// The baked size `want` snaps to **for a specific family** — keyed by the face it resolved, so + /// a request never snaps to a size only another face bakes. + pub(super) fn snap_for(&self, family: &str, want: f32) -> f32 { + let sizes = self.sizes_by_family.get(family).unwrap_or(&self.sizes); + nearest_size(sizes, want) + } + + /// One character's step at `(family, size)`. `None` for a character with no baked glyph: it + /// draws nothing, so it measures nothing. That is a deliberate, narrow divergence from the + /// render **pen**, which still tracks such a glyph's shaped advance + /// ([`super::layout::layout_text_quads`]) — it applies only to text outside the baked charset, + /// i.e. text this client cannot draw at all. + pub(super) fn char_step(&self, family: &str, size: f32, c: char) -> Option { + self.steps + .get(&(family.to_string(), size.to_bits())) + .and_then(|m| m.get(&c)) + .copied() + } +} + /// The baked glyph atlas + live shaping state, built once at Startup from the client's own Friz /// Quadrata TTF. See the module doc's v1 simplifications for why only one face/size is baked. #[derive(Resource)] @@ -530,29 +604,15 @@ pub(crate) struct UiFontAtlas { pub(super) glyphs: HashMap, /// The packed atlas bitmap, already uploaded. pub(super) image: Handle, - /// Blizzard font path (lowercased, e.g. `"fonts\\arialn.ttf"`) → `fontdb` family name — how a - /// `FontString`'s resolved `font` path selects its baked face. A path not here falls back to - /// [`Self::default_family`]. - pub(super) path_to_family: HashMap, /// Per-family baseline-ascender fraction `hhea.asc / (asc + |desc|)` (see [`hhea_ascent_ratio`]) /// — the term in the client's placement law; [`crate::ui_text::layout`] seats each line's /// baseline at `round(size · ratio)`, the face pixel ascender `[CGxFont+0x17c]` that /// `glyph_vplace` 0x5d1360 hangs ink from. pub(super) family_ascent: HashMap, - /// The fallback family (Friz Quadrata) for a `FontString` with no/unknown font path. - pub(super) default_family: String, - /// The default family's baked sizes (Friz — the superset: [`FONT_SIZES`] + - /// [`FRIZ_SPLASH_SIZES`], ascending) — what [`Self::snap_size`] and any family miss snap to. - pub(super) sizes: Vec, - /// Per-family baked sizes (ascending) — [`Self::snap_for`]'s lookup, so a face only ever - /// snaps to a size it actually baked (a non-Friz face never lands on the Friz-only 26/32). - pub(super) sizes_by_family: HashMap>, - /// The physical/logical scale the atlas was baked at (`Window::scale_factor` at build). Every - /// baked [`GlyphInfo`] metric is physical px = logical × this; [`layout_text_quads`]/ - /// [`measure_text`] divide by it to work in logical units and snap positions to `1/scale` - /// (the physical-pixel grid). Read once at build; a runtime monitor hop is **ignored** (logged), - /// not rebaked — see [`load_fonts_and_build_atlas`]. - pub(super) scale: f32, + /// The width law's inputs — face resolution, size snapping, per-character steps, bake scale. + /// Shared (never copied) with the script VM's synchronous measurer, so a string measured + /// mid-tick and the same string measured at extract are the same number by construction. + pub(crate) metrics: Arc, } impl UiFontAtlas { @@ -561,10 +621,8 @@ impl UiFontAtlas { /// The overhead-name mesh bake (`crate::nameplates`) uses it to hang each line from its /// byte-law baseline (the world-mode seat, wow-re `name-render-geometry-law.md`). pub(crate) fn ascent_ratio(&self, path: Option<&str>) -> f32 { - let family = path - .and_then(|p| self.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&self.default_family); - self.family_ascent.get(family).copied().unwrap_or(0.794) + let family = self.metrics.family_for(path); + self.family_ascent.get(&family).copied().unwrap_or(0.794) } /// The baked size [`nearest_size`] snaps a requested height to — for a caller outside the @@ -573,14 +631,12 @@ impl UiFontAtlas { /// target size around its own anchor. Snaps against the default family (Friz) — the superset /// list, and the face those callers draw. pub(crate) fn snap_size(&self, want: f32) -> f32 { - nearest_size(&self.sizes, want) + nearest_size(&self.metrics.sizes, want) } - /// The baked size `want` snaps to **for a specific family** — the layout path's snap, keyed - /// by the face it resolved, so a request never snaps to a size only another face bakes. - pub(super) fn snap_for(&self, family: &str, want: f32) -> f32 { - let sizes = self.sizes_by_family.get(family).unwrap_or(&self.sizes); - nearest_size(sizes, want) + /// The shared width-law table — what the script VM's synchronous measurer is built from. + pub(crate) fn metrics(&self) -> Arc { + Arc::clone(&self.metrics) } /// The packed atlas texture — for the world-pass consumers (the nameplate meshes) that bind @@ -810,19 +866,84 @@ fn load_fonts_and_build_atlas( .get(&friz_family) .cloned() .unwrap_or_else(|| FONT_SIZES.to_vec()); + let steps = build_char_steps(&mut font_system, &built.glyphs, &faces, &charset, scale); commands.insert_resource(UiFontAtlas { font_system, glyphs: built.glyphs, image, - path_to_family, family_ascent, - default_family: friz_family, - sizes: default_sizes, - sizes_by_family, - scale, + metrics: Arc::new(TextMetrics { + steps, + path_to_family, + default_family: friz_family, + sizes: default_sizes, + sizes_by_family, + scale, + }), }); } +/// Precompute every baked character's [`CharStep`] at every (face, size) — the width law's lookup +/// table, shaped **once** here so no measurement ever needs a shaper again. +/// +/// Each character is shaped alone, which is exactly right rather than a shortcut: the advance that +/// lands in the sum is the *baked cell's* (`GlyphInfo::advance`), which is the face's own advance +/// for that glyph and carries no neighbour term — so shaping "abc" and shaping "a","b","c" produce +/// the same three advances. (Kerning cannot enter: the client's law drops it, and the only path +/// that would have leaked cosmic-text's kerned `LayoutGlyph::w` is the no-baked-cell fallback, +/// which by construction cannot fire for a character that is in the charset we just baked.) +fn build_char_steps( + font_system: &mut FontSystem, + glyphs: &HashMap, + faces: &[(fontdb::ID, String, Vec)], + charset: &[char], + scale: f32, +) -> HashMap<(String, u32), HashMap> { + let mut out: HashMap<(String, u32), HashMap> = HashMap::new(); + let mut scratch = String::new(); + for (_, family, sizes) in faces { + let attrs = Attrs::new().family(Family::Name(family)); + for &size in sizes { + let mut table: HashMap = HashMap::new(); + for &c in charset { + scratch.clear(); + scratch.push(c); + // Shaped at the PHYSICAL size, exactly as the render pass shapes — so the + // no-baked-cell fallback below reads a physical advance like every other one. + let phys = size * scale; + let mut buffer = Buffer::new(font_system, Metrics::new(phys, phys)); + buffer.set_wrap(font_system, Wrap::None); + buffer.set_text(font_system, &scratch, &attrs, Shaping::Advanced, None); + buffer.shape_until_scroll(font_system, false); + let mut floor_sum = 0.0f32; + let mut n = 0u16; + for run in buffer.layout_runs() { + for g in run.glyphs { + let key: GlyphKey = (g.font_id, g.glyph_id, size.to_bits(), 0); + // A charset character with no baked cell was reported in `built.missing` + // and cannot be drawn; its shaped width is still the honest answer for the + // space the render pen will leave. + let adv = glyphs.get(&key).map(|i| i.advance).unwrap_or(g.w); + floor_sum += adv.floor(); + n += 1; + } + } + if n > 0 { + table.insert( + c, + CharStep { + floor_sum, + glyphs: n, + }, + ); + } + } + out.insert((family.clone(), size.to_bits()), table); + } + } + out +} + /// Snap a requested font height to the nearest baked size in `sizes` (never empty — always at least /// one baked face/size). Keeps the shaped size identical to a baked bitmap variant. pub(super) fn nearest_size(sizes: &[f32], requested: f32) -> f32 { @@ -914,3 +1035,155 @@ mod outlined_cell_tests { assert_eq!((w, h, pad), (6, 6, 2)); } } + +#[cfg(test)] +mod metrics_tests { + use super::*; + + /// **The differential test: the step table must measure exactly what shaping the whole string + /// measured**, over the real client fonts. + /// + /// The refactor that made a synchronous `GetStringWidth` possible replaced "shape the run, sum + /// each glyph's baked advance under the step law" with "sum each character's precomputed step". + /// The two are the same number only because of two facts, and this asserts both at once: + /// + /// 1. **The decomposition is per character** — these four faces carry no ligature or contextual + /// substitution, so shaping `"Onewarrior"` yields the same glyph sequence as shaping its ten + /// characters one at a time. (Not a given: it is a property of the fonts, and exactly the + /// kind that would go silently wrong.) + /// 2. **The advance in the sum was never the kerned one.** [`build_atlas`] shapes each charset + /// character alone and stores *that* advance, and the old measure looked the baked advance + /// up by glyph — so cosmic-text's kerned `LayoutGlyph::w` never entered a baked glyph's + /// width. Which is the client's own law: `ComputeStep 0x5ca2d0` drops kerning. + /// + /// So the reference implementation below is the pre-refactor code, verbatim, and the table must + /// match it to the bit. Skips without an install; the fonts are client data. + #[test] + fn the_step_table_measures_what_shaping_the_whole_string_measured() { + let data = benilla_formats::wow_data_or_skip!(); + let chain = match benilla_formats::open_chain(&data) { + Ok(c) => c, + Err(e) => { + eprintln!("skipping: patch chain would not open: {e:#}"); + return; + } + }; + // Real strings, of the kinds that actually reach a measure: character names (the bug's own + // subject), item names, prose, the sequences a ligature table would target, the Latin-1 + // tail of the baked charset, and the digits the money frame sums. + const CORPUS: &[&str] = &[ + "", + " ", + "Onewarrior", + "Probezero", + "Small Brown Pouch", + "Staff of the Shadow Flame", + "The affluent fiend's fickle offer", + "fi fl ffi ffl ff", + "AV Ta Wo Yo LT P, r. v.", + "0123456789", + "Grüße, Ärger; élan côté", + "!@#$%^&*()_+-=[]{}|;':\",./<>?", + ]; + let charset = default_charset(); + let mut checked = 0usize; + for (i, &path) in CLIENT_FONTS.iter().enumerate() { + let Ok(bytes) = chain.read(path) else { + eprintln!("skipping {path}: not in the chain"); + continue; + }; + let mut font_system = FontSystem::new_with_fonts(std::iter::empty()); + let mut swash = SwashCache::new(); + let (id, family) = register_font(&mut font_system, bytes).expect("register"); + // Two sizes, one small and one large, so a size-dependent hinting difference could not + // hide between them; two bake scales, so the physical-px floor is exercised off 1.0. + for scale in [1.0f32, 2.0] { + let sizes = vec![10.0f32, 20.0]; + let faces = vec![(id, family.clone(), sizes.clone())]; + let built = build_atlas( + &mut font_system, + &mut swash, + &faces, + &charset, + (1024.0 * scale).ceil() as u32, + scale, + &HashMap::new(), + ); + let steps = + build_char_steps(&mut font_system, &built.glyphs, &faces, &charset, scale); + let metrics = TextMetrics { + steps, + path_to_family: HashMap::from([(path.to_ascii_lowercase(), family.clone())]), + default_family: family.clone(), + sizes: sizes.clone(), + sizes_by_family: HashMap::from([(family.clone(), sizes.clone())]), + scale, + }; + let attrs = Attrs::new().family(Family::Name(&family)); + for &size in &sizes { + // Both step biases: plain/NORMAL (+1) and THICK (+2). + for step_extra in [1.0f32, 2.0] { + for text in CORPUS { + let want = reference_line_width( + &mut font_system, + &built.glyphs, + &attrs, + size, + scale, + step_extra, + text, + ); + let got = crate::ui_text::measure_line_width_for_test( + &metrics, &family, size, step_extra, text, + ); + assert_eq!( + got, want, + "{path} @{size} scale {scale} extra {step_extra}: {text:?} — the \ + step table and the shaped sum must be one number" + ); + checked += 1; + } + } + } + } + let _ = i; + } + assert!( + checked > 0, + "no client font was readable — nothing was proven" + ); + } + + /// The measure **exactly as it was before the step table**: shape the run, take each glyph's + /// baked advance (cosmic-text's own `w` only where nothing was baked), floor it, add the + /// outline bias, divide back to logical. Kept here, and only here, as the thing the table is + /// diffed against. + #[allow(clippy::too_many_arguments)] + fn reference_line_width( + font_system: &mut FontSystem, + glyphs: &HashMap, + attrs: &Attrs, + font_size: f32, + scale: f32, + step_extra: f32, + text: &str, + ) -> f32 { + if text.is_empty() { + return 0.0; + } + let phys = font_size * scale; + let mut buffer = Buffer::new(font_system, Metrics::new(phys, phys)); + buffer.set_wrap(font_system, Wrap::None); + buffer.set_text(font_system, text, attrs, Shaping::Advanced, None); + buffer.shape_until_scroll(font_system, false); + let mut w = 0.0f32; + for run in buffer.layout_runs() { + for g in run.glyphs { + let key: GlyphKey = (g.font_id, g.glyph_id, font_size.to_bits(), 0); + let adv = glyphs.get(&key).map(|i| i.advance).unwrap_or(g.w); + w += (adv.floor() + step_extra) / scale; + } + } + w + } +} diff --git a/crates/benilla-app/src/ui_text/layout/mod.rs b/crates/benilla-app/src/ui_text/layout/mod.rs index 20d438cd..d0d7e1eb 100644 --- a/crates/benilla-app/src/ui_text/layout/mod.rs +++ b/crates/benilla-app/src/ui_text/layout/mod.rs @@ -1,14 +1,12 @@ -use std::collections::HashMap; - use bevy::math::Rect; use bevy::prelude::*; -use cosmic_text::{Attrs, Buffer, Family, FontSystem, Metrics, Shaping, Wrap}; +use cosmic_text::{Attrs, Buffer, Family, Metrics, Shaping, Wrap}; use benilla_ui::script::{JustifyH, JustifyV, Outline}; use crate::ui_pass::{UiQuad, UvRect}; -use super::atlas::{GlyphInfo, GlyphKey, UiFontAtlas}; +use super::atlas::{GlyphKey, TextMetrics, UiFontAtlas}; use super::markup::{parse_markup, ColorRun}; use super::DEFAULT_FONT_SIZE; @@ -104,42 +102,44 @@ fn step_extra_of(outline: Outline) -> f32 { } } -/// Shape `text` on one line and return its total laid-out width (logical px) at `font_size` under -/// the client's step law ([`client_step`]): the sum of per-glyph steps — the baked atlas advance -/// (or cosmic-text's own `glyph.w` for a char with no baked glyph), floored, `+ step_extra`. This -/// is the client's `GetTextWidth` (a sum of `ComputeStep`s), so measure == render == the real -/// client's width. Empty text is zero-width. Shares [`UiFontAtlas`]'s two disjoint fields by -/// direct field access, so the caller keeps its `&mut atlas` intact. -fn measure_line_width( - font_system: &mut FontSystem, - glyphs: &HashMap, - attrs: &Attrs, +/// One line's total laid-out width (logical px) at `font_size` under the client's step law +/// ([`client_step`]): the sum of per-character steps, read from the bake-time table +/// ([`TextMetrics::char_step`]). This is the client's `GetTextWidth` (a sum of `ComputeStep`s), so +/// measure == render == the real client's width. Empty text is zero-width. +/// +/// **No shaper, and that is the point.** The client's law has no kerning term and no neighbour +/// term, so a string's width is a pure function of its characters, face, size and outline — which +/// is what lets the lookup be precomputed, and therefore what lets this answer from *inside* a Lua +/// call ([`benilla_ui::script::TextMeasure`]), where a `&mut FontSystem` could never reach. Every +/// measurement in the app now runs through this one function, so the synchronous answer and the +/// batch answer cannot be different numbers. +/// +/// A character with no baked glyph contributes 0 — see [`TextMetrics::char_step`] for the single +/// narrow place that differs from the render pen. +#[cfg(test)] +pub(crate) fn measure_line_width_for_test( + metrics: &TextMetrics, + family: &str, font_size: f32, - scale: f32, step_extra: f32, text: &str, ) -> f32 { - if text.is_empty() { - return 0.0; - } - // Shape at the PHYSICAL size (the resolution the atlas baked at), then divide the physical - // advances back to logical — the same scaled-face-divided-back path the emit pass uses. - let phys = font_size * scale; - let mut buffer = Buffer::new(font_system, Metrics::new(phys, phys)); - buffer.set_wrap(font_system, Wrap::None); - buffer.set_text(font_system, text, attrs, Shaping::Advanced, None); - buffer.shape_until_scroll(font_system, false); - let mut w = 0.0f32; - for run in buffer.layout_runs() { - for g in run.glyphs { - // Measurement always reads the plain (r=0) cell: advances are variant-invariant, - // and every face/size bakes a plain cell (outlined variants are a planned subset). - let key: GlyphKey = (g.font_id, g.glyph_id, font_size.to_bits(), 0); - let adv = glyphs.get(&key).map(|i| i.advance).unwrap_or(g.w); - w += client_step(adv, step_extra, scale); - } - } - w + measure_line_width(metrics, family, font_size, step_extra, text) +} + +fn measure_line_width( + metrics: &TextMetrics, + family: &str, + font_size: f32, + step_extra: f32, + text: &str, +) -> f32 { + // The table's steps are PHYSICAL px (the resolution the atlas baked at); the divide back to + // logical is [`client_step`]'s, applied per glyph because the bias is per glyph. + text.chars() + .filter_map(|c| metrics.char_step(family, font_size, c)) + .map(|s| (s.floor_sum + step_extra * f32::from(s.glyphs)) / metrics.scale) + .sum() } /// The EditBox advance-table answer ([`benilla_ui::script::UiScript::set_editbox_advances`]'s @@ -169,14 +169,12 @@ pub(crate) fn line_advances(atlas: &mut UiFontAtlas, text: &str, font: FontSpec) if drawn.is_empty() { return cum; // pure markup draws nothing — every boundary sits at x = 0 } - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); + let family = atlas.metrics.family_for(font.path); let attrs = Attrs::new().family(Family::Name(&family)); - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; + let font_size = atlas + .metrics + .snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); + let scale = atlas.metrics.scale; let step_extra = step_extra_of(font.outline); // Byte offset of each buffer line ('\n'-split) within the DRAWN string — glyph cluster ranges // are line-relative, and the map below is indexed in drawn bytes. @@ -259,14 +257,10 @@ pub(crate) fn line_rows( wrap_width: f32, font: FontSpec, ) -> (Vec, f32) { - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); - let attrs = Attrs::new().family(Family::Name(&family)); - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; + let family = atlas.metrics.family_for(font.path); + let font_size = atlas + .metrics + .snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); let step_extra = step_extra_of(font.outline); let mut rows = Vec::new(); let mut base = 0usize; // byte offset of the current '\n' segment within `text` @@ -277,11 +271,9 @@ pub(crate) fn line_rows( .iter() .flat_map(|line| { wrap_line( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, + &atlas.metrics, + &family, font_size, - scale, step_extra, line, wrap_width, @@ -348,13 +340,10 @@ fn segment_row_starts(seg: &str, sub: &[Vec], base: usize, rows: &mut /// mid-word breaks become opportunities even when a space exists) are unmodeled; no shipped /// FontString renders CJK, and the one `nonspacewrap` consumer (MinimapZoneText) sits in a /// one-line box where opportunity choice can't change the outcome. -#[allow(clippy::too_many_arguments)] // the shared shaping context + the step-law bias, one each fn wrap_line( - font_system: &mut FontSystem, - glyphs: &HashMap, - attrs: &Attrs, + metrics: &TextMetrics, + family: &str, font_size: f32, - scale: f32, step_extra: f32, line: &[ColorRun], max_width: f32, @@ -368,7 +357,7 @@ fn wrap_line( } greedy_pack(words, max_width, |t| { - measure_line_width(font_system, glyphs, attrs, font_size, scale, step_extra, t) + measure_line_width(metrics, family, font_size, step_extra, t) }) } @@ -396,38 +385,21 @@ fn wrap_line( /// ([`benilla_ui::script::UiScript::fontstrings_needing_measure`]) sizes height-less FontStrings /// with this — the real client's layout asks its font engine for string metrics the same way. pub(crate) fn measure_text( - atlas: &mut UiFontAtlas, + metrics: &TextMetrics, text: &str, wrap_width: Option, font: FontSpec, ) -> (f32, f32) { let lines = parse_markup(text, [1.0, 1.0, 1.0, 1.0]); - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); - let attrs = Attrs::new().family(Family::Name(&family)); - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; + let family = metrics.family_for(font.path); + let font_size = metrics.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); // The step-law bias ([`client_step`]): measure MUST see the same outline-biased steps the // render pass lays, or wrap points and measured widths drift from what's drawn. let step_extra = step_extra_of(font.outline); let render_lines: Vec> = match wrap_width { Some(w) if w > WRAP_MIN_WIDTH => lines .iter() - .flat_map(|line| { - wrap_line( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, - font_size, - scale, - step_extra, - line, - w, - ) - }) + .flat_map(|line| wrap_line(metrics, &family, font_size, step_extra, line, w)) .collect(), _ => lines, }; @@ -436,15 +408,7 @@ pub(crate) fn measure_text( let mut w = 0.0f32; for run in line { if !run.text.is_empty() { - w += measure_line_width( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, - font_size, - scale, - step_extra, - &run.text, - ); + w += measure_line_width(metrics, &family, font_size, step_extra, &run.text); } } max_w = max_w.max(w); @@ -471,23 +435,17 @@ pub(crate) fn measure_wrapped_rows( wrap_width: f32, font: FontSpec, ) -> u16 { - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); - let attrs = Attrs::new().family(Family::Name(&family)); + let family = atlas.metrics.family_for(font.path); // Per-family size snap (the rebase-landed law): rows must count at the exact size the render // pass bakes this face at, or band heights drift from the drawn block. - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; + let font_size = atlas + .metrics + .snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); let step_extra = step_extra_of(font.outline); let rows = wrapped_rows( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, + &atlas.metrics, + &family, font_size, - scale, step_extra, text, wrap_width, @@ -498,13 +456,10 @@ pub(crate) fn measure_wrapped_rows( /// The wrapped display-row count of `text` at `wrap_width` — the exact [`wrap_line`] pass the /// render uses (same step law, same outline bias), shared by [`measure_wrapped_rows`] (the chat /// band half) and [`ellipsize_to_fit`]'s fit test. An unconstrained width counts the `\n` lines. -#[allow(clippy::too_many_arguments)] // the shared shaping context, same shape as wrap_line's fn wrapped_rows( - font_system: &mut FontSystem, - glyphs: &HashMap, - attrs: &Attrs, + metrics: &TextMetrics, + family: &str, font_size: f32, - scale: f32, step_extra: f32, text: &str, wrap_width: f32, @@ -513,19 +468,7 @@ fn wrapped_rows( if wrap_width > WRAP_MIN_WIDTH { lines .iter() - .map(|line| { - wrap_line( - font_system, - glyphs, - attrs, - font_size, - scale, - step_extra, - line, - wrap_width, - ) - .len() - }) + .map(|line| wrap_line(metrics, family, font_size, step_extra, line, wrap_width).len()) .sum() } else { lines.len() @@ -552,29 +495,22 @@ pub(crate) fn ellipsize_to_fit( if rect.width() <= WRAP_MIN_WIDTH || rect.height() <= HEIGHT_LIMIT_MIN { return None; } - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); - let attrs = Attrs::new().family(Family::Name(&family)); - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; + let family = atlas.metrics.family_for(font.path); + let font_size = atlas + .metrics + .snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); let step_extra = step_extra_of(font.outline); // The line pitch — identical to the emit pass's ([`layout_text_quads`]: the font em, NO // outline pad — the byte law, `fontstring-vertical-placement.md`), so the fit test and the // drawn stack agree line for line. - let font_system = &mut atlas.font_system; - let glyphs = &atlas.glyphs; // The line-count law lives in `overflow` (it is the FIT law here, not the render stack's) — // see `overflow::ellipsize_in_box`. + let metrics = &atlas.metrics; overflow::ellipsize_in_box(text, rect.height(), font_size, |candidate| { wrapped_rows( - font_system, - glyphs, - &attrs, + metrics, + &family, font_size, - scale, step_extra, candidate, rect.width(), @@ -582,39 +518,6 @@ pub(crate) fn ellipsize_to_fit( }) } -/// Per-digit (`'0'..='9'`) step widths (logical px) at the resolved face/size — the synchronous -/// number-metrics feed behind the script side's money layout ([`benilla_ui::script::UiScript:: -/// set_digit_advances`]). The real `SmallMoneyFrame` sizes each denomination button with -/// `GetTextWidth` *mid-update* (ref MoneyFrame.lua l.202); our `GetStringWidth` measure round-trip -/// is a frame late, so the app feeds these once per atlas scale and the Lua sums digits. Each is -/// the glyph's [`client_step`] under the font's true outline (NumberFontNormal is outlined ⇒ +2), -/// exactly the width the render pass lays — none of [`measure_text`]'s wrap headroom. -pub(crate) fn digit_advances(atlas: &mut UiFontAtlas, font: FontSpec) -> [f32; 10] { - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); - let attrs = Attrs::new().family(Family::Name(&family)); - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; - let step_extra = step_extra_of(font.outline); - let mut out = [0.0f32; 10]; - for (i, slot) in out.iter_mut().enumerate() { - let s = char::from_digit(i as u32, 10).expect("0..=9").to_string(); - *slot = measure_line_width( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, - font_size, - scale, - step_extra, - &s, - ); - } - out -} - /// The drawn line's origin for the EditBox text-UI overlays: the line's x0 under `justifyH` /// (where the engine's advance-derived caret/selection x-offsets are measured from), plus the /// single-line cell's top and height — the same family/size snap, step law, and justify math as @@ -627,14 +530,10 @@ pub(crate) fn line_origin( justify: Justify, font: FontSpec, ) -> (f32, f32, f32) { - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); - let attrs = Attrs::new().family(Family::Name(&family)); - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); - let scale = atlas.scale; + let family = atlas.metrics.family_for(font.path); + let font_size = atlas + .metrics + .snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); let step_extra = step_extra_of(font.outline); // Everything below returns DRAWN-space geometry ([`drawn_k`]): the string's drawn width, the // caret cell's height, and the cell top all carry the 0581 rescale the glyph quads take. @@ -643,15 +542,7 @@ pub(crate) fn line_origin( let x0 = match justify.h { JustifyH::Left => rect.min.x, JustifyH::Center | JustifyH::Right => { - let w = k * measure_line_width( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, - font_size, - scale, - step_extra, - drawn, - ); + let w = k * measure_line_width(&atlas.metrics, &family, font_size, step_extra, drawn); if matches!(justify.h, JustifyH::Center) { rect.min.x + ((rect.width() - w) * 0.5).max(0.0) } else { @@ -788,14 +679,13 @@ fn layout_text_quads_inner( let gradient = font.alpha_gradient; let lines = parse_markup(text, base_color); // Resolve the face: the FontString's `font` path → its baked family, else Friz (the fallback). - let family = font - .path - .and_then(|p| atlas.path_to_family.get(&p.to_ascii_lowercase())) - .unwrap_or(&atlas.default_family) - .clone(); + let family = atlas.metrics.family_for(font.path); let attrs = Attrs::new().family(Family::Name(&family)); // Resolve the size: the requested height snapped to the nearest baked size (default 12 if none). - let font_size = atlas.snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); + let font_size = atlas + .metrics + .snap_for(&family, font.height.unwrap_or(DEFAULT_FONT_SIZE)); + let scale = atlas.metrics.scale; // DPI-aware layout: shape at the PHYSICAL size the atlas baked at, then divide advances/bearings // back to logical (kerning/hinting computed once at device resolution — never logical shaping with // scaled bitmaps pasted in, which drifts). `snap` rounds a logical coordinate to the nearest @@ -803,7 +693,6 @@ fn layout_text_quads_inner( // integer-device-pixel snap the real client does, which keeps a physical-res bitmap crisp instead // of resampled. The quad's far edge is then automatically physical-aligned (a baked cell is an // integer count of physical texels). - let scale = atlas.scale; let phys_size = font_size * scale; let snap = |v: f32| (v * scale).round() / scale; @@ -852,11 +741,9 @@ fn layout_text_quads_inner( .iter() .flat_map(|line| { wrap_line( - &mut atlas.font_system, - &atlas.glyphs, - &attrs, + &atlas.metrics, + &family, font_size, - scale, step_extra, line, rect.width(), diff --git a/crates/benilla-app/src/ui_text/measurer.rs b/crates/benilla-app/src/ui_text/measurer.rs new file mode 100644 index 00000000..26a8fecb --- /dev/null +++ b/crates/benilla-app/src/ui_text/measurer.rs @@ -0,0 +1,87 @@ +//! **The font engine, as the script VM holds it** — the app's half of +//! [`benilla_ui::script::TextMeasure`]. +//! +//! The engine seam (`benilla-ui`'s `script::measure`) exists because the reference answers +//! `GetStringWidth` from its font engine *inline* and ours could not: measuring needed the baked +//! atlas, and the atlas only met the VM at the frame boundary. What closes the gap is that the +//! client's width law is a **table**, not a computation — a sum of per-glyph steps with no kerning +//! and no neighbour term — so [`TextMetrics`] carries everything a measure needs and nothing that +//! needs `&mut`. That table is `Arc`-shared with [`UiFontAtlas`](super::atlas::UiFontAtlas) rather +//! than copied, so the answer a Lua call gets mid-tick and the answer the extract pass computes are +//! not merely equal — they are the same lookup in the same table. +//! +//! **One body, both callers.** [`measure_request`] is the whole of it; the batch pass +//! (`ui_script::extract::measure_fontstrings`) and [`AtlasMeasurer`] both call it and neither has +//! any measuring logic of its own. That is the trait's own contract, kept structurally rather than +//! by discipline. + +use std::sync::Arc; + +use benilla_ui::script::{MeasureRequest, TextMeasure}; + +use super::atlas::TextMetrics; + +/// Answer one [`MeasureRequest`]: `(laid_out_w, laid_out_h, natural_w)` in the region's own +/// frame-local units. +/// +/// `seam` is the host's screen scale ([`crate::ui_script::seam_scale`]); the request's own `scale` +/// is the owner frame's `effective_scale`. Measuring happens at the product — the exact drawn +/// raster size — and the answer divides the whole product back out, because integer-stepped glyph +/// advances do not commute with scaling (see [`MeasureRequest::scale`]). +pub(crate) fn measure_request( + metrics: &TextMetrics, + seam: f32, + r: &MeasureRequest, +) -> (f32, f32, f32) { + let rs = seam * r.scale; + let spec = || super::FontSpec { + path: r.font.as_deref(), + // The render pass's exact drawn px (two regimes × the full scale) — measure == render, and + // Lua's GetStringWidth/Height echo the DRAWN size (0x772890); results divide back to + // frame-local UI units below. + height: super::drawn_px(r.height, r.text_height, rs), + // The TRUE outline: THICK biases the client's step law (+1px per glyph — GlyphStepBase + // 0x5ca2b0, THICK-only per outline-bake-tint.md) and any outline adds the +2r line pitch, + // so measure must see it. + outline: r.outline, + paint_halo: true, // measure never paints; irrelevant here + alpha_gradient: None, // alpha never changes metrics + }; + let wrap = r.wrap_width.map(|w| w * rs); + let (w, h) = super::measure_text(metrics, &r.text, wrap, spec()); + // …and the NATURAL width, which is what `GetStringWidth` answers with (the reference measures + // its getter's string with no wrap constraint — wow-re `fontstring-overflow.md`, "The + // measurement echo"). A second pass only for the regions that actually carry a declared width; + // for the rest the two are one number. + let natural = if wrap.is_some() { + super::measure_text(metrics, &r.text, None, spec()).0 + } else { + w + }; + (w / rs, h / rs, natural / rs) +} + +/// The measurer installed into the VM — the shared [`TextMetrics`] plus the host's current screen +/// seam. +/// +/// **Rebuilt, not mutated, when the seam moves.** A measurer is only correct for the raster +/// environment it was made under (glyph advances step to whole *physical* pixels), so the host +/// installs a fresh one at exactly the moment it calls +/// [`UiScript::forget_text_metrics`](benilla_ui::script::UiScript::forget_text_metrics) — the same +/// edge, for the same reason. Building one is an `Arc` clone and an `f32`. +pub(crate) struct AtlasMeasurer { + metrics: Arc, + seam: f32, +} + +impl AtlasMeasurer { + pub(crate) fn new(metrics: Arc, seam: f32) -> Self { + Self { metrics, seam } + } +} + +impl TextMeasure for AtlasMeasurer { + fn measure(&mut self, req: &MeasureRequest) -> (f32, f32, f32) { + measure_request(&self.metrics, self.seam, req) + } +} diff --git a/crates/benilla-app/src/ui_text/mod.rs b/crates/benilla-app/src/ui_text/mod.rs index 52b6ff63..30eae166 100644 --- a/crates/benilla-app/src/ui_text/mod.rs +++ b/crates/benilla-app/src/ui_text/mod.rs @@ -45,11 +45,18 @@ mod atlas; mod layout; mod markup; +mod measurer; pub(crate) use atlas::{UiFontAtlas, UiTextPlugin}; +pub(crate) use measurer::{measure_request, AtlasMeasurer}; + +/// The width law itself, for the differential test that pins it to the pre-table shaped sum +/// (`atlas::metrics_tests`). Not a production seam — every caller goes through [`measure_text`]. +#[cfg(test)] +pub(crate) use layout::measure_line_width_for_test; pub(crate) use layout::{ - digit_advances, ellipsize_to_fit, layout_text_quads, layout_text_quads_links, line_advances, - line_origin, line_rows, measure_text, measure_wrapped_rows, FontSpec, Justify, UI_SEAT_NUDGE, + ellipsize_to_fit, layout_text_quads, layout_text_quads_links, line_advances, line_origin, + line_rows, measure_text, measure_wrapped_rows, FontSpec, Justify, UI_SEAT_NUDGE, }; /// The default body text size (logical px) — WoW's own `GameFontNormal`. A `FontString` with no diff --git a/crates/benilla-ui/src/script/channel.rs b/crates/benilla-ui/src/script/channel.rs index c37a98c4..fac9c391 100644 --- a/crates/benilla-ui/src/script/channel.rs +++ b/crates/benilla-ui/src/script/channel.rs @@ -30,15 +30,25 @@ use mlua::{Lua, MultiValue, Value}; use super::Model; -/// The 1-based slot of `name` in join order, case-insensitively — `GetChannelName`'s first return. +/// The 1-based slot of `name`, case-insensitively — `GetChannelName`'s first return. fn slot_of(model: &Model, name: &str) -> Option { model .joined_channels .iter() - .position(|c| c.eq_ignore_ascii_case(name)) + .position(|c| c.as_deref().is_some_and(|c| c.eq_ignore_ascii_case(name))) .map(|i| i + 1) } +/// The name occupying slot `n` (1-based), or `None` for out of range **or a freed slot**. +/// +/// The hole case is the reference's, not a convenience: its by-index lookup `0x49bf30` bounds-checks +/// against the record count and then demands the entry's own number field equal the index asked for +/// (`cmp esi,ecx / jnz`), which a leave zeroed (`0x49bbd0`). So a channel left is a number that +/// answers "not joined" while every channel above it keeps its own (1286). +fn name_at(model: &Model, n: usize) -> Option<&str> { + model.joined_channels.get(n.checked_sub(1)?)?.as_deref() +} + pub(super) fn install(lua: &Lua) -> mlua::Result<()> { let g = lua.globals(); @@ -71,7 +81,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { }; usize::try_from(n) .ok() - .filter(|n| *n >= 1 && *n <= model.joined_channels.len()) + .filter(|n| name_at(&model, *n).is_some()) } // The name form. A numeric STRING arrives here and must still resolve as a number: // `ChatFrame.lua:2113` passes the result of a `gsub` — `GetChannelName("1")` — and @@ -79,7 +89,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Value::String(s) => { let name = s.to_str()?; match name.trim().parse::() { - Ok(n) if n >= 1 && n <= model.joined_channels.len() => Some(n), + Ok(n) if name_at(&model, n).is_some() => Some(n), Ok(_) => None, Err(_) => slot_of(&model, &name), } @@ -92,7 +102,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // caller on this branch (all four sites bail on the number first). return Ok(MultiValue::from_vec(vec![Value::Integer(0)])); }; - let name = model.joined_channels[slot - 1].clone(); + let name = name_at(&model, slot).unwrap_or_default().to_string(); Ok(MultiValue::from_vec(vec![ Value::Integer(slot as i64), Value::String(lua.create_string(&name)?), @@ -132,7 +142,12 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); let mut out = Vec::with_capacity(model.joined_channels.len() * 2); + // Occupied slots only — a freed one is a number nothing is on, so it has no pair to + // contribute (the reference walks its record array and skips the cleared entries). for (i, name) in model.joined_channels.iter().enumerate() { + let Some(name) = name.as_deref() else { + continue; + }; out.push(Value::Integer(i as i64 + 1)); out.push(Value::String(lua.create_string(name)?)); } @@ -150,7 +165,7 @@ impl super::UiScript { /// shape the chat-window work used: that one is a QUEUE the app drains (Lua → app), and this is /// app state READ BY Lua, which is the opposite direction. `ui_chat::feed` owns both edges that /// change it — the server's YOU_JOINED and YOU_LEFT notices — so it pushes here from one place. - pub fn set_joined_channels(&mut self, joined: Vec) { + pub fn set_joined_channels(&mut self, joined: Vec>) { self.model_mut().joined_channels = joined; } } diff --git a/crates/benilla-ui/src/script/layout.rs b/crates/benilla-ui/src/script/layout.rs index 9f46e84e..e4317663 100644 --- a/crates/benilla-ui/src/script/layout.rs +++ b/crates/benilla-ui/src/script/layout.rs @@ -670,46 +670,9 @@ impl UiScript { .map(|(&rh, _)| rh) .collect(); for rh in handles { - let is_fs = model - .arena - .region(rh) - .is_some_and(|r| matches!(r.kind, crate::widget::RegionKind::FontString)); - if !is_fs { - continue; + if let Some(req) = measure_request_for(&mut model, rh) { + out.push(req); } - // The owner's effective_scale: the host measures at the drawn raster size - // ([`MeasureRequest::scale`]), and the key carries it so a SetScale re-measures. - let scale = model - .arena - .region(rh) - .and_then(|r| model.arena.frame(r.owner)) - .map(|f| f.effective_scale) - .unwrap_or(1.0); - let d = model.region_data.get(&rh).expect("live region data"); - let text = d.text.clone().unwrap_or_default(); - let wrap_width = d.size.map(|s| s.0).filter(|w| *w > 0.0); - let font = d.font_path.clone(); - let height = d.font_height; - let text_height = d.text_height; - let outline = d.outline; - // The shared key recipe ([`RegionData::measure_key`]) — the metric reads - // (region.rs) check the stored measure against the same hash. - let key = d.measure_key(scale); - if d.measured.map(|m| m.key) == Some(key) { - continue; - } - let id = model.region_id(rh); - out.push(MeasureRequest { - id, - font, - height, - text_height, - wrap_width, - outline, - scale, - text, - key, - }); } out } @@ -772,3 +735,49 @@ impl UiScript { } } } + +/// The [`MeasureRequest`] region `rh` needs right now, or `None` if it needs none — it is not a +/// FontString, it holds no text, or its stored measure is already this exact string's. +/// +/// Hoisted out of [`UiScript::fontstrings_needing_measure`] so the **synchronous** measure +/// ([`super::measure::ensure_measured`]) builds its request with the identical recipe. The two must +/// agree byte for byte on the cache key, or a same-tick measure and the batch pass would each think +/// the other's answer was stale and re-measure forever. +pub(super) fn measure_request_for(model: &mut Model, rh: RegionHandle) -> Option { + let region = model.arena.region(rh)?; + if !matches!(region.kind, crate::widget::RegionKind::FontString) { + return None; + } + // The owner's effective_scale: the host measures at the drawn raster size + // ([`MeasureRequest::scale`]), and the key carries it so a SetScale re-measures. + let scale = model + .arena + .frame(region.owner) + .map(|f| f.effective_scale) + .unwrap_or(1.0); + let d = model.region_data.get(&rh)?; + let text = d.text.clone().filter(|t| !t.is_empty())?; + let wrap_width = d.size.map(|s| s.0).filter(|w| *w > 0.0); + let font = d.font_path.clone(); + let height = d.font_height; + let text_height = d.text_height; + let outline = d.outline; + // The shared key recipe ([`RegionData::measure_key`]) — the metric reads (region.rs) check the + // stored measure against the same hash. + let key = d.measure_key(scale); + if d.measured.map(|m| m.key) == Some(key) { + return None; + } + let id = model.region_id(rh); + Some(MeasureRequest { + id, + font, + height, + text_height, + wrap_width, + outline, + scale, + text, + key, + }) +} diff --git a/crates/benilla-ui/src/script/measure.rs b/crates/benilla-ui/src/script/measure.rs new file mode 100644 index 00000000..687e6474 --- /dev/null +++ b/crates/benilla-ui/src/script/measure.rs @@ -0,0 +1,148 @@ +//! **The synchronous text measure** — the host's font engine, installed *into* the VM so a +//! `GetStringWidth` answers during the Lua call that asked, not a frame later. +//! +//! ## Why this exists +//! +//! The real client's `GetStringWidth 0x79e510` calls straight through to its font engine +//! (`0x772890`) and returns. Ours could not: measuring needs the baked glyph atlas, the atlas +//! lives in the app, and the app only meets the VM at the frame boundary — so the metric was +//! served from the [`MeasureRequest`](super::MeasureRequest) round-trip, which lands at **extract, +//! one frame after the tick that set the text**. Every same-tick `SetText` → `GetStringWidth` pair +//! read **0**. +//! +//! That is not a rounding difference, it is the difference between a laid-out UI and a broken one, +//! and it is the shape the corpus actually writes: +//! +//! ```lua +//! button:SetText(player) -- Bagnon_Forever/database/ui.lua:58 +//! if button:GetTextWidth() + 40 > width then -- …:59, same tick +//! ``` +//! +//! With the answer 0, that dropdown sized itself to 40px and seven character names drew outside +//! their own frame. The reference's own `SmallMoneyFrame` does the same thing (`MoneyFrame.lua` +//! l.202), which is why [`UiScript::set_digit_advances`](super::UiScript::set_digit_advances) +//! existed: a digits-only stand-in for exactly this, built because the general answer was missing. +//! This is the general answer, and it retires that special case. +//! +//! ## The seam +//! +//! [`TextMeasure`] is one method over the request type the round-trip already speaks, so the host +//! answers a same-tick measure and a batch measure with the same code — there is no second +//! measuring path to drift. The host owns the engine; the engine owns the *when*. +//! +//! **Absent by default, and that is a supported state.** A VM with no measurer installed (every +//! engine test, every headless run without a font atlas) behaves exactly as it did: metrics stay +//! 0 until the round-trip fills them. Nothing here is load-bearing for correctness of the +//! round-trip; it only removes the latency. + +use mlua::Lua; + +use super::{MeasureRequest, MeasuredText, Model}; +use crate::widget::RegionHandle; + +/// The host's font engine, as the VM sees it — installed with +/// [`UiScript::set_text_measurer`](super::UiScript::set_text_measurer). +/// +/// One method, taking the same [`MeasureRequest`] the batch round-trip carries: the host must +/// answer a synchronous measure with the identical code that answers +/// [`UiScript::fontstrings_needing_measure`](super::UiScript::fontstrings_needing_measure), or the +/// two disagree and a string's width depends on *when* it was asked for. +pub trait TextMeasure { + /// `(laid_out_w, laid_out_h, natural_w)` for `req`, in the region's own frame-local units. + /// + /// `laid_out_*` is the text wrapped inside `req.wrap_width` when it has one; `natural_w` is + /// the width it would take **unwrapped**, which is the number `GetStringWidth` reports (see + /// [`MeasuredText::natural_w`]). For a request with no wrap width the two are one number. + /// + /// Must be **pure with respect to the VM** — it is called with the model mutably borrowed and + /// must not re-enter Lua. + fn measure(&mut self, req: &MeasureRequest) -> (f32, f32, f32); +} + +/// Measure region `rh` **now** if its stored measure is stale and a measurer is installed. +/// +/// The one entry point for the metric reads (`GetStringWidth`, `GetWidth`/`GetHeight` on a +/// FontString): they already treat a key-mismatched measure as absent, so filling the cache here +/// is invisible to them beyond the answer arriving. A no-op without a measurer, and a no-op when +/// the stored measure is already current — which is what keeps a per-frame poll from re-measuring. +pub(super) fn ensure_measured(lua: &Lua, rh: RegionHandle) { + let mut model = lua.app_data_mut::().expect("model app_data"); + if model.measurer.is_none() { + return; + } + let Some(req) = super::layout::measure_request_for(&mut model, rh) else { + return; // not a FontString, no text, or the stored measure is already this string's + }; + // Taken out for the call and put straight back: the measurer needs `&mut self` (a font shaper + // is stateful) while the model is already mutably borrowed. Safe because the trait forbids + // re-entering the VM, so nothing can observe the gap. + let mut engine = model.measurer.take().expect("checked above"); + let (w, h, natural_w) = engine.measure(&req); + model.measurer = Some(engine); + let new = MeasuredText { + w, + h, + natural_w, + key: req.key, + }; + if let Some(d) = model.region_data.get_mut(&rh) { + if d.measured != Some(new) { + d.measured = Some(new); + // Measured extents are the auto-size axes' inputs — the layout gate's read set, the + // same touch `set_measured_text` does. Guarded on a real change so a steady poll does + // not dirty the layout every frame. + model.touch_layout(); + } + } +} + +impl super::UiScript { + /// Install the host's font engine, so metric reads answer inside the tick that asked. + /// + /// Call again to replace it — which the host must do whenever its **raster environment** + /// changes (window resize, `uiScale`), for the same reason + /// [`UiScript::forget_text_metrics`](super::UiScript::forget_text_metrics) exists: glyph + /// advances step to whole *physical* pixels, so a measurer built under one seam does not + /// answer for another. + pub fn set_text_measurer(&mut self, measurer: Box) { + self.model_mut().measurer = Some(measurer); + } + + /// Whether a [`TextMeasure`] is installed — the host's gate for skipping its own batch measure + /// pass, which would find nothing to do anyway. + pub fn has_text_measurer(&self) -> bool { + self.model_mut().measurer.is_some() + } + + /// Answer every pending FontString measure inline. Returns whether anything changed (i.e. + /// whether the layout needs another solve). No-op without a measurer. + /// + /// This is [`UiScript::resolve`](super::UiScript::resolve)'s own half of the round-trip: with + /// an engine installed the VM closes the loop itself, so a FontString's box is right in the + /// frame its text was set rather than the frame after — and the host's batch pass finds an + /// empty request list and does nothing. + pub(super) fn fill_measures(&mut self) -> bool { + if !self.has_text_measurer() { + return false; + } + let requests = self.fontstrings_needing_measure(); + if requests.is_empty() { + return false; + } + let answers: Vec<(u32, f32, f32, f32, u64)> = { + let mut model = self.model_mut(); + let mut engine = model.measurer.take().expect("checked above"); + let answers = requests + .iter() + .map(|r| { + let (w, h, natural_w) = engine.measure(r); + (r.id, w, h, natural_w, r.key) + }) + .collect(); + model.measurer = Some(engine); + answers + }; + self.set_measured_text(&answers); + true + } +} diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 95cdaaf4..ab04d9b6 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -77,6 +77,7 @@ mod loot_roll; mod lua50; mod macros; mod mail; +mod measure; mod merchant; mod messageframe; mod minimap; @@ -148,6 +149,7 @@ pub use loot::{LootRow, LootState}; pub use loot_roll::{LootRollEntry, LootRollsState}; pub use macros::{MacroState, MacroView, MAX_MACROS, MAX_MACRO_BODY, MAX_MACRO_NAME}; pub use mail::{MailInboxRow, MailSendRequest, MailState}; +pub use measure::TextMeasure; pub use merchant::{ItemStatsHead, MerchantItem, MerchantState}; pub(crate) use model::Model; pub use party::{PartyMemberInfo, PartyRequest, PartyState, RaidMemberInfo}; @@ -523,7 +525,6 @@ impl UiScript { lua, instructions: std::sync::Arc::new(std::sync::atomic::AtomicU64::new(0)), }; - s.publish_number_width()?; Ok(s) } @@ -796,6 +797,15 @@ impl UiScript { let mut model = self.model_mut(); Self::resolve_layout(&mut model); } + // With a font engine installed ([`Self::set_text_measurer`]) the VM closes the measure + // round-trip itself, right here — solve, measure what the solve revealed, solve again. + // That is the same two-solve shape the host's own drive loop already runs, moved inside so + // a FontString's box is right in the frame its text was set rather than the frame after. + // Without an engine this is a no-op and the host's batch pass stays the only answer. + if self.fill_measures() { + let mut model = self.model_mut(); + Self::resolve_layout(&mut model); + } event::fire_size_changes(&self.lua); } @@ -880,80 +890,6 @@ impl UiScript { model.touch_layout(); } - /// Feed the number font's per-digit advance widths (logical px, `'0'..='9'` in order) as the - /// Lua global table `BENILLA_DIGIT_W` (keyed by digit *string*, `["0"].."["9"]`). - /// - /// The synchronous half of the money layout's metrics. The real `SmallMoneyFrame` calls - /// `GetTextWidth` mid-update (MoneyFrame.lua l.202) and the real client answers it inline; - /// our `GetStringWidth` is served from the measure round-trip, which lands at extract — a frame - /// AFTER the tick that set the text. A kit that sizes itself from its own label therefore reads - /// 0 the first time and stays wrong until something redraws it. - /// - /// The sum lives in [`Self::publish_number_width`] beside this, not in XML: its two callers are - /// in different shipped files and no third file is a dependency of both. - pub fn set_digit_advances(&mut self, advances: &[f32; 10]) { - self.model_mut().digit_advances = Some(*advances); - let set = || -> mlua::Result<()> { - let t = self.lua.create_table()?; - for (i, w) in advances.iter().enumerate() { - t.set(i.to_string(), *w)?; - } - self.lua.globals().set("BENILLA_DIGIT_W", t) - }; - if let Err(e) = set() { - self.model_mut() - .errors - .push(format!("set_digit_advances: {e}")); - } - } - - /// Publish `BenillaNumberWidth(n)` — the laid-out width of a number in NumberFontNormal, the - /// sum of its digits' advances from the [`Self::set_digit_advances`] feed. - /// - /// **Exact, not an approximation.** The client's width law is a floor-and-add over a per-glyph - /// advance with no kerning term, so a number's width IS the sum of its digits'. That is equally - /// why it stops at numbers: arbitrary text still needs the real measure, and a caller that wants - /// one still waits a frame. - /// - /// **Published from here rather than written in XML** — which is where it started, and the move - /// is the point. Its callers are `BenillaMoney_Set` (MerchantFrame.xml) and `ShowCoin` - /// (MoneyFrame.xml), and there is no third shipped file both of them load: fixtures take - /// arbitrary subsets, so whichever file owned it, some fixture met it as nil. Seated at VM - /// construction it is simply always there, and the headless fallback lives in one place. - /// - /// The 7px fallback is for a VM whose host never fed the advances — every engine test, since - /// there is no font atlas without the app. In-app the feed lands on the first frame the atlas - /// exists. - fn publish_number_width(&self) -> mlua::Result<()> { - let f = self.lua.create_function(|lua, n: mlua::Value| { - // `tostring(n)` semantics: the callers pass a number, but a string is as valid an - // argument here as it is to the Lua original this replaces. - let s = match &n { - mlua::Value::Integer(i) => i.to_string(), - mlua::Value::Number(x) => { - if *x == x.trunc() && x.is_finite() { - format!("{}", *x as i64) - } else { - x.to_string() - } - } - mlua::Value::String(s) => s.to_str()?.to_string(), - _ => String::new(), - }; - let model = lua.app_data_ref::().expect("model"); - let adv = model.digit_advances; - Ok(s.chars() - .map(|c| match (c.to_digit(10), adv) { - (Some(d), Some(a)) => a[d as usize], - // A non-digit (a minus sign, a separator) has no advance in the feed, and - // neither does anything before the host has fed one. - _ => 7.0, - }) - .sum::()) - })?; - self.lua.globals().set("BenillaNumberWidth", f) - } - // ── Input: pointer-leaves-window cleanup (decision 0216 §3; the hit-test/mouse dispatch that // used to sit here now lives in [`pointer`]) ───────────────────────────────────────────────── diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 23c6394c..f0e520e0 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -24,10 +24,10 @@ pub(crate) struct Model { pub(crate) addons: Vec, /// The AddOns folder, so `LoadAddOn` can read an addon's files from inside a Lua binding. pub(crate) addons_root: Option, - /// NumberFontNormal's per-digit advances, `'0'..='9'` ([`super::UiScript::set_digit_advances`]) - /// — the synchronous stand-in for a text measure, read by the `BenillaNumberWidth` binding. - /// `None` until the host feeds them, which is every VM without a font atlas (every engine test). - pub(crate) digit_advances: Option<[f32; 10]>, + /// The host's font engine ([`super::UiScript::set_text_measurer`]) — what lets a metric read + /// answer inside the Lua call that asked, instead of a frame later. `None` in every VM without + /// a font atlas, which is the measure round-trip's own world and behaves exactly as it did. + pub(crate) measurer: Option>, /// Where per-addon saved variables live: the account-scoped folder, then this character's. /// Both are directories holding one `.lua` per declaring addon. pub(crate) addons_saved_account: Option, @@ -250,7 +250,7 @@ pub(crate) struct Model { /// The channels this session has CONFIRMED joining, in join order — the client-side number /// law `GetChannelName` answers with ([`super::channel`]). Mirrored from `ui_chat`'s /// `ChannelState` by `set_joined_channels`; empty until the server's first YOU_JOINED. - pub(crate) joined_channels: Vec, + pub(crate) joined_channels: Vec>, pub(crate) party: party::PartyState, /// Party/loot intents (`AcceptGroup`/`InviteToParty`/`SetLootMethod`/…) queued since the /// app's last [`super::UiScript::take_party_requests`] drain — the outbound seam ([`party`]). @@ -1014,7 +1014,7 @@ impl Model { Model { addons: Vec::new(), addons_root: None, - digit_advances: None, + measurer: None, addons_saved_account: None, addons_saved_character: None, framexml_templates: Default::default(), diff --git a/crates/benilla-ui/src/script/region.rs b/crates/benilla-ui/src/script/region.rs index 90462716..e4151109 100644 --- a/crates/benilla-ui/src/script/region.rs +++ b/crates/benilla-ui/src/script/region.rs @@ -646,6 +646,9 @@ pub(super) fn region_set_point( /// live in `region::text` and `GetWidth`/`GetHeight` in `region::layout`, and both read it. pub(super) fn measured_wh(lua: &Lua, this: &Table) -> mlua::Result<(f32, f32)> { let rh = region_handle_of(lua, this)?; + // Same-tick measure when a host font engine is installed — see `region::text`'s `natural_w`. + // A no-op for a Texture (not a FontString) and for an already-current measure. + super::measure::ensure_measured(lua, rh); let model = lua.app_data_ref::().expect("model"); let d = model.region_data.get(&rh); // The key carries the owner's effective_scale ([`RegionData::measure_key`]) — the same diff --git a/crates/benilla-ui/src/script/region/text.rs b/crates/benilla-ui/src/script/region/text.rs index 26160e52..ae9a458e 100644 --- a/crates/benilla-ui/src/script/region/text.rs +++ b/crates/benilla-ui/src/script/region/text.rs @@ -100,6 +100,11 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // macro window's character tab changing width every frame). `0` until measured, as ever. fn natural_w(lua: &Lua, this: &Table) -> mlua::Result { let rh = region_handle_of(lua, this)?; + // Measure NOW if a host font engine is installed — the reference answers this getter from + // its font engine inline (`0x79e510` → `0x772890`), and a same-tick `SetText` → + // `GetStringWidth` is the corpus's own idiom (`Bagnon_Forever/database/ui.lua:58-59`). + // Without an engine this is a no-op and the number below stays 0 until the round-trip. + crate::script::measure::ensure_measured(lua, rh); let model = lua.app_data_ref::().expect("model"); let Some(d) = model.region_data.get(&rh) else { return Ok(0.0); diff --git a/crates/benilla-ui/src/script/tests/channel.rs b/crates/benilla-ui/src/script/tests/channel.rs index 1ccd43a3..e6a94817 100644 --- a/crates/benilla-ui/src/script/tests/channel.rs +++ b/crates/benilla-ui/src/script/tests/channel.rs @@ -11,7 +11,7 @@ use super::common::script; /// YOU_JOINED. Join ORDER is the whole numbering law, so the fixture is deliberately not sorted. fn joined() -> crate::script::UiScript { let mut s = script(); - s.set_joined_channels(vec!["World".into(), "Trade - City".into()]); + s.set_joined_channels(vec![Some("World".into()), Some("Trade - City".into())]); s } diff --git a/crates/benilla-ui/src/script/tests/measure.rs b/crates/benilla-ui/src/script/tests/measure.rs index bf425690..e0912343 100644 --- a/crates/benilla-ui/src/script/tests/measure.rs +++ b/crates/benilla-ui/src/script/tests/measure.rs @@ -340,3 +340,153 @@ fn get_string_width_is_the_natural_width_not_the_box() { "the string did not change, so neither did its natural width" ); } + +// ───────────────────────────────────────────────────────────────────────────────────────────── +// The SYNCHRONOUS measure — a host font engine installed into the VM ([`TextMeasure`]) +// ───────────────────────────────────────────────────────────────────────────────────────────── + +/// A stand-in font engine: every character is `PER_CHAR` wide and every line `LINE_H` tall, wrapped +/// greedily at the request's wrap width. Deliberately not the real one — what these tests are about +/// is *when* the answer arrives, and a fake with arithmetic anyone can do in their head is what +/// makes the assertions readable. The real engine's own numbers are the app's to test. +struct BlockFont; + +const PER_CHAR: f32 = 7.0; +const LINE_H: f32 = 14.0; + +impl TextMeasure for BlockFont { + fn measure(&mut self, req: &MeasureRequest) -> (f32, f32, f32) { + let natural = req.text.chars().count() as f32 * PER_CHAR; + match req.wrap_width { + Some(w) if w > 0.0 && natural > w => { + let rows = (natural / w).ceil(); + (w, rows * LINE_H, natural) + } + _ => (natural, LINE_H, natural), + } + } +} + +/// **The director's bug, at the engine.** `SetText` then `GetStringWidth` in ONE tick must answer +/// with the string's width — the shape `Bagnon_Forever/database/ui.lua:58-59` writes over every +/// saved character (`button:SetText(player)` then `button:GetTextWidth() + 40`), and the shape the +/// reference's own `SmallMoneyFrame` writes (MoneyFrame.lua l.202). +/// +/// Without an engine installed this is 0 until the host's round-trip lands at extract — a frame +/// later. That is what sized Bagnon's character dropdown to 40px with seven names hanging out of it. +#[test] +fn a_width_read_in_the_tick_that_set_the_text_is_not_zero() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.set_text_measurer(Box::new(BlockFont)); + s.run( + r#" + local f = CreateFrame("Frame", "Host") + f:SetPoint("TOPLEFT"); f:SetSize(400, 300) + local fs = f:CreateFontString("Label", "ARTWORK") + fs:SetPoint("TOPLEFT") + -- the corpus idiom: set, then measure, in one statement sequence + fs:SetText("Onewarrior") + Answer = fs:GetStringWidth() + "#, + ) + .unwrap(); + assert_eq!( + s.eval::("return Answer").unwrap(), + "Onewarrior".len() as f32 * PER_CHAR, + "GetStringWidth must answer inside the tick that set the text" + ); +} + +/// The same read WITHOUT an engine stays 0 and stays served by the round-trip — the pre-existing +/// behaviour, still exactly itself. Every engine test and every headless run is this VM, so the +/// synchronous path may never be a correctness precondition for anything. +#[test] +fn with_no_engine_installed_the_round_trip_is_still_the_only_answer() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + local f = CreateFrame("Frame", "Host") + f:SetPoint("TOPLEFT"); f:SetSize(400, 300) + local fs = f:CreateFontString("Label", "ARTWORK") + fs:SetPoint("TOPLEFT") + fs:SetText("Onewarrior") + Answer = fs:GetStringWidth() + "#, + ) + .unwrap(); + assert_eq!(s.eval::("return Answer").unwrap(), 0.0); + assert_eq!( + s.fontstrings_needing_measure().len(), + 1, + "and the request is still queued for the host" + ); +} + +/// A FontString's **box** is right in the frame its text was set, not the frame after: `resolve` +/// closes the round-trip itself when an engine is installed, so a caller reading `GetWidth()` (the +/// laid-out extent, not the natural one) gets this string's number and not the previous string's. +#[test] +fn resolve_closes_the_round_trip_when_an_engine_is_installed() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.set_text_measurer(Box::new(BlockFont)); + s.run( + r#" + local f = CreateFrame("Frame", "Host") + f:SetPoint("TOPLEFT"); f:SetSize(400, 300) + local fs = f:CreateFontString("Label", "ARTWORK") + fs:SetPoint("TOPLEFT") + fs:SetWidth(35) -- a declared width ⇒ the two extents differ + fs:SetText("Onewarrior") -- 70 natural, wraps to 2 rows inside 35 + "#, + ) + .unwrap(); + s.resolve(); + assert!( + s.fontstrings_needing_measure().is_empty(), + "the engine answered its own request during resolve" + ); + assert_eq!( + s.eval::>( + "return { Label:GetWidth(), Label:GetHeight(), Label:GetStringWidth() }" + ) + .unwrap(), + vec![35.0, LINE_H * 2.0, 70.0], + "laid-out extent from the box, natural width unwrapped — the two must not be conflated" + ); +} + +/// The same-tick measure and the batch round-trip write the **same cache**, so asking twice costs +/// one measure — and a caller polling `GetStringWidth` every frame does not re-measure a string +/// that has not changed. +#[test] +fn a_synchronous_measure_satisfies_the_batch_request_too() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.set_text_measurer(Box::new(BlockFont)); + s.run( + r#" + local f = CreateFrame("Frame", "Host") + f:SetPoint("TOPLEFT"); f:SetSize(400, 300) + local fs = f:CreateFontString("Label", "ARTWORK") + fs:SetPoint("TOPLEFT") + fs:SetText("hello") + Answer = fs:GetStringWidth() + "#, + ) + .unwrap(); + assert_eq!(s.eval::("return Answer").unwrap(), 5.0 * PER_CHAR); + assert!( + s.fontstrings_needing_measure().is_empty(), + "the Lua read already filled the cache the batch pass keys on" + ); + // …and a new string invalidates it exactly as it always did. + s.run("Label:SetText('a longer label')").unwrap(); + assert_eq!( + s.eval::("return Label:GetStringWidth()").unwrap(), + "a longer label".len() as f32 * PER_CHAR, + "a stale measure is not this string's metric" + ); +} diff --git a/crates/benilla-world/src/terrain_stream.rs b/crates/benilla-world/src/terrain_stream.rs index b6602483..7953f4cb 100644 --- a/crates/benilla-world/src/terrain_stream.rs +++ b/crates/benilla-world/src/terrain_stream.rs @@ -329,6 +329,20 @@ impl ViewFocus { pub(crate) fn body_pos(&self) -> Option<[f32; 3]> { self.body } + + /// Is there a body **and** is the world under it the one it is standing in? + /// + /// The area authority's gate (1287) — the same bit [`ViewFocus::paced`] carries, read through + /// its own name because it answers a different question here. Through entry, a teleport and a + /// world swap the body has been snapped but its destination is still streaming, so the leaf + /// area under it is whatever tile happens to be resident: at first login that is the pre-snap + /// position's — the map centre, Eastern Plaguelands on map 0 and The Barrens on map 1 — and a + /// beat later the outdoor parent of an interior whose WMO claim has not landed yet. Publishing + /// those makes the client announce, play the music of, and join the channels of zones the + /// player was never in. + pub(crate) fn body_settled(&self) -> bool { + self.body.is_some() && self.paced + } } /// **Where the world streams from when nothing else can say** — the Human start (Northshire), diff --git a/crates/benilla-world/src/terrain_stream/queries.rs b/crates/benilla-world/src/terrain_stream/queries.rs index 896ed21d..542c7318 100644 --- a/crates/benilla-world/src/terrain_stream/queries.rs +++ b/crates/benilla-world/src/terrain_stream/queries.rs @@ -135,6 +135,13 @@ pub(super) fn update_current_area( let Some(wow) = focus.body_pos() else { return; // no avatar — the area authority follows the character, and there is none }; + if !focus.body_settled() { + // …and one whose own world is still arriving has no area worth publishing: the leaf under + // it is not yet the leaf it is standing in. Hold the last real answer instead — the + // reference is behind a loading screen for exactly this window + // ([`ViewFocus::body_settled`], 1287). + return; + } // WMO interior first: the player's down-ray group resolved to its WMOAreaTable world area. let interior_area = interior.0.zip(wmo_areas.as_ref()).and_then(|(k, cat)| { cat.0