diff --git a/crates/benilla-app/src/capture/probes/visual_census.rs b/crates/benilla-app/src/capture/probes/visual_census.rs index 03b3d4e2..d5461785 100644 --- a/crates/benilla-app/src/capture/probes/visual_census.rs +++ b/crates/benilla-app/src/capture/probes/visual_census.rs @@ -13,7 +13,7 @@ //! One line per streamed entity says which, without an eye: //! //! ```text -//! UVIS 0xF130003A7200013B Unit display=13069 d=6.4 cube=1 meshes=0 held=[] Zandalarian Event Generator +//! UVIS 0xF130003A7200013B Unit display=13069 d=6.4 cube=1 meshes=0 pick=1 held=[] Zandalarian Event Generator //! ``` //! //! - **`cube`** is the [`FallbackCube`] marker — literally the arm that spawned it, not a guess @@ -25,8 +25,15 @@ //! `meshes` counts *direct* children and an attached model is a grandchild, so a unit whose //! whole visible self is a held weapon — the Naxxramas `Unholy Axe`, an `InvisibleStalker` body //! with an axe in its hand — reads `meshes=0` whether the axe is there or not. That blind spot -//! is what let decision 1618's defect sit under a green census; `held=[main]` is the line that +//! is what let decision 1656's defect sit under a green census; `held=[main]` is the line that //! says the weapon arrived. +//! - **`pick`** counts the parts the **mouseover** would actually ray-test — every skinned part +//! across the unit's chained model roots ([`crate::target::hover::pick_model_roots`]: the body, +//! everything it wears, its mount), plus its fallback-box children where it has no skinned parts +//! at all. `pick=0` means the unit cannot be hovered or clicked *at all* and only its name plate +//! can select it — which is the second half of what the Naxxramas weapon mobs reported and what +//! neither `meshes` nor `held` could say: `held=[main]` proves the axe is *attached*, never that +//! it is *clickable* (decision 1658). //! - **`pending`** marks an entity whose visual has not been built yet (a model still streaming) — //! never to be confused with one that built nothing, which is the distinction the census exists //! to keep. @@ -51,6 +58,7 @@ use super::ProbeClock; use crate::entities::{FallbackCube, HeldAttached, VisualAttached, ATTACH_SLOT_NAMES}; use crate::names::NameCache; use crate::net::{Guid, NetEntity, SelfPlayer}; +use crate::target::hover::pick_model_roots; /// How far from the body the census looks, in yards. Comfortably past the server's own creature /// visibility radius for a spot the operator has just `.go`ne to, so "nothing found" means the @@ -92,6 +100,7 @@ struct UnitVisuals { /// What the census reads per entity: identity, kind + display, pose, and whether its visual has /// been built yet. type VisualQuery = ( + Entity, &'static Guid, &'static NetEntity, &'static Transform, @@ -100,10 +109,13 @@ type VisualQuery = ( // The attach slots standing under this unit — the visual truth for everything that rides a // BONE rather than the entity, which `Children` above cannot see. Option<&'static HeldAttached>, + // …and its mount, the third chained-model root the pick offers (decision 0441). + Option<&'static crate::entities::mount::MountChild>, ); /// One line per streamed entity within [`UnitVisuals::radius`] of the body — cubes first, then /// everything else — under a summary line naming the count that matters. +#[allow(clippy::too_many_arguments)] // one Bevy system's full input set fn fire_unit_visuals( mut probe: ResMut, time: ProbeClock, @@ -112,6 +124,11 @@ fn fire_unit_visuals( entities: Query, cubes: Query<(), With>, meshes: Query<(), With>, + // The pick's own populations, read exactly as `update_hover` reads them: the skinned parts of + // each chained model root (pass 1/2) and, for a unit with none, its fallback-box children. + child_sets: Query<&Children>, + rig_parts: Query<(), With>, + box_parts: Query<(), With>, ) { let now = time.elapsed_secs(); if probe.next <= 0.0 || now < probe.next { @@ -133,7 +150,7 @@ fn fire_unit_visuals( let mut rows: Vec<(bool, i64, String)> = Vec::new(); let (mut cube_n, mut blank_n, mut pending_n) = (0u32, 0u32, 0u32); let mut cube_displays: Vec = Vec::new(); - for (guid, net, t, children, attached, held) in &entities { + for (entity, guid, net, t, children, attached, held, mount) in &entities { if matches!(net.kind, EntityKind::DynamicObject | EntityKind::Other) || t.translation.distance_squared(body.translation) > radius2 { @@ -158,6 +175,24 @@ fn fire_unit_visuals( } else if mesh_n == 0 { blank_n += 1; } + // What the mouseover would test. The enumeration is the picker's own + // ([`pick_model_roots`]) so the census can never report a set the pick doesn't use; the + // fallback-box leg mirrors `update_hover`'s AABB path, which owns exactly the units the + // skinned path does not. + let worn = held.map_or(&[][..], |h| h.spawned_slots().as_slice()); + let mut pick_n: u32 = pick_model_roots(entity, worn, mount.map(|m| m.0)) + .filter_map(|root| child_sets.get(root).ok()) + .flat_map(|kids| kids.iter()) + .filter(|kid| rig_parts.contains(*kid)) + .count() as u32; + if pick_n == 0 { + pick_n = children + .map(|c| c.iter()) + .into_iter() + .flatten() + .filter(|kid| box_parts.contains(*kid)) + .count() as u32; + } let dist = t.translation.distance(body.translation); let held: Vec<&str> = held .map(|h| { @@ -173,7 +208,7 @@ fn fire_unit_visuals( (dist * 100.0) as i64, format!( "UVIS {:#018x} {:<12} display={display:<6} d={dist:6.1} cube={} meshes={mesh_n:<3} \ - held=[{:<12}] {:<8} {}", + pick={pick_n:<3} held=[{:<12}] {:<8} {}", guid.0, format!("{:?}", net.kind), u8::from(cube), diff --git a/crates/benilla-app/src/debug_panel/mod.rs b/crates/benilla-app/src/debug_panel/mod.rs index 61d14c6e..45f1ce33 100644 --- a/crates/benilla-app/src/debug_panel/mod.rs +++ b/crates/benilla-app/src/debug_panel/mod.rs @@ -266,7 +266,28 @@ fn feed_gated_egui_output( fn spawn_egui_camera(mut commands: Commands) { commands.spawn(( PrimaryEguiContext, // `#[require(EguiContext)]` ⇒ this camera carries EguiContext + // Named so [`crate::preflight`]'s MSAA check can say WHICH cameras disagree — an + // entity id in that error is a lookup, a name is the answer (decision 1659). + Name::new("egui dev-overlay camera"), Camera2d, + // **No MSAA — and saying so is what keeps the panel openable at all** (decision 1659). + // Silence here does not mean "none": `Camera` requires `Msaa`, whose `Default` is + // `Sample4`, so this camera used to carry four samples without ever naming them. That was + // invisible while the player-UI camera silently carried four too — and became a fatal + // `Attachments have differing sample counts` the moment 1628 gave that one `Msaa::Off`, + // because Bevy's two prepare passes disagree about whether MSAA is part of a texture's + // identity: `prepare_view_targets` keys the COLOUR target on `(target, usage, hdr, msaa)`, + // while `core_2d::prepare_core_2d_depth_textures` keys the DEPTH texture on the target + // ALONE (`core_3d`'s equivalent does include it — the asymmetry is 2D-only). Two + // `Camera2d`s on one window then share one depth texture at whichever sample count the + // first-iterated camera stamped on it, and the other one's pass is invalid. + // + // Off is also simply right here: egui feathers its own edges in the tessellator, so + // multisampling this overlay buys nothing and costs a full-window 4× colour texture and 4× + // the fill — the same arithmetic as 1628. [`crate::preflight`] now checks the agreement + // every run, so the next camera that stays silent is named in the log instead of in a wgpu + // validation error that names no camera at all. + bevy::render::view::Msaa::Off, RenderLayers::none(), Camera { order: 2, diff --git a/crates/benilla-app/src/entities/attach/mod.rs b/crates/benilla-app/src/entities/attach/mod.rs index b628f11c..1d91d68d 100644 --- a/crates/benilla-app/src/entities/attach/mod.rs +++ b/crates/benilla-app/src/entities/attach/mod.rs @@ -61,7 +61,7 @@ pub(super) use redress::redress_player_looks; /// `skins` is whether anything will actually **skin through this slot** — the same law the item /// lane's rider states (`equipment::spawn`: "a slot only earns its keep if something will /// actually SKIN through it"). A body that drew no batch at all (the invisible trigger creature -/// holding a visible weapon — decision 1618) still needs the *pose*, because its attachment +/// holding a visible weapon — decision 1656) still needs the *pose*, because its attachment /// points carry what IS drawn, but it has nothing of its own to skin: it takes the slot-less rig /// [`benilla_world::rig_anim::finalize_rig_worlds`] has supported since 1365, and no `RigStarved` /// marker either (there is no starvation to heal). @@ -327,7 +327,7 @@ pub(super) fn attach_entity_visuals( // rather than flash a cube we'd swap out. // // **The gate is "did the display name a model", not "did it build a batch"** (decision - // 1618). A model that named a file and built ZERO batches is a *loaded model instance that + // 1656). A model that named a file and built ZERO batches is a *loaded model instance that // draws nothing* — the reference builds its `CM2Model` like any other and the per-batch // loop simply has no trip to make — and everything an entity takes from its model but its // pixels comes from that instance: the skeleton, the attachment points a held weapon rides, @@ -465,7 +465,7 @@ pub(super) fn attach_entity_visuals( .is_ok_and(|s| crate::go_anim::go_animates(s.0.gameobject_type_id())); // Will anything actually SKIN through this instance's palette slot? Every M2 part // carries a skinned twin, so for an ordinary body this is simply "it drew something"; - // a body that drew NOTHING (decision 1618's invisible weapon-holder) gets the rig + // a body that drew NOTHING (decision 1656's invisible weapon-holder) gets the rig // without the slot — see [`setup_skinned_instance`]. let skins = parts.iter().any(|p| p.skinned_mesh.is_some()); let mut skin: Option = match (net.kind, dm) { diff --git a/crates/benilla-app/src/minimap/composite.rs b/crates/benilla-app/src/minimap/composite.rs index 86dfe2bf..24b9581c 100644 --- a/crates/benilla-app/src/minimap/composite.rs +++ b/crates/benilla-app/src/minimap/composite.rs @@ -123,6 +123,7 @@ pub(super) fn setup_composite( let camera = commands .spawn(( + Name::new("minimap composite camera"), Camera2d, // **No MSAA** — the client's target is a plain RGBA8 surface with none, and here it is // load-bearing rather than cosmetic: multisampling an alpha-TESTED edge hands back diff --git a/crates/benilla-app/src/preflight.rs b/crates/benilla-app/src/preflight.rs index c36f5649..d47337de 100644 --- a/crates/benilla-app/src/preflight.rs +++ b/crates/benilla-app/src/preflight.rs @@ -32,6 +32,12 @@ //! it: that one waits on [`EnteredWorldMessage`], which needs a server. So this notice fires at //! **startup**, before anything can be concluded from the run. //! +//! 5. **Two `Camera2d`s on one target disagree about MSAA** ([`camera_2d_msaa_agrees`], decision +//! 1659). Not an avatar state, but the same shape as the four above: it is invisible until it +//! kills the frame, and when it does, wgpu's `Attachments have differing sample counts` names +//! two numbers and no camera. One scan on the frame the cameras spawn turns that into a line +//! that names them. +//! //! The banner is **not env-gated**: a warning nobody knows to switch on is not a warning. It costs //! one line per world entry when everything is fine, and it re-fires on every re-entry (a relog, a //! `.character race` forced logout, a reconnect) because the state can have changed. @@ -47,7 +53,12 @@ //! [`crate::run_mode`] now, not here: it is consulted by the login policy, and 1174's seam does not //! let gameplay call an instrument. Its reasoning went with it. +use std::collections::HashMap; + +use bevy::camera::{NormalizedRenderTarget, RenderTarget}; use bevy::prelude::*; +use bevy::render::view::Msaa; +use bevy::window::PrimaryWindow; use crate::area::AreaTableRes; use crate::names::NameCache; @@ -111,6 +122,7 @@ impl Plugin for PreflightPlugin { fn build(&self, app: &mut App) { app.init_resource::() .add_systems(Startup, offline_notice) + .add_systems(Update, camera_2d_msaa_agrees) .add_systems( Update, report_session.after(benilla_world::schedule::WorldStage::Net), @@ -137,6 +149,71 @@ fn offline_notice(net: Option>) { ); } +/// **Every `Camera2d` sharing a render target must carry the same `Msaa`** — checked out loud on +/// the frame they spawn, because the failure is fatal, immediate, and names nobody (decision 1659). +/// +/// Bevy's two prepare passes disagree about whether the sample count is part of a texture's +/// identity. `view::prepare_view_targets` keys the **colour** target on `(target, usage, hdr, +/// msaa)`, so every camera gets an attachment at its own count. `core_2d:: +/// prepare_core_2d_depth_textures` keys the **depth** texture on `camera.target` *alone* and stamps +/// it with whichever camera the hash map reaches first — `core_3d`'s equivalent keys on `(target, +/// msaa)`, so the asymmetry is 2D-only and there is nothing here for `Camera3d`. Two `Camera2d`s on +/// one window at different counts therefore share one depth texture, and the one that did not +/// create it opens its pass with mismatched attachments: `Attachments have differing sample +/// counts: the depth attachment's texture view has count N but ... count M`. Two numbers, no +/// camera, no system, no frame — a panic in `render_system` on a thread called `Compute Task Pool`. +/// +/// It costs a session an afternoon of source archaeology, and it is one query to catch. The trap +/// that produces it is silence: `Camera` requires `Msaa` and `Msaa::default()` is `Sample4`, so a +/// camera that never mentions multisampling does not get none, it gets four (1628 found that +/// costing a full-window 4× texture on the player-UI camera; 1659 found the same silence on the +/// egui overlay camera crashing the debug panel outright, one commit after 1628 broke the tie that +/// had been hiding it). So the error below says *which* camera and *what* count, and names the +/// likely cause. +/// +/// Gated on `Added` rather than run once at `PostStartup`: our three 2D cameras all spawn +/// in `Startup` today, but a fourth added later must be checked too, and the guard makes the +/// steady-state cost one empty query. +fn camera_2d_msaa_agrees( + added: Query<(), (With, Added)>, + cams: Query<(Entity, &RenderTarget, &Msaa, Option<&Name>), With>, + primary: Query>, +) { + if added.is_empty() { + return; + } + let primary = primary.single().ok(); + let mut first: HashMap, u32)> = HashMap::new(); + for (entity, target, msaa, name) in &cams { + // A target that will not normalize has no swapchain yet and no depth texture either. + let Some(key) = target.normalize(primary) else { + continue; + }; + let label = name.map(|n| n.as_str().to_owned()); + let samples = msaa.samples(); + let Some((other, other_label, other_samples)) = first.get(&key) else { + first.insert(key, (entity, label, samples)); + continue; + }; + if *other_samples == samples { + continue; + } + error!( + "preflight: {} ({other}) renders {}x MSAA and {} ({entity}) renders {}x, both to the \ + SAME target ({key:?}) — one of them will die with `Attachments have differing sample \ + counts` the frame it goes active. Bevy keys the Core2d DEPTH texture on the target \ + alone (core_2d::prepare_core_2d_depth_textures) but the COLOUR target on the sample \ + count too, so they share one depth attachment and cannot share a colour one. Usually \ + the cause is a camera that never named an Msaa: silence is Sample4, not off \ + (decisions 1628, 1659).", + other_label.as_deref().unwrap_or("an unnamed Camera2d"), + other_samples, + label.as_deref().unwrap_or("an unnamed Camera2d"), + samples, + ); + } +} + /// The banner's once-per-entry latch: armed by [`EnteredWorldMessage`], disarmed when the report /// goes out (or when the wait expires). #[derive(Resource, Default)] diff --git a/crates/benilla-app/src/target/hover.rs b/crates/benilla-app/src/target/hover.rs index e5d9c9e5..1e980b4b 100644 --- a/crates/benilla-app/src/target/hover.rs +++ b/crates/benilla-app/src/target/hover.rs @@ -14,6 +14,7 @@ use bevy::prelude::*; use bevy::window::PrimaryWindow; use crate::creature_anim::AnimDriver; +use crate::entities::HeldAttached; use crate::net::{Guid, NetEntity, ObjectStore, SelfPlayer}; use crate::player::CameraControl; use crate::ui_script::PointerOverUi; @@ -68,6 +69,30 @@ pub(super) fn update_pick_occlusion( } } +/// Every model instance whose parts are `unit`'s pick geometry: **the body, everything it wears, +/// and its mount** (decision 1658; the mount leg is 0441's). +/// +/// This is the reference's chained-model set. `0x480d90` registers a pick candidate by walking the +/// object's CM2 **attachment tree** — `[model+0x1dc]` list head → `[+0x1e4]` sibling, the same two +/// fields the dress and paperdoll lanes walk — and calling the registrar `0x713cb0` on *each* model +/// it finds, all under one candidate node. So the ray tests the held axe's own triangles and the +/// hit comes back as the unit holding it (wow-re `object-layer/scratch/selection-circle.md` §3 + +/// `ui/scratch/paperdoll-liveness-law.md`). +/// +/// Takes the worn roots as a plain slice rather than the `HeldAttached` that supplies them, so the +/// rule is testable without building one. `pub(crate)` for the **census** +/// ([`crate::capture::probes`]'s `pick=` column), which must count what the picker will actually +/// test and not a second opinion about it. +pub(crate) fn pick_model_roots( + unit: Entity, + worn: &[Option], + mount: Option, +) -> impl Iterator + use<'_> { + std::iter::once(unit) + .chain(worn.iter().flatten().copied()) + .chain(mount) +} + /// Recompute the unit under the cursor each frame — the real client's two-phase pick (wow-re /// pick-volume RE `bd630be` + `31562f1d`): **broad** = the cursor ray vs the *current animation's* /// bounds sphere (world-placed + world-scaled, no pad); **pass 1** = the ray vs the unit's **posed @@ -79,6 +104,11 @@ pub(super) fn update_pick_occlusion( /// alive unit beats a dead one even when farther, ties by distance. A unit with no skinned parts /// (cube fallback) keeps the interim render-mesh AABB test, competing at pass-1 level. Inert while /// mouse-looking (cursor hidden) or over the dev UI. +/// +/// **A unit's pick geometry includes what it wears** (decision 1658): every model chained to the +/// body — held weapons, the helm, the shoulders, a mount — is its own pass-1/pass-2 candidate +/// resolving to the same unit, because the reference registers the whole CM2 attachment tree into +/// the pick scene under one candidate node (`0x480d90` walking `[model+0x1dc]`/`[+0x1e4]`). #[allow(clippy::type_complexity, clippy::too_many_arguments)] pub(super) fn update_hover( camera: Query<(&Camera, &GlobalTransform), With>, @@ -117,6 +147,10 @@ pub(super) fn update_hover( Option<&Children>, Option<&crate::entities::mount::MountChild>, Option<&InheritedVisibility>, + // What this unit WEARS — the spawned root per attach slot. Each is a chained model + // and therefore pick geometry of its wearer (see the system doc); read here rather + // than by a query of its own because this system sits at Bevy's 16-param ceiling. + Option<&HeldAttached>, ), (With, Without), >, @@ -178,7 +212,7 @@ pub(super) fn update_hover( // Units the faithful path *owns* (they have skinned parts): excluded from the AABB fallback // even when the broad phase rejects them — the reference wouldn't click them there either. let mut faithful: HashSet = HashSet::new(); - for (entity, gt, net, anims, drv, store, children, mount_child, drawn) in &roots { + for (entity, gt, net, anims, drv, store, children, mount_child, drawn, held) in &roots { if !matches!(net.kind, EntityKind::Unit | EntityKind::Player) { continue; } @@ -190,12 +224,23 @@ pub(super) fn update_hover( continue; } let Some(children) = children else { continue }; + // The models this unit WEARS, each its own pick candidate below. The reference registers + // every model **chained** to the object, not just its body: `0x480d90` walks the CM2 + // attachment tree (`[model+0x1dc]` list head → `[+0x1e4]` sibling — the same two fields + // the dress and paperdoll lanes walk) and registers each child model into the pick scene + // under the SAME candidate node, so a ray that strikes the axe resolves to the unit + // holding it (wow-re `object-layer/scratch/selection-circle.md` §3 + + // `ui/scratch/paperdoll-liveness-law.md`; decision 1658). Ours tested the body alone, + // which is why a Naxxramas weapon mob — an `InvisibleStalker` body that draws nothing, + // whose entire visible self is the weapon in its hand — could only be targeted through + // its name plate. + let worn = held.map_or(&[][..], |h| h.spawned_slots().as_slice()); // Ownership probe only — no collect yet. The `skinned` Vec used to be built HERE, for // every drawn unit in the scene, ahead of the few-FLOP broad phase that rejects nearly // all of them (1473 §3's hover row); the survivors alone pay it now, below. Membership // in `faithful` must still be decided pre-reject — a broad-phase-rejected skinned unit // stays excluded from the AABB fallback by RULE, not by accident. - if !children.iter().any(|c| parts.contains(c)) { + if !children.iter().any(|c| parts.contains(c)) && worn.iter().all(Option::is_none) { continue; // static/cube unit → the AABB fallback below } faithful.insert(entity); @@ -220,7 +265,6 @@ pub(super) fn update_hover( continue; } } - let skinned: Vec<_> = children.iter().filter_map(|c| parts.get(c).ok()).collect(); // The posed joint palette (parts of one skeleton share the rig): the same // world-from-bind-pose matrices GPU skinning applies, read from the owned palette rows // (decision 0720 — last frame's propagated pose, exactly what the previous @@ -232,29 +276,31 @@ pub(super) fn update_hover( palettes.world_palette(rig.slot, rig.bones() as usize) }) }; - let Some(palette) = palette_of(&skinned) else { - continue; - }; - let mesh_ids = skinned.iter().map(|(m, _)| m.id()).collect(); let priority = if store.is_some_and(|s| s.0.unit_is_dead()) { 2 } else { 3 }; - candidates.push((entity, priority, mesh_ids, palette)); - // The mount child's parts join as a second candidate resolving to the SAME unit - // (decision 0441): its own skeleton, its own palette, one hoverable whole. - if let Some(mc) = mount_child { - if let Ok(mc_children) = child_sets.get(mc.0) { - let mount_skinned: Vec<_> = mc_children - .iter() - .filter_map(|c| parts.get(c).ok()) - .collect(); - if let Some(mount_palette) = palette_of(&mount_skinned) { - let mount_mesh_ids = mount_skinned.iter().map(|(m, _)| m.id()).collect(); - candidates.push((entity, priority, mount_mesh_ids, mount_palette)); - } - } + // **One candidate per model instance, every one of them resolving to this unit** — the + // reference's own shape (one candidate node; every chained model registered under it). + // The body contributes nothing when it has no parts, which is not a reason to stop: a + // trigger creature's whole visible self is what it wears. An attached item rides a rigid + // one-frame palette of its own (`RigRider`, 1609), so it reads here exactly like a body — + // its parts, its rig, its pose — and so does a mount child (decision 0441). + for root in pick_model_roots(entity, worn, mount_child.map(|mc| mc.0)) { + let Ok(kids) = child_sets.get(root) else { + continue; + }; + let sk: Vec<_> = kids.iter().filter_map(|c| parts.get(c).ok()).collect(); + let Some(palette) = palette_of(&sk) else { + continue; + }; + candidates.push(( + entity, + priority, + sk.iter().map(|(m, _)| m.id()).collect(), + palette, + )); } } @@ -686,4 +732,49 @@ mod tests { ); assert_eq!(resolved[4], net, "the net entity resolves to itself"); } + + /// **A unit's pick geometry is every model chained to it** — decision 1658, the Naxxramas + /// weapon mobs (`.go xyz 2729.28 -3159.80 267.54 533`). + /// + /// "Unholy Axe" is an `InvisibleStalker` body — 80 bones, 135 animations, zero vertices — + /// holding a real axe on its right-hand attachment. Enumerating only the body left the unit + /// with no pick candidate at all, so the model could not be hovered or clicked and the name + /// plate was the only way to select it. The reference registers each chained model under the + /// same candidate node, which is what makes the axe itself the clickable surface. + #[test] + fn a_units_pick_geometry_is_every_model_chained_to_it() { + let unit = Entity::from_raw_u32(1).unwrap(); + let main_hand = Entity::from_raw_u32(2).unwrap(); + let helm = Entity::from_raw_u32(3).unwrap(); + let mount = Entity::from_raw_u32(4).unwrap(); + + let bare: Vec<_> = pick_model_roots(unit, &[], None).collect(); + assert_eq!(bare, vec![unit], "a unit wearing nothing is just its body"); + + // The empty slots between the filled ones are the real `HeldAttached` shape (main/off/ + // ranged/helm/shL/shR/ammo/quiver), and must not become candidates. + let worn = [ + Some(main_hand), + None, + None, + Some(helm), + None, + None, + None, + None, + ]; + let dressed: Vec<_> = pick_model_roots(unit, &worn, Some(mount)).collect(); + assert_eq!( + dressed, + vec![unit, main_hand, helm, mount], + "body, then every worn model, then the mount — all one unit's geometry" + ); + + // The case that was broken: the body draws nothing, so the axe is the whole hit surface. + let trigger: Vec<_> = pick_model_roots(unit, &worn[..1], None).collect(); + assert!( + trigger.contains(&main_hand), + "a body with no geometry of its own still offers what it holds" + ); + } } diff --git a/crates/benilla-app/src/target/mod.rs b/crates/benilla-app/src/target/mod.rs index 97b33125..f67d6a9d 100644 --- a/crates/benilla-app/src/target/mod.rs +++ b/crates/benilla-app/src/target/mod.rs @@ -57,7 +57,7 @@ pub(crate) mod click; pub(crate) mod cursor_mode; mod flash; mod highlight; -mod hover; +pub(crate) mod hover; pub(crate) mod lock; mod relations; mod reticle; diff --git a/crates/benilla-app/src/target/ring.rs b/crates/benilla-app/src/target/ring.rs index d393c320..a58e3872 100644 --- a/crates/benilla-app/src/target/ring.rs +++ b/crates/benilla-app/src/target/ring.rs @@ -83,8 +83,11 @@ pub(super) struct RingAssets { /// The reference's ground selection-circle texture (`Textures\UnitSelectTexture.blp`, wow-re /// selection-circle RE) — a white ring, sampled top-down, tinted + additively blended below. const RING_TEXTURE: &str = "mpq://textures/unitselecttexture.blp"; -/// Model-local ring radius for a unit with no model (a cube fallback), since it has no M2 footprint. -const RING_FALLBACK_RADIUS: f32 = 0.7; +/// Model-local ring radius for a unit with **no model at all** (a cube fallback), since it has no M2 +/// footprint to measure. The reference's own degenerate-box constant (decision 1658) rather than a +/// number of ours: a body whose box measures zero and a body with no box are the same question, and +/// `0x60aee0` answers it with 1.2. +const RING_FALLBACK_RADIUS: f32 = benilla_formats::DEGENERATE_RING_FOOTPRINT; /// The ring's own palette — **trace + byte verified end-to-end** (wow-re selection-circle §5, the /// `CGUnit::GetSelectionCircleColor` selector `0x605960`, per-object vtable `+0x2c`; the dword is /// written verbatim as every decal vertex's diffuse, alpha 255 — no tint global, no tex-env @@ -241,7 +244,8 @@ pub(super) fn load_factions(mut commands: Commands, world_assets: Option match targets.0.get(target) { Ok((unit, sel_radius, store, net, mount_child)) => { - // A real model uses its own footprint (even if small); only a model-less unit falls back. - // The 0.05 floor just avoids a degenerate (zero-bounds) model rendering an invisible ring. + // A real model uses its own footprint, whatever it measures; only a model-less unit + // falls back. **No floor** — the reference has none, and the one we used to keep + // (0.05) is what a zero-bounds model landed on: `ring_footprint` now carries the + // writer's own degenerate-box answer (1.2) instead of a 0 for the picker to clamp, + // so the Naxxramas weapon mobs ring at 1.2 × 2.25 ≈ 2.7 yd like the reference and + // not at a coin's width (decision 1658). // Mounted, the footprint and the extra scale column come from the mount child // (the `mount_parts` doc above); a still-loading mount rides the fallback the // way any model-less unit does until its bounds land. @@ -313,7 +321,6 @@ pub(super) fn update_ring( Some((mnet, msel)) => (msel.map_or(RING_FALLBACK_RADIUS, |r| r.0), mnet.scale), None => (sel_radius.map_or(RING_FALLBACK_RADIUS, |r| r.0), 1.0), }; - let local = local.max(0.05); let radius = local * (unit.scale.x * mount_scale).max(0.01); *fade_angle = ring_fade_angle(&camera).unwrap_or(*fade_angle); // The rebuild gate (0733 §5): a still target under a still camera keeps the diff --git a/crates/benilla-app/src/ui_pass.rs b/crates/benilla-app/src/ui_pass.rs index a96c0afd..a97615f7 100644 --- a/crates/benilla-app/src/ui_pass.rs +++ b/crates/benilla-app/src/ui_pass.rs @@ -591,6 +591,7 @@ fn demo_enabled() -> bool { fn spawn_ui_camera(mut commands: Commands) { commands.spawn(( PlayerUiCamera, + Name::new("player-UI camera"), Camera2d, // **No MSAA — named, because silence here means 4×.** `bevy_render`'s `CameraPlugin` // registers `Camera` → `Msaa` as a required component (`bevy_render/src/camera.rs:56`) and diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index e1b3c3f3..06ff25a3 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -67,14 +67,21 @@ fn install_boot_vm(world: &mut World) { world.insert_non_send_resource(script); } -/// Wire up the two halves of `Interface\AddOns\` texture art (decision 1322): the sprite -/// decoder's **loose-file root** (so addon-shipped BLP/TGA files render at all — the store's +/// Wire up the halves of `Interface\AddOns\` texture art (decision 1322): the sprite decoder's +/// **loose-file root** (so addon-shipped BLP/TGA files render at all — the store's /// [`benilla_assets::WorldAssets::set_loose_sprite_root`]) and the VM's **texture probe** (so the /// path form of `SetTexture` can answer the reference's 1|nil load verdict — Atlas picks its map /// art by that return). Both resolve the same one folder ([`addons::root`], hermetic-`None` under /// `$WOW_CAPTURE`), and the probe walks the same [`benilla_assets::sprite_candidates`] the /// renderer decodes with, so the verdict the Lua caller gets is the verdict the screen shows. /// +/// The **size probe** beside it is the same oracle answering a different question — how many texels +/// wide and tall is the art — which is what lets a region that authored no size on an axis take +/// that span from its content, as the client's virtual size getters do (decision 1349, the fix for +/// B342's page-sized book crest). It goes through the decoder, so the number layout resolves with +/// is the number the screen shows, and memoises per texture key: the ask is per zero-size region +/// per resolve, and the answer cannot change for a key that already read. +/// /// No `WorldAssets` (no client data) means no backend: nothing to install, and the VM's path form /// keeps answering nil — the engine-less truth. fn install_texture_resolvers(world: &mut World, script: &mut UiScript) { @@ -84,6 +91,8 @@ fn install_texture_resolvers(world: &mut World, script: &mut UiScript) { }; assets.set_loose_sprite_root(root.clone()); let chain = assets.chain.clone(); + let size_chain = chain.clone(); + let size_root = root.clone(); script.set_texture_probe(Box::new(move |path| { benilla_assets::sprite_candidates(path).iter().any(|c| { chain.lock_recover().contains(c) @@ -92,6 +101,21 @@ fn install_texture_resolvers(world: &mut World, script: &mut UiScript) { .is_some_and(|r| benilla_assets::loose_sprite_file(r, c).is_some()) }) })); + // Keyed by the reference string exactly as the region carries it, so the hit path allocates + // nothing: this is asked from inside the layout sweep, once per zero-size textured region per + // pass. Two spellings of one file (`Foo` and `Foo.blp`) cost two entries holding the same + // number, which is cheaper than normalising every ask to avoid it. Misses are cached too — a + // path that resolves to nothing measures nothing however often we look. + let sizes: std::cell::RefCell>> = + std::cell::RefCell::new(std::collections::HashMap::new()); + script.set_texture_size_probe(Box::new(move |path| { + if let Some(cached) = sizes.borrow().get(path) { + return *cached; + } + let measured = benilla_assets::sprite_dimensions(&size_chain, size_root.as_deref(), path); + sizes.borrow_mut().insert(path.to_string(), measured); + measured + })); } /// Does this run want the player UI at all? Captures stay pristine — their baselines regression-test diff --git a/crates/benilla-app/tests/item_text_frame.rs b/crates/benilla-app/tests/item_text_frame.rs index 541aaa3e..bd19b89e 100644 --- a/crates/benilla-app/tests/item_text_frame.rs +++ b/crates/benilla-app/tests/item_text_frame.rs @@ -268,6 +268,134 @@ fn the_reported_html_page_draws_as_blocks_not_as_its_own_markup() { ); } +/// **B342, on the reported page, through the real archives.** Goudy, 2026-08-27 (`#bugs` +/// `1542371921486811236`): *"html images in books are not scaled correctly"* — the Alliance crest +/// on *A Treatise on Military Ranks* drawn several times the reference's size with the page's own +/// text over it, beside a 1.12.1 shot of the same page for comparison. +/// +/// The body is `page_text` 2654, quoted verbatim below, and its one `` carries **no `width=` +/// and no `height=`**. In the reference that is the CONTENT-derived span: the resolver's size call +/// is virtual, and `CSimpleTexture`'s override answers an authored `0.0` with the loaded texture's +/// texel extent, one texel to one FrameXML unit (wow-re `region-size-fallback.md` §2, decision +/// 1349). `Interface\PvPRankBadges\PvPRankAlliance` is a 128×128 BLP, so the crest is a 128-unit +/// square inside a 270-wide page. +/// +/// The engine half is pinned in `benilla-ui`'s own `simplehtml` tests against a stub oracle; what +/// this adds is the two ends only the app has — the **real** reader XML the report was +/// photographed against, and the **real** file, measured by the same decoder the renderer draws +/// with. It skips on a machine with no client data, like every other archive-backed sweep. +#[test] +fn the_reported_book_crest_draws_at_the_blps_own_size() { + let data = benilla_formats::wow_data_or_skip!(); + let chain = std::sync::Mutex::new(benilla_formats::open_chain(&data).expect("open chain")); + + let mut s = UiScript::new().unwrap(); + // The host oracle, wired exactly as `ui_script::lifecycle::install_texture_resolvers` wires the + // live one: the same decoder, so the size the layout resolves with is the size the screen shows. + s.set_texture_size_probe(Box::new(move |path| { + benilla_assets::sprite_dimensions(&chain, None, path) + })); + load_ui(&s); + s.set_item_text(Some(ItemTextState { + item: "A Treatise on Military Ranks".into(), + creator: None, + text: "\n\n\ +

A TREATISE ON MILITARY RANKS

\n\ +
\n
\n\ + \n\ +
\n\ +

What follows are

\n\ +

the military ranks

\n\ + \n" + .into(), + page: 1, + has_next: true, + material: None, + })); + s.fire_event("ITEM_TEXT_BEGIN", vec![]); + s.fire_event("ITEM_TEXT_READY", vec![]); + s.resolve(); + + let crest: Vec<_> = s + .extract() + .into_iter() + .filter(|q| match &q.content { + benilla_ui::script::QuadContent::Texture { path, .. } => { + path.as_deref().is_some_and(|t| { + t.eq_ignore_ascii_case("Interface\\PvPRankBadges\\PvPRankAlliance") + }) + } + _ => false, + }) + .filter_map(|q| q.rect) + .collect(); + assert_eq!(crest.len(), 1, "one crest quad on the page"); + let r = crest[0]; + assert_eq!( + (r.right - r.left, r.top - r.bottom), + (128.0, 128.0), + "the crest draws at the BLP's own 128x128 — a texel is a FrameXML unit" + ); + // The reported symptom, stated as the thing that must not come back: the page is 270 units + // wide, and the crest used to be stretched across all of it with the text over the top. + assert!( + r.right - r.left < 270.0, + "the crest spans the whole page again — that is the photograph" + ); + + // **The adjacent state**: page 2 of the same book (`page_text` 2655) is five unsized ``s, + // one rank badge per officer rank, and every one of them is a 32x32 BLP. Under the reported + // defect all five were page-wide slabs stacked over each other's text; each is now its own + // 32-unit square. (What separates them vertically is the `
`/`

` blocks between them — + // a floated image reserves nothing — so their spacing is the font engine's business and this VM + // has none: every text block measures zero here and the five land on one line. The SIZES are + // the claim.) + s.set_item_text(Some(ItemTextState { + item: "A Treatise on Military Ranks".into(), + creator: None, + text: "\n\n\ +

OFFICER RANKS OF THE ALLIANCE


\n\ +

Part 1

\n\ +
\n\ +

Grand Marshal



\n\ +
\n\ +

Field Marshal



\n\ +
\n\ +

Marshal



\n\ +
\n\ +

Commander



\n\ +
\n\ +

Lieutenant Commander



\n\ + \n" + .into(), + page: 2, + has_next: true, + material: None, + })); + s.fire_event("ITEM_TEXT_READY", vec![]); + s.resolve(); + + let badges: Vec<_> = s + .extract() + .into_iter() + .filter(|q| match &q.content { + benilla_ui::script::QuadContent::Texture { path, .. } => path + .as_deref() + .is_some_and(|t| t.starts_with("Interface\\PvPRankBadges\\PvPRank")), + _ => false, + }) + .filter_map(|q| q.rect) + .collect(); + assert_eq!(badges.len(), 5, "one quad per officer rank"); + for b in &badges { + assert_eq!( + (b.right - b.left, b.top - b.bottom), + (32.0, 32.0), + "each rank badge is its own 32x32 BLP, not a page-wide slab" + ); + } +} + /// **B288, closed at the reported symptom** (CarlG, decision 1507): the Verdant Note open from /// the bag, then a quest giver's gossip — both frames drew at the same TOPLEFT 0,-104 anchor, /// page text and greeting interleaved. The cause was the reader's missing `UIPanelWindows` row: diff --git a/crates/benilla-assets/src/world_assets/mod.rs b/crates/benilla-assets/src/world_assets/mod.rs index c0db3706..ac0d518b 100644 --- a/crates/benilla-assets/src/world_assets/mod.rs +++ b/crates/benilla-assets/src/world_assets/mod.rs @@ -165,6 +165,23 @@ pub fn sprite_candidates(path: &str) -> [String; 2] { } } +/// The **texel dimensions** of a UI sprite reference, or `None` when nothing resolves — the number +/// the client's `CSimpleTexture` size getters read out of `[tex+0x144]`/`[tex+0x148]` when a region +/// authored no size on an axis (wow-re `region-size-fallback.md` §2, decision 1349: one texel is one +/// FrameXML unit). +/// +/// It goes through [`decode_sprite`] rather than sniffing a header, so the size layout uses is the +/// size the screen shows: the same candidate order, the same BLP-vs-TGA content sniff, and the same +/// verdict on a candidate that reads but will not decode. The caller memoises — this is asked once +/// per distinct path, for the handful of regions whose rect is derived from their art. +pub fn sprite_dimensions( + chain: &Mutex, + loose_root: Option<&Path>, + path: &str, +) -> Option<(u32, u32)> { + decode_sprite(chain, loose_root, path).map(|(w, h, _)| (w, h)) +} + /// Decode a UI sprite key to RGBA8, **warning once per key that fails to resolve**. /// /// Every sprite cache below stores misses as well as hits, and each calls this on the miss path diff --git a/crates/benilla-formats/src/lib.rs b/crates/benilla-formats/src/lib.rs index 96029e3e..a20e8237 100644 --- a/crates/benilla-formats/src/lib.rs +++ b/crates/benilla-formats/src/lib.rs @@ -231,7 +231,8 @@ pub use models::{ ParentBasis, PlayableAnim, RenderSubmesh, RgbAnim, ScalarAnim, SeqLoops, Skeleton, SkeletonBone, StringAnchors, UvAnim, WmoBatchClass, WmoDoodad, WmoDoodadSet, WmoFog, WmoGroupHeader, WmoGroupInfo, WmoLight, WmoPortalInfo, WmoPortalRef, WmoPortals, WmoRoot, - ALPHA_KEY_REF, GLUE_AUTHORED_ASPECT, NO_GROUP_LIQUID, OWNER_RUNG_BUCKETS, + ALPHA_KEY_REF, DEGENERATE_RING_FOOTPRINT, GLUE_AUTHORED_ASPECT, NO_GROUP_LIQUID, + OWNER_RUNG_BUCKETS, }; mod terrain; pub use terrain::{ diff --git a/crates/benilla-formats/src/models/bounds.rs b/crates/benilla-formats/src/models/bounds.rs index 43c38b40..59164b77 100644 --- a/crates/benilla-formats/src/models/bounds.rs +++ b/crates/benilla-formats/src/models/bounds.rs @@ -34,6 +34,10 @@ pub struct M2Bounds { /// `0x608e00`/`0x60aee0`): the ring's world radius = this × `OBJECT_FIELD_SCALE_X`. The nested sqrt is /// a range-compressor (which is why the render-sphere never fit). Falls back to the header render-box /// XY extents for a model with no animation sequences. + /// + /// A **degenerate** box — `max.x == min.x` *and* `max.y == min.y` — takes + /// [`DEGENERATE_RING_FOOTPRINT`] instead of the formula, which is the writer's own first branch + /// and not a floor of ours (decision 1658). pub ring_footprint: f32, /// Model-space **Z** of attachment id 17 — the reference's follow-camera pivot height (wow-re /// `follow-camera`: `feet + (attach17.z + 0.0972)·scale`). `None` for a model with no slot-17 @@ -66,6 +70,20 @@ pub fn load_m2_bounds(chain: &mut Chain, raw_path: &str) -> Result { parse_m2_bounds(&bytes).with_context(|| format!("parsing M2 bounds {path}")) } +/// The ring footprint the real client stores for a **degenerate** box — one whose X *and* Y extents +/// are both exactly zero. Byte-read at `0x60af4f..0x60af67` (wow-re `selection-ring-scale.md`): the +/// writer `0x60aee0` compares `max.x==min.x` and `max.y==min.y` and, when both hold, stores the +/// literal `0x3f99999a` = **1.2** into `[unit+0xcf0]` without ever running the `sqrt(0.5·sqrt(dx²+dy²))` +/// formula. wow-re recorded it as a branch that "never fires for real creatures", which is true of +/// the four life-size units it measured and false of the whole trigger-creature family: an +/// `InvisibleStalker` body authors all 135 sequence boxes at zero, so this **is** its ring — and the +/// Naxxramas weapon mobs, whose visible self is the axe in that body's hand, are exactly where a +/// player sees it. Ours read 0 and drew a ring the width of a coin (decision 1658). +/// +/// It is the model-less fallback too: "no box to measure" and "a box that measures zero" are the +/// same question, and this is the reference's answer to it. +pub const DEGENERATE_RING_FOOTPRINT: f32 = 1.2; + /// Read the horizontal (X,Y) extents of the **Stand** animation's bounding box from a raw M2 — the input /// the real client's living-unit selection ring is sized from (wow-re selection-ring RE, `0x60aee0`). /// The animation `M2Sequence` array is at MD20 `0x1c`(count)/`0x20`(offset), stride `0x44`, with the @@ -128,7 +146,11 @@ pub fn parse_m2_bounds(bytes: &[u8]) -> Result { h.bounding_box_max[1] - h.bounding_box_min[1], h.bounding_box_max[2] - h.bounding_box_min[2], )); - let ring_footprint = (0.5 * (rx * rx + ry * ry).sqrt()).sqrt(); + let ring_footprint = if rx == 0.0 && ry == 0.0 { + DEGENERATE_RING_FOOTPRINT + } else { + (0.5 * (rx * rx + ry * ry).sqrt()).sqrt() + }; Ok(M2Bounds { sphere_radius: h.bounding_sphere_radius, bbox_min: h.bounding_box_min, diff --git a/crates/benilla-formats/tests/selection_ring_radius.rs b/crates/benilla-formats/tests/selection_ring_radius.rs index cbf602c0..d198cdba 100644 --- a/crates/benilla-formats/tests/selection_ring_radius.rs +++ b/crates/benilla-formats/tests/selection_ring_radius.rs @@ -52,3 +52,34 @@ fn ring_footprint_matches_reference_pixels() { ); } } + +/// The sizer's **other** branch: a box whose X and Y extents are both exactly zero takes the +/// literal 1.2 (`0x60af4f..0x60af67`, `0x3f99999a`) and never reaches the formula. +/// +/// wow-re recorded this branch as one that "never fires for real creatures" — true of the four +/// life-size units it measured, false of the whole trigger-creature family. `InvisibleStalker` +/// authors **all 135** of its sequence boxes at zero, so 1.2 is its ring, and the Naxxramas weapon +/// mobs (an `InvisibleStalker` body holding a visible axe, display 15294 at scale 2.25 ⇒ a 2.7 yd +/// ring) are exactly where a player meets it. We computed `sqrt(0.5·sqrt(0))` = 0 and drew a ring +/// the width of a coin — the director's A/B against the reference is what caught it. Decision 1658. +#[test] +fn a_degenerate_stand_box_rings_at_the_reference_constant() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = open_chain(&data).expect("open chain"); + + let path = "Creature\\InvisibleStalker\\InvisibleStalker.mdx"; + let b = load_m2_bounds(&mut chain, path).unwrap_or_else(|e| panic!("bounds {path}: {e}")); + // The premise: this model really is the degenerate case, on the header box and the Stand box + // alike (0 vertices ⇒ nothing to bound). If Blizzard had authored a box here the constant + // below would be the wrong answer, so pin the input, not just the output. + assert_eq!( + (b.bbox_min, b.bbox_max, b.sphere_radius), + ([0.0; 3], [0.0; 3], 0.0), + "{path}: expected an entirely unauthored bound" + ); + assert_eq!( + b.ring_footprint, + benilla_formats::DEGENERATE_RING_FOOTPRINT, + "{path}: a zero-extent box takes the writer's 1.2, not the formula's 0" + ); +} diff --git a/crates/benilla-ui/src/script/layout.rs b/crates/benilla-ui/src/script/layout.rs index 90de866b..4182ba18 100644 --- a/crates/benilla-ui/src/script/layout.rs +++ b/crates/benilla-ui/src/script/layout.rs @@ -160,9 +160,70 @@ fn region_node_hash(data: &RegionData) -> u64 { } None => node.feed(u64::MAX), } + // **The ART, on a region whose rect is derived from it** (decision 1349): an axis authored `0` + // takes its span from the content ([`content_span`]), so swapping the texture on such a region + // MOVES it and the node must re-hash. Fed only on that shape — a sized region's art cannot + // change its rect, and hashing every icon path would put a string walk in the derive's hot loop + // for a fact with no reader. + if !data.anchors.is_empty() && data.size.is_none_or(|(w, h)| w == 0.0 || h == 0.0) { + node.feed(data.fill.is_some() as u64); + match &data.texture { + Some(path) => { + node.feed(path.len() as u64); + for b in path.as_bytes() { + node.feed(u64::from(*b)); + } + } + None => node.feed(u64::MAX), + } + } node.finish() } +/// A region's **content-derived span**, in FrameXML units — the client's virtual size getters, +/// which the resolver calls instead of reading a stored width/height (wow-re +/// `region-size-fallback.md` §1/§2, decision 1349). +/// +/// `CSimpleTexture::GetWidth 0x770720` returns the authored value only when it is **not exactly +/// `0.0`**; on `0.0` with a texture loaded it returns the texture's own texel extent, through +/// bit-for-bit the same converter the `` attribute uses — so **one texel is one +/// FrameXML unit, at every aspect ratio**. `0x770790` is the identical body for height. The +/// colour form generates an **8×8** surface (`0x770360` → `0x44a900` stamps +/// `[tex+0x144] = [tex+0x148] = 8`), so a ``-only texture spans 8 units. +/// +/// `None` = nothing to measure (no art, or no host oracle to measure it with), and the caller +/// leaves the axis at zero — which is the client's answer too when a texture has no `CGxTex*` +/// at all. +/// +/// **This is the texture half of 1349 §4 only.** A `CSimpleFontString`'s span is likewise derived +/// (its measured extent, floored at one unit — `0x772930`), and our floor is still the zero-span +/// collapse the sweep applies below; that half, and deleting the owner-edge fallback the derived +/// spans make redundant, stay 1349's own arc. +fn content_span( + data: &RegionData, + probe: Option<&crate::script::TextureSizeProbe>, +) -> Option<(f32, f32)> { + if data.fill.is_some() { + return Some((SOLID_TEXTURE_TEXELS, SOLID_TEXTURE_TEXELS)); + } + texel_span(probe, data.texture.as_deref()?) +} + +/// [`content_span`]'s texture arm, for a caller holding a path rather than a region — the +/// SimpleHTML flow reservation, which asks the reference's `GetHeight` override the same way +/// (`simplehtml-markup-engine.md` §7 step 6). +pub(super) fn texel_span( + probe: Option<&crate::script::TextureSizeProbe>, + path: &str, +) -> Option<(f32, f32)> { + let (w, h) = probe?(path)?; + Some((w as f32, h as f32)) +} + +/// The edge of the surface `CSimpleTexture::SetTexture(const CImVector*)` generates for the colour +/// form — 64 texels, `0x44a900` stamping `[tex+0x144] = [tex+0x148] = 8` (decision 1349 §1). +const SOLID_TEXTURE_TEXELS: f32 = 8.0; + /// [`LayoutScope::node_of`] holds one roster index per id, and the two rosters are separate /// (`plan` for frames, `regions` for regions) — this bit says which one. Node counts run in the /// thousands, so the top bit is free forever. @@ -768,6 +829,7 @@ impl UiScript { layout_last_scope, layout_rounds, pending_size_changed, + texture_size_probe, .. } = model; // ── The incremental seed (decision 1388) ───────────────────────────────────────────── @@ -1120,6 +1182,29 @@ impl UiScript { width = m.w; } } + // A TEXTURE's unset axis is the client's *other* content-derived span: the art's + // own texel extent ([`content_span`]). It is the same shape as the FontString + // measure above — the reference's size getter is virtual, and both region classes + // override it — and it has to be applied HERE, before the resolve, because + // `combine_edge` reads a zero span as "no size at all" and leaves the opposite edge + // unresolved. + // + // That vacuum is what B342 fell into. `page_text` 2654 (*A Treatise on Military + // Ranks*) carries `` — **no width=, no + // height=** — so SimpleHTML sized the block `0 × 0`, one TOPLEFT anchor pinned the + // corner, and the owner-edge fallback below stretched a 128×128 crest across the + // rest of the 270×304 page with the text over it. The reference draws it at 128×128 + // units, because a texel is a FrameXML unit. + if !is_fontstring && (width == 0.0 || height == 0.0) { + if let Some((cw, ch)) = content_span(data, texture_size_probe.as_ref()) { + if width == 0.0 { + width = cw; + } + if height == 0.0 { + height = ch; + } + } + } scratch.anchors.clear(); scratch.anchors.extend_from_slice(&data.anchors); scratch.width = width; diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 75eb2c67..5674c825 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -183,7 +183,7 @@ pub use mail::{MailInboxRow, MailInvoice, MailSendRequest, MailState}; pub use measure::TextMeasure; pub use merchant::{ItemStatsHead, MerchantItem, MerchantState}; pub(crate) use model::Model; -pub use model::TextureProbe; +pub use model::{TextureProbe, TextureSizeProbe}; pub use party::{PartyMemberInfo, PartyRequest, PartyState, RaidMemberInfo, SavedInstanceInfo}; pub use pet::{PetActionView, PetStats}; pub use pvp::{HonorState, InspectHonorData}; diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 8412ce62..e48be97a 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -18,6 +18,9 @@ use super::{ /// The host's texture-path resolvability oracle — see [`Model::texture_probe`]. pub type TextureProbe = Box bool>; +/// The host's texture **texel-size** oracle — see [`Model::texture_size_probe`]. +pub type TextureSizeProbe = Box Option<(u32, u32)>>; + /// The Rust-side model behind the Lua VM — the arena, the layout inputs + resolved rects, the /// id↔handle bijection, region visuals, and the event/script registrations. Held in `lua.app_data` /// (interior-mutable) so callbacks reach it; contains **no** mlua handles (the MAXCSTACK discipline). @@ -38,6 +41,20 @@ pub(crate) struct Model { /// engine-less VM (tests, the addon harness), where the path form keeps answering nil: no /// backend, nothing loads — which is also exactly what those tests always saw. pub(crate) texture_probe: Option, + /// The host's texture **texel-size** oracle ([`super::UiScript::set_texture_size_probe`]): how + /// many texels wide and tall is the art at this path? + /// + /// What lets a region whose authored size on an axis is exactly `0` take its span from its + /// CONTENT, the way the real client's size getters do — `CSimpleTexture::GetWidth 0x770720` / + /// `GetHeight 0x770790` return the authored value only when it is not `0.0`, and otherwise read + /// the loaded texture's `[tex+0x144]`/`[tex+0x148]` through the same converter `` + /// uses, so **one texel is one FrameXML unit** (wow-re `region-size-fallback.md` §2, decision + /// 1349). The host answers off the same candidate walk the renderer decodes with and memoises, + /// so the size layout resolves with is the size the screen shows. + /// + /// `None` in an engine-less VM (tests, the addon harness), where a zero-size texture keeps the + /// rect it always had — there is no art to measure without a backend. + pub(crate) texture_size_probe: 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, @@ -1360,6 +1377,7 @@ impl Model { addons_root: None, measurer: None, texture_probe: None, + texture_size_probe: None, addons_saved_account: None, addons_saved_character: None, framexml_templates: Default::default(), diff --git a/crates/benilla-ui/src/script/region/paint.rs b/crates/benilla-ui/src/script/region/paint.rs index 89a4452a..c6f32a04 100644 --- a/crates/benilla-ui/src/script/region/paint.rs +++ b/crates/benilla-ui/src/script/region/paint.rs @@ -148,83 +148,112 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { lua.create_function( |lua, (this, arg, g, b, a): (Table, Value, Value, Value, Value)| { let rh = region_handle_of(lua, &this)?; - let mut model = lua.app_data_mut::().expect("model"); - // A plain SetTexture makes the region ordinary again — drop any portrait circular mask - // and any live-unit-portrait binding. - let data = model.region_data.entry(rh).or_default(); - data.circular = false; - data.portrait_unit = None; - // **`SetTexture` CLEARS the desaturation** (wow-re `texture-desaturate-law.md` §2.3, - // VERIFIED): `+0x128` is a `CGxShader*`, and `CSimpleTexture::SetTexture` - // (`0x770200`) writes it from its 4th stack arg, for which the Lua binding - // (`0x79bb40`) pushes slot 0 — permanently NULL — on both of its legs. A - // re-implementation that keeps a desaturate boolean independent of the texture - // handle diverges on every `icon:SetDesaturated(1)` followed by `icon:SetTexture(t)`. - // - // Scoped exactly as the binary scopes it: - // · the **same path** is inert — `0x770225` returns before ever reaching the write, - // so a repaint that re-sets the icon it already shows keeps its grey; - // · `nil`/`""` DO clear — the `test esi,esi` leg falls through to the write; - // · the **colour form** does NOT — that is `0x770360`, which is not among the four - // writers of the field. - let same_path = matches!((&arg, &data.texture), + // Scoped, so the model borrow is provably gone before the layout touch below — + // the path arm hands it back mid-way (to call the host probe without holding a + // mutable borrow across a host callback) and the others do not. + let (loaded, derived) = { + let mut model = lua.app_data_mut::().expect("model"); + // A plain SetTexture makes the region ordinary again — drop any portrait circular mask + // and any live-unit-portrait binding. + let data = model.region_data.entry(rh).or_default(); + data.circular = false; + data.portrait_unit = None; + // **`SetTexture` CLEARS the desaturation** (wow-re `texture-desaturate-law.md` §2.3, + // VERIFIED): `+0x128` is a `CGxShader*`, and `CSimpleTexture::SetTexture` + // (`0x770200`) writes it from its 4th stack arg, for which the Lua binding + // (`0x79bb40`) pushes slot 0 — permanently NULL — on both of its legs. A + // re-implementation that keeps a desaturate boolean independent of the texture + // handle diverges on every `icon:SetDesaturated(1)` followed by `icon:SetTexture(t)`. + // + // Scoped exactly as the binary scopes it: + // · the **same path** is inert — `0x770225` returns before ever reaching the write, + // so a repaint that re-sets the icon it already shows keeps its grey; + // · `nil`/`""` DO clear — the `test esi,esi` leg falls through to the write; + // · the **colour form** does NOT — that is `0x770360`, which is not among the four + // writers of the field. + let same_path = matches!((&arg, &data.texture), (Value::String(s), Some(cur)) if s.to_str().is_ok_and(|s| *s == **cur)); - let colour_form = matches!(&arg, Value::Number(_) | Value::Integer(_)); - if !same_path && !colour_form { - data.desaturated = false; - } - // Both forms write the SAME `+0xcc` texture slot — the path form loads a file - // there (`0x770200`), the colour form generates an 8×8 solid into it - // (`0x770360`) — so each clears the other. NEITHER touches the vertex colour at - // `+0xb8`: a tint outlives the art it was tinting. - let loaded = match &arg { - // SetTexture("") clears, same as SetTexture(nil) — the ref lua blanks state - // art with the empty string (QuestLogFrame.lua:165-166). The return for "" - // is read as the path form's failure (nothing loads from an empty name); - // INFERRED — no corpus caller reads it. - Value::String(s) if s.to_str()?.is_empty() => { - data.texture = None; - data.fill = None; - false + let colour_form = matches!(&arg, Value::Number(_) | Value::Integer(_)); + if !same_path && !colour_form { + data.desaturated = false; } - Value::String(s) => { - let path = s.to_str()?.to_string(); - data.texture = Some(path.clone()); - data.fill = None; - drop(model); - let model = lua.app_data_ref::().expect("model"); - model - .texture_probe - .as_ref() - .is_some_and(|probe| probe(&path)) - } - // The colour form, and the ONLY branch that looks at the trailing three. A - // non-numeric there takes the same default a missing one does, which is what - // reading off a C stack does: `lua_tonumber` on a non-number yields 0. - Value::Number(_) | Value::Integer(_) => { - let chan = |v: &Value, dflt: f32| match v { - Value::Number(_) | Value::Integer(_) => as_f32(v), - Value::String(s) => s - .to_str() - .ok() - .and_then(|s| s.parse::().ok()) - .unwrap_or(dflt), - _ => dflt, - }; - data.fill = - Some([as_f32(&arg), chan(&g, 0.0), chan(&b, 0.0), chan(&a, 1.0)]); - data.texture = None; - true - } - // SetTexture(nil) clears (the live API's blank-the-region form); a cleared - // texture region draws nothing. Returns 1 (Q1's `SetTexture(nil) / ()` row). - Value::Nil => { - data.texture = None; - data.fill = None; - true - } - _ => false, + // Does this region's rect come from its ART? An axis authored `0` takes its span + // from the content (decision 1349, `script::layout::content_span`), so on that + // shape — and only there — swapping the texture MOVES the region and the resolve + // has to hear about it. Read before the match, which writes the art and never the + // size. + // + // **An ANCHOR-LESS region is not that shape**, whatever its size: with no + // pinned edge and no center there is nothing for a span to be added to + // (`combine_edge` needs one), so every edge stays unset — which is why the + // resolve sweep skips such regions outright. Painting one is a paint, not a + // layout change, and saying otherwise would re-open the change gate on every + // `CreateTexture(…):SetTexture(…)` in the UI (decisions 0740/1385/1388). + let derived = !data.anchors.is_empty() + && data.size.is_none_or(|(w, h)| w == 0.0 || h == 0.0); + // Both forms write the SAME `+0xcc` texture slot — the path form loads a file + // there (`0x770200`), the colour form generates an 8×8 solid into it + // (`0x770360`) — so each clears the other. NEITHER touches the vertex colour at + // `+0xb8`: a tint outlives the art it was tinting. + let loaded = match &arg { + // SetTexture("") clears, same as SetTexture(nil) — the ref lua blanks state + // art with the empty string (QuestLogFrame.lua:165-166). The return for "" + // is read as the path form's failure (nothing loads from an empty name); + // INFERRED — no corpus caller reads it. + Value::String(s) if s.to_str()?.is_empty() => { + data.texture = None; + data.fill = None; + false + } + Value::String(s) => { + let path = s.to_str()?.to_string(); + data.texture = Some(path.clone()); + data.fill = None; + drop(model); + let model = lua.app_data_ref::().expect("model"); + model + .texture_probe + .as_ref() + .is_some_and(|probe| probe(&path)) + } + // The colour form, and the ONLY branch that looks at the trailing three. A + // non-numeric there takes the same default a missing one does, which is what + // reading off a C stack does: `lua_tonumber` on a non-number yields 0. + Value::Number(_) | Value::Integer(_) => { + let chan = |v: &Value, dflt: f32| match v { + Value::Number(_) | Value::Integer(_) => as_f32(v), + Value::String(s) => s + .to_str() + .ok() + .and_then(|s| s.parse::().ok()) + .unwrap_or(dflt), + _ => dflt, + }; + data.fill = + Some([as_f32(&arg), chan(&g, 0.0), chan(&b, 0.0), chan(&a, 1.0)]); + data.texture = None; + true + } + // SetTexture(nil) clears (the live API's blank-the-region form); a cleared + // texture region draws nothing. Returns 1 (Q1's `SetTexture(nil) / ()` row). + Value::Nil => { + data.texture = None; + data.fill = None; + true + } + _ => false, + }; + (loaded, derived) }; + // Named precisely, and only on the content-derived shape: an ordinary sized icon's + // rect cannot move here, and touching the layout on every icon repaint would + // re-open the resolve's change gate every frame for a rect nobody moved + // (decisions 0740/1385/1388). + if derived { + lua.app_data_mut::() + .expect("model") + .touch_layout_region(rh); + } Ok(if loaded { Value::Number(1.0) } else { @@ -454,4 +483,12 @@ impl crate::script::UiScript { pub fn set_texture_probe(&mut self, probe: crate::script::TextureProbe) { self.model_mut().texture_probe = Some(probe); } + + /// Install the host's texture **texel-size** oracle — what lets a region with an authored size + /// of `0` on an axis take that span from its art, as the client's virtual size getters do + /// ([`Model::texture_size_probe`], decision 1349 / wow-re `region-size-fallback.md` §2). A VM + /// that never gets one leaves such a region exactly where it was. + pub fn set_texture_size_probe(&mut self, probe: crate::script::TextureSizeProbe) { + self.model_mut().texture_size_probe = Some(probe); + } } diff --git a/crates/benilla-ui/src/script/simplehtml/mod.rs b/crates/benilla-ui/src/script/simplehtml/mod.rs index 455f5a8b..a968269f 100644 --- a/crates/benilla-ui/src/script/simplehtml/mod.rs +++ b/crates/benilla-ui/src/script/simplehtml/mod.rs @@ -43,9 +43,6 @@ //! `SimpleHTML:SetSpacing` *does* honour here is the half SimpleHTML itself owns and computes: //! the inter-block step. That is a real, drawn effect rather than a stored-and-ignored number, //! which is why this pair ships where the FontString/Font ones do not. -//! - **An `` with no declared height reserves nothing in the flow.** The reference -//! substitutes the texture's natural pixel height (`0x770790`), which needs the BLP loaded — -//! knowledge the engine core does not have at `SetText` time. //! - **A block's wrap width of 0** (a SimpleHTML sized only by anchors) lets the measure //! round-trip supply the natural text width, where the reference leaves the block at width 0. //! That is this engine's standing law for every height-less/width-less FontString, and inverting @@ -375,8 +372,23 @@ fn build(model: &mut Model, fh: FrameHandle, blocks: &[Block]) { // `prevBlock` is **never** written by the image path (three reads of `+0x348` in // the whole function, zero writes) — the next block still hangs off the last TEXT // block, only lower. + // + // The height it reserves is `texture.GetHeight()`, which is `CSimpleTexture`'s + // **override**: a `` with no `height=` reserves its art's own texel height, + // not nothing (decision 1349 — one texel is one FrameXML unit). Asked of the same + // host oracle the resolve derives the region's rect from, so the space the flow + // leaves and the space the image fills are one number. if !*floated { - next_y -= *h; + let reserved = if *h == 0.0 { + src.as_deref() + .and_then(|p| { + super::layout::texel_span(model.texture_size_probe.as_ref(), p) + }) + .map_or(0.0, |(_, th)| th) + } else { + *h + }; + next_y -= reserved; } made.push(rh); } diff --git a/crates/benilla-ui/src/script/tests/simplehtml.rs b/crates/benilla-ui/src/script/tests/simplehtml.rs index 676989c1..faaf65d5 100644 --- a/crates/benilla-ui/src/script/tests/simplehtml.rs +++ b/crates/benilla-ui/src/script/tests/simplehtml.rs @@ -542,6 +542,130 @@ fn an_unfloated_image_reserves_height_without_becoming_the_anchor() { ); } +/// **B342, at the engine.** Goudy, 2026-08-27 (`#bugs` `1542371921486811236`): *"html images in +/// books are not scaled correctly"* — the Alliance crest on *A Treatise on Military Ranks* drawn +/// several times the reference's size, with the page's text over the top of it. +/// +/// The body is `page_text` 2654 and its `` carries **no `width=` and no `height=`**, so the +/// block is sized `0 × 0`. In the reference that is not "no size": the resolver's size call is +/// VIRTUAL, and `CSimpleTexture::GetWidth 0x770720` / `GetHeight 0x770790` answer an authored `0.0` +/// with the loaded texture's own texel extent, through bit-for-bit the converter `` +/// uses — **one texel is one FrameXML unit** (wow-re `region-size-fallback.md` §2, decision 1349). +/// `PvPRankAlliance.blp` is 128×128, so the crest is a 128-unit square. +/// +/// The falsification is the reported shape itself, asserted below: a zero span leaves the opposite +/// edge unresolved (`combine_edge`'s `span != 0.0` leg), and the region sweep's owner-edge fallback +/// then hands the image the rest of the **270-wide** page. +#[test] +fn the_books_unsized_crest_is_its_arts_texel_square_not_the_whole_page() { + const CREST: &str = "Interface\\PvPRankBadges\\PvPRankAlliance"; + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.set_texture_size_probe(Box::new(|path| (path == CREST).then_some((128, 128)))); + page(&s); + s.run(&format!( + "Page:SetText({})", + lua_str( + "\n\ +

A TREATISE ON MILITARY RANKS

\n\ +
\n
\n\ + \n\ +
\n\ +

What follows are

\n\ + " + ) + )) + .unwrap(); + measure_at_16px(&mut s); + + let b = blocks(&s, "Page"); + let img = b + .iter() + .position(|x| x.texture.is_some()) + .expect("an IMG block"); + assert_eq!(b[img].texture.as_deref(), Some(CREST)); + assert_eq!( + b[img].size, + Some((0.0, 0.0)), + "the block still stores what the markup authored — the span is DERIVED at resolve, as the \ + reference's virtual getter derives it" + ); + + let rect = resolved_rect(&s, "Page", img); + assert_eq!( + (rect.right - rect.left, rect.top - rect.bottom), + (128.0, 128.0), + "a texel is a FrameXML unit: the 128x128 crest is a 128-unit square, not the page" + ); + // The reported symptom, stated as the thing that must not come back: the page is 270 wide and + // 304 tall, and the crest used to take all of the first and everything left of the second. + assert!( + rect.right - rect.left < 270.0, + "the crest is stretched across the page width again" + ); +} + +/// The flow half of the same law (`simplehtml-markup-engine.md` §7 step 6): an **unfloated** image +/// reserves `texture.GetHeight()` — `CSimpleTexture`'s override — so one with no `height=` reserves +/// its art's texel height, and the next text block hangs that far lower. A floated one (the book's +/// `align="left"`) still reserves nothing, which is why its text overlaps it in both clients. +#[test] +fn an_unsized_image_reserves_its_arts_texel_height_unless_it_is_floated() { + let mut s = script(); + s.set_texture_size_probe(Box::new(|_| Some((64, 48)))); + page(&s); + s.run( + r#"Page:SetText("

a

" .. + "" .. + "

b

")"#, + ) + .unwrap(); + assert_eq!( + blocks(&s, "Page")[2].anchor.unwrap(), + (Point::TopLeft, Point::BottomLeft, 0.0, -48.0, Rel::Block(0)), + "no height= reserves the ART's height, not nothing" + ); + + let mut s = script(); + s.set_texture_size_probe(Box::new(|_| Some((64, 48)))); + page(&s); + s.run( + r#"Page:SetText("

a

" .. + "" .. + "

b

")"#, + ) + .unwrap(); + assert_eq!( + blocks(&s, "Page")[2].anchor.unwrap(), + (Point::TopLeft, Point::BottomLeft, 0.0, 0.0, Rel::Block(0)), + "a FLOATED image reserves nothing — the text runs over it, in both clients" + ); +} + +/// A VM with no host oracle has no art to measure, so a zero-size image stays exactly where it +/// always was. The engine-less truth, stated so the fix cannot be read as "the engine now knows +/// texel sizes". +#[test] +fn without_a_size_oracle_an_unsized_image_is_unchanged() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + page(&s); + s.run(r#"Page:SetText("

a

")"#) + .unwrap(); + measure_at_16px(&mut s); + let b = blocks(&s, "Page"); + let img = b + .iter() + .position(|x| x.texture.is_some()) + .expect("an IMG block"); + let rect = resolved_rect(&s, "Page", img); + assert_eq!( + rect.right - rect.left, + 270.0, + "the owner-edge fallback, unchanged" + ); +} + // ───────────────────────────────────────────────────────────────────────────────────────────── // The Lua table // ───────────────────────────────────────────────────────────────────────────────────────────── @@ -825,6 +949,18 @@ fn resolved_tops(s: &UiScript, name: &str) -> Vec<(f32, f32)> { .collect() } +/// One block's resolved rect, by index in the block list. +fn resolved_rect(s: &UiScript, name: &str, block: usize) -> crate::layout::Rect { + let lua = s.lua(); + let model = lua.app_data_ref::().expect("model"); + let fh = model.arena.lookup(name).expect("frame"); + let st = model.simple_html.get(&fh).expect("state"); + *model + .region_resolved + .get(&st.blocks[block]) + .expect("block rect") +} + /// The text of every quad the extract emits for the page — what is actually on screen. fn page_texts_on_screen(s: &UiScript) -> Vec { s.extract()