From a48441bca8f76ddd35d0e4660b3f270b949ffe33 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 27 Aug 2026 15:15:45 +0700 Subject: [PATCH] trilinear ships on, and every line of text rounds once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - the texture filter is the reference's policy — two globals, one process-wide mode, no per-texture override — registered as real CVars every sampler reads: the hardcoded trilinear + aniso 8 at five sampler sites had been read off an apitrace of an install whose `Config.wtf` set `anisotropic "16"`, a configured value taken for a default; and `trilinear` ships ON, because the registrar's `"0"` is not what a fresh install renders at — first-launch hardware detection overwrites sixteen video CVars from its hardware table, both rows a real GPU can reach set trilinear, and the reference's own `gx.log` on this machine names that row — while `anisotropic` is not among the sixteen, so anisotropy stays off - the MSAA device clamp ran before the config was read, so a saved `gxMultisample = 8` carried from a machine that offers it still killed the render thread on one that stops at 4; the ceiling is now a property of the write itself, covering the config file, `SetCVar`, the dropdown and any writer added later — and an empty device list is no opinion, so headless runs pass through - one line, one rounding: at a fractional display scale the per-glyph snap's f32 round-trip moved half-pixel ties by an ulp, so caps and ascenders rounded up while x-height letters stayed — the letters left the baseline, the thing the reporter photographed; the line's two anchors now round once and each glyph adds integer device-pixel offsets, so a line cannot be anything but flat at any scale, proven over 15,680 layouts — and `$WOW_DPI`'s size guard, which compared a physical request against the logical size and refused every non-1x run, now reads the unit it is denominated in - a test that panics while holding `ENV_LOCK` no longer poisons the tests behind it — one missing knob resource had reported as twelve red tests, eleven naming the mutex and none naming the bug; there is nothing for the poison to protect, since the guard's `Drop` restores the environment during the panicking test's own unwind Co-Authored-By: Claude Fable 5 --- crates/benilla-app/src/cvars.rs | 172 +++++++++- crates/benilla-app/src/local_state.rs | 16 +- crates/benilla-app/src/ui_macro/tests.rs | 8 +- crates/benilla-app/src/ui_saved.rs | 12 +- crates/benilla-app/src/ui_text/engine/mod.rs | 5 +- .../benilla-app/src/ui_text/layout/measure.rs | 21 +- crates/benilla-app/src/ui_text/layout/mod.rs | 152 +++++++-- crates/benilla-app/src/video.rs | 22 +- crates/benilla-assets/src/blp.rs | 23 +- crates/benilla-assets/src/lib.rs | 2 + crates/benilla-assets/src/terrain.rs | 10 +- crates/benilla-assets/src/tex_filter.rs | 307 ++++++++++++++++++ .../benilla-assets/src/world_assets/images.rs | 26 +- crates/benilla-world/src/static_gx/render.rs | 20 +- crates/benilla-world/src/view.rs | 56 +++- crates/benilla-world/src/world_plugins.rs | 4 + 16 files changed, 771 insertions(+), 85 deletions(-) create mode 100644 crates/benilla-assets/src/tex_filter.rs diff --git a/crates/benilla-app/src/cvars.rs b/crates/benilla-app/src/cvars.rs index 789c0528..32673dec 100644 --- a/crates/benilla-app/src/cvars.rs +++ b/crates/benilla-app/src/cvars.rs @@ -289,6 +289,32 @@ pub(crate) const REGISTERED: &[(&str, &str)] = &[ // enumeration. ("gxColorBits", "32"), ("gxDepthBits", "32"), + // **The texture filter policy** — 1.12's own `trilinear` and `anisotropic`, over + // `benilla_assets::TexFilterSetting`. The defaults are the reference's registered strings, and + // benilla had neither CVar: it hardcoded trilinear + aniso 8 at every sampler it built, which + // is mode 5 — the *top* of what these two can ask for — shipped as the thing you get before + // asking. `tex_filter.rs` carries the derivation and the cost. + // + // Latched, exactly like `gxMultisample` above and for a harder reason: a sampler is baked into + // the `Image` at load and lives in the uploaded texture, so a live change would mean rebuilding + // every texture in the world. The reference's own UI says "enabled upon restart". + // `$WOW_TRILINEAR` / `$WOW_ANISO` override session-only, below. + // **`trilinear` registers "1", not the registrar's "0"** (decision 1645, correcting 1642). + // The reference's `CVar::Register` string is `"0"`, but `hwDetect` — registered `"1"` — runs + // `DetectHardware 0x641260` at boot and `CVar::Set`s sixteen video CVars from the matched + // `VideoHardware.dbc` row before the first frame, then self-clears. Every GPU this client runs + // on is unlisted in a 2004 table, so the row is the fallback scan's, whose reachable set is + // exactly rows 168/169/170 — and `trilinear` is 1 on 169 and 170, at both CPU tiers, with no + // CPU bias term. Measured as well as derived: the reference's own `WoW/Logs/gx.log` reads + // `VID: 106b` → `DID: 2` → `videoID: 170`. + // + // This is the same shape as `gxMultisample` above, which also registers the value the hardware + // table yields rather than a literal the registrar never emits on a modern machine. + ("trilinear", "1"), + // `anisotropic` registers `"1"` — off — and here the registrar's string IS the answer: it is + // **not** one of `hwDetect`'s sixteen (scan of `[0x639a60, 0x639b80)`: sixteen record-pointer + // reads, `0xc7f2e4` absent), so nothing overwrites it on any path. + ("anisotropic", "1"), // **Render scale** (decision 1639) — benilla's own CVar, no 1.12 counterpart, in the // `boothHalfRate` / `SoundOutputLimiter` mould: the reference has no such dial because it has // no second buffer to hang one on. The world renders into the composite lane's off-screen image @@ -394,7 +420,12 @@ pub(crate) struct CvarPlugin; impl Plugin for CvarPlugin { fn build(&self, app: &mut App) { app.init_resource::() - .add_systems(Startup, load_config.in_set(CvarLoad)) + .add_systems( + Startup, + (load_config, publish_filter_policy) + .chain() + .in_set(CvarLoad), + ) .add_systems(Update, sync_cvars); // **The flush is on the exit edge, not beside its feed** (decision 1528). It used to be // `(sync_cvars, save_config).chain()` in `Update`, which made the "or the app exiting" @@ -433,6 +464,7 @@ pub(crate) struct KnobParams<'w> { follow: ResMut<'w, FollowConfig>, video: ResMut<'w, VideoConfig>, render_scale: ResMut<'w, RenderScale>, + tex_filter: ResMut<'w, benilla_assets::TexFilterSetting>, pane_rate: ResMut<'w, PaneRate>, guild_notify: ResMut<'w, crate::ui_guild::GuildMemberNotify>, } @@ -451,6 +483,7 @@ impl KnobParams<'_> { scale: &mut self.scale, view: &mut self.view, msaa: &mut self.msaa, + msaa_formats: &self.msaa_formats, look: &mut self.look, click: &mut self.click, loot: &mut self.loot, @@ -463,6 +496,7 @@ impl KnobParams<'_> { follow: &mut self.follow, video: &mut self.video, render_scale: &mut self.render_scale, + tex_filter: &mut self.tex_filter, pane_rate: &mut self.pane_rate, guild_notify: &mut self.guild_notify, } @@ -476,6 +510,8 @@ struct Knobs<'a> { scale: &'a mut UiScaleCvar, view: &'a mut ViewDistance, msaa: &'a mut MsaaSetting, + /// What the device actually offers — the ceiling `gxMultisample` is clamped to (1643). + msaa_formats: &'a benilla_world::view::MsaaFormats, look: &'a mut LookConfig, click: &'a mut ClickConfig, loot: &'a mut LootConfig, @@ -488,6 +524,7 @@ struct Knobs<'a> { follow: &'a mut FollowConfig, video: &'a mut VideoConfig, render_scale: &'a mut RenderScale, + tex_filter: &'a mut benilla_assets::TexFilterSetting, pane_rate: &'a mut PaneRate, guild_notify: &'a mut crate::ui_guild::GuildMemberNotify, } @@ -597,13 +634,72 @@ fn apply_to_knobs(name: &str, value: &str, knobs: &mut Knobs) -> bool { // Writing the knob live is faithful, not a bug: the CVar holds the PENDING value (latched), // and nothing reads this resource after the world camera's spawn. "gxmultisample" => { - knobs.msaa.samples = (v as u32).clamp(*MSAA_RANGE.start(), *MSAA_RANGE.end()) + // TWO ceilings, and the second one was missing until 1643. The reference's own + // `atoi`-then-clamp `[1, 16]` comes first; then the DEVICE's, because a count this + // GPU does not offer is not a setting that degrades — it is a wgpu validation error + // that kills the render thread on frame one ("Sample count 8 is not supported by + // format Rgba16Float on this device", 2026-08-26). + // + // `MsaaSupportPlugin::finish` already clamped, but it runs once, before the first + // update — so it saw `MsaaSetting::default()` and never the value `load_config` was + // about to fold in from `config.toml`. A config written on a machine that offers 8x + // and opened on one that stops at 4 therefore reached the camera untouched. Clamping + // at the WRITE covers every writer there is: the file, a Lua `SetCVar`, the dropdown, + // and the Defaults button. + let asked = (v as u32).clamp(*MSAA_RANGE.start(), *MSAA_RANGE.end()); + let granted = knobs.msaa_formats.clamp(asked); + if granted != asked { + // At `warn`, the same posture as the seed clamp: the player asked for something + // and did not get it, and this is the only place that fact exists. + warn!( + "cvar {name}: this GPU does not offer {asked}x multisampling — using {granted}x" + ); + } + knobs.msaa.samples = granted; + } + // The filter policy's two halves. Both write the PENDING value — latched, like + // `gxMultisample`: the process policy is published once at the end of `load_config` and + // nothing reads this resource afterwards. `anisotropic` takes the reference's own + // parse-then-clamp `[1, 16]` (`0x689110`); `trilinear` is a flag like every other. + "trilinear" => knobs.tex_filter.trilinear = v != 0.0, + "anisotropic" => { + knobs.tex_filter.aniso = (v as u32).clamp( + *benilla_assets::ANISO_RANGE.start(), + *benilla_assets::ANISO_RANGE.end(), + ) } _ => return false, } true } +/// Freeze the texture filter policy for the process, and say what it resolved to. +/// +/// **A separate system, chained after [`load_config`], deliberately.** `load_config` returns early +/// on an absent or malformed file, and the policy has to be published on every one of those paths: +/// the sampler lanes are an async `AssetLoader` and a set of ordinary systems, none of which can +/// read a resource the others own, so a run that never published would be reading +/// [`benilla_assets::tex_filter`]'s fallback while a player's `config.toml` said otherwise. +/// +/// The log line is not decoration — it is the same reasoning as `video::log_display_session` +/// (1627). Every filtering report this client will get comes from a machine nobody here can run, +/// and "which mode was that run actually in" must be readable off the log a player pastes rather +/// than reasoned about. +fn publish_filter_policy(filter: Res) { + benilla_assets::publish_tex_filter(*filter); + let mode = filter.mode(); + let name = match mode { + 3 => "bilinear + nearest-mip select, aniso off", + 4 => "trilinear, aniso off", + _ => "trilinear + aniso", + }; + info!( + "texture filter: mode {mode} ({name}) — trilinear={} anisotropic={}", + u8::from(filter.trilinear), + filter.aniso + ); +} + /// A stored minimap zoom level → a valid index: truncate to int and clamp into /// `[0, MINIMAP_ZOOM_LEVELS)`, the client's own `set_zoom` clamp. fn zoom_index(v: f32) -> u8 { @@ -631,6 +727,15 @@ fn load_config(mut persist: ResMut, mut params: KnobParams) { if crate::video::novsync_env() { persist.env_overridden.insert("gxvsync".into()); } + // The filter policy's A/B levers, under the same law: pricing mode 3 against mode 5 on one + // machine in one session is exactly what these are for, and a value that stuck in + // `config.toml` would silently denominate every later reading. + if std::env::var_os("WOW_TRILINEAR").is_some() { + persist.env_overridden.insert("trilinear".into()); + } + if std::env::var_os("WOW_ANISO").is_some() { + persist.env_overridden.insert("anisotropic".into()); + } // `$WOW_WIN`, a capture scenario, or any instrumented run owns the window's geometry for the // session (decision 1627), so the two CVars that would otherwise move it mid-run are // session-only under exactly the same law as the four above. @@ -773,6 +878,7 @@ fn sync_cvars( guild_notify, msaa, msaa_formats, + tex_filter, } = ¶ms; // The config file's values go in FIRST (decision 1291): registration — ours below, or an // addon's `RegisterCVar` later — starts a key at its saved value. This is what carries a @@ -805,7 +911,7 @@ fn sync_cvars( .collect(), ); let flag = |b: bool| if b { "1" } else { "0" }.to_string(); - let session: [(&str, String); 37] = [ + let session: [(&str, String); 39] = [ ("MasterVolume", sound.master.to_string()), ("SoundVolume", sound.sfx.to_string()), ("MusicVolume", sound.music.to_string()), @@ -857,6 +963,8 @@ fn sync_cvars( ("boothHalfRate", flag(pane_rate.half)), ("renderScale", render_scale.0.to_string()), ("gxMultisample", msaa.samples.to_string()), + ("trilinear", flag(tex_filter.trilinear)), + ("anisotropic", tex_filter.aniso.to_string()), ]; for (name, value) in session { script.set_cvar_host(name, &value); @@ -1149,8 +1257,19 @@ mod tests { let mut guild_notify = crate::ui_guild::GuildMemberNotify::default(); // Literal, not Default: MsaaSetting::default() reads $WOW_MSAA. let mut msaa = MsaaSetting { samples: 1 }; + // What an Apple GPU answers for the trio we render into (Rgba16Float / Depth32Float / + // the swapchain). 8 and 16 are NOT in it — which is the whole point below. + let msaa_formats = benilla_world::view::MsaaFormats { + formats: vec![(32, 32, 1), (32, 32, 2), (32, 32, 4)], + }; // Literal for the same reason: RenderScale::default() reads $WOW_RENDER_SCALE. let mut render_scale = RenderScale(1.0); + // Literal for the same reason again (1642): TexFilterSetting::default() reads + // $WOW_TRILINEAR / $WOW_ANISO. These are what ships (1645). + let mut tex_filter = benilla_assets::TexFilterSetting { + trilinear: true, + aniso: 1, + }; let mut knobs = Knobs { sound: &mut sound, scale: &mut scale, @@ -1170,6 +1289,8 @@ mod tests { pane_rate: &mut pane_rate, guild_notify: &mut guild_notify, msaa: &mut msaa, + tex_filter: &mut tex_filter, + msaa_formats: &msaa_formats, }; assert!(apply_to_knobs("MusicVolume", "0.7", &mut knobs)); assert_eq!(knobs.sound.music, 0.7); @@ -1182,8 +1303,33 @@ mod tests { // does — the value reaching the camera is a sample COUNT, where 1 is none (1629). assert!(apply_to_knobs("gxMultisample", "4", &mut knobs)); assert_eq!(knobs.msaa.samples, 4); + // The filter policy's two rows: `anisotropic` takes the reference's own [1, 16] clamp, + // `trilinear` is a flag. Both write the pending value; the process policy is already + // published by the time either can be typed (1642). + assert!(apply_to_knobs("anisotropic", "99", &mut knobs)); + assert_eq!(knobs.tex_filter.aniso, *benilla_assets::ANISO_RANGE.end()); + assert!(apply_to_knobs("anisotropic", "0", &mut knobs)); + assert_eq!(knobs.tex_filter.aniso, *benilla_assets::ANISO_RANGE.start()); + // Both directions: the knob starts at what ships (on), so only the flip to 0 proves the + // arm does anything. + assert!(apply_to_knobs("trilinear", "0", &mut knobs)); + assert!(!knobs.tex_filter.trilinear); + assert!(apply_to_knobs("trilinear", "1", &mut knobs)); + assert!(knobs.tex_filter.trilinear); + // **The DEVICE's ceiling, not the reference's** (decision 1643). 99 clamps to the + // reference's 16 and then to the 4 this GPU offers — before 1643 it stopped at 16 and the + // camera was handed a sample count wgpu refuses, killing the render thread on frame one. assert!(apply_to_knobs("gxmultisample", "99", &mut knobs)); - assert_eq!(knobs.msaa.samples, *MSAA_RANGE.end()); + assert_eq!(knobs.msaa.samples, 4); + // The realistic route in: a config written where 8x exists, opened where it does not. + assert!(apply_to_knobs("gxMultisample", "8", &mut knobs)); + assert_eq!( + knobs.msaa.samples, 4, + "a device that stops at 4x must never be handed an 8" + ); + // A count the device DOES offer is untouched. + assert!(apply_to_knobs("gxmultisample", "2", &mut knobs)); + assert_eq!(knobs.msaa.samples, 2); assert!(apply_to_knobs("gxmultisample", "0", &mut knobs)); assert_eq!(knobs.msaa.samples, *MSAA_RANGE.start()); // Render scale takes a fraction and clamps to its own range at both ends (1639). @@ -1308,7 +1454,9 @@ mod tests { #[test] fn a_lua_setcvar_lands_in_config_toml_end_to_end() { use crate::local_state::test_env::{EnvGuard, ENV_LOCK}; - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-cvar-e2e-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _c = EnvGuard::unset("WOW_CAPTURE"); @@ -1365,6 +1513,12 @@ mod tests { .insert_resource(UiScaleCvar(DEFAULT_UI_SCALE)) .insert_resource(ViewDistance { farclip: 350.0 }) .insert_resource(MsaaSetting { samples: 1 }) + // Literal for the same reason (1642): TexFilterSetting::default() reads + // $WOW_TRILINEAR / $WOW_ANISO. These are what ships (1645). + .insert_resource(benilla_assets::TexFilterSetting { + trilinear: true, + aniso: 1, + }) // The device menu the Video dropdown reads. A real-shaped list, not empty: these // tests exercise `GetCurrentMultisampleFormat`'s lookup, which needs rows to find. .insert_resource(benilla_world::view::MsaaFormats { @@ -1403,7 +1557,9 @@ mod tests { fn entering_the_world_survives_the_quit_and_comes_back_selected() { use crate::char_select::{ClientState, Roster}; use crate::local_state::test_env::{EnvGuard, ENV_LOCK}; - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-lastchar-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _c = EnvGuard::unset("WOW_CAPTURE"); @@ -1477,7 +1633,9 @@ mod tests { #[test] fn a_minimap_setzoom_reaches_the_knob_and_the_file() { use crate::local_state::test_env::{EnvGuard, ENV_LOCK}; - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-mmzoom-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _c = EnvGuard::unset("WOW_CAPTURE"); diff --git a/crates/benilla-app/src/local_state.rs b/crates/benilla-app/src/local_state.rs index 2d48bc37..283cb076 100644 --- a/crates/benilla-app/src/local_state.rs +++ b/crates/benilla-app/src/local_state.rs @@ -362,7 +362,9 @@ mod tests { /// this one owns is 0954's layout table, unchanged by 1175's move. #[test] fn the_home_law_override_then_the_residents_and_hermetic_captures() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-ls-{}", std::process::id())); std::fs::create_dir_all(&tmp).unwrap(); @@ -459,7 +461,9 @@ mod tests { /// changed. #[test] fn the_state_folder_no_longer_depends_on_finding_the_install() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let _c = EnvGuard::unset("WOW_CAPTURE"); let _h = EnvGuard::unset("BENILLA_HOME"); let _d = EnvGuard::set("WOW_DATA", "/nonexistent/benilla-test/Data"); @@ -483,7 +487,9 @@ mod tests { /// whether it runs in the primary or in a slot. #[test] fn the_state_folder_lands_where_the_build_says() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let _c = EnvGuard::unset("WOW_CAPTURE"); let _h = EnvGuard::unset("BENILLA_HOME"); let h = home().expect("home() always resolves outside a capture"); @@ -519,7 +525,9 @@ mod tests { #[test] fn write_atomic_creates_dirs_and_replaces_whole_files() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-wa-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let path = tmp.join("nested/config.toml"); diff --git a/crates/benilla-app/src/ui_macro/tests.rs b/crates/benilla-app/src/ui_macro/tests.rs index 437d8fa6..749a906d 100644 --- a/crates/benilla-app/src/ui_macro/tests.rs +++ b/crates/benilla-app/src/ui_macro/tests.rs @@ -22,7 +22,9 @@ fn macro_view(name: &str, body: &str) -> MacroView { /// macros back — the whole persistence loop over the real `local_state` law. #[test] fn a_saved_macro_table_round_trips_through_benilla_macros() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-macros-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _c = EnvGuard::unset("WOW_CAPTURE"); @@ -57,7 +59,9 @@ fn a_saved_macro_table_round_trips_through_benilla_macros() { /// a capture is session-only and nothing is written under anyone's install. #[test] fn a_capture_run_persists_nothing() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let _h = EnvGuard::set("BENILLA_HOME", "/tmp/benilla-should-not-exist"); let _c = EnvGuard::set("WOW_CAPTURE", "ui-macro"); assert_eq!(crate::local_state::macros_account_path(), None); diff --git a/crates/benilla-app/src/ui_saved.rs b/crates/benilla-app/src/ui_saved.rs index 850d454f..3b3fb3c2 100644 --- a/crates/benilla-app/src/ui_saved.rs +++ b/crates/benilla-app/src/ui_saved.rs @@ -129,7 +129,9 @@ mod tests { /// VM's value is replaced by the saved one — then `VARIABLES_LOADED` fires, once, after it. #[test] fn the_file_round_trips_through_the_folder_and_then_fires_variables_loaded() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-sv-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _c = EnvGuard::unset("WOW_CAPTURE"); @@ -165,7 +167,9 @@ mod tests { /// `BENILLA_HOME` pointing somewhere writable (0954's law, and the reason captures are stable). #[test] fn a_capture_run_neither_reads_nor_writes() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-svcap-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _h = EnvGuard::set("BENILLA_HOME", tmp.to_str().unwrap()); @@ -184,7 +188,9 @@ mod tests { /// Nothing registered = the UI never loaded: writing would be a wipe, so it is skipped. #[test] fn an_empty_declaration_set_writes_nothing() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); let tmp = std::env::temp_dir().join(format!("benilla-svempty-{}", std::process::id())); std::fs::remove_dir_all(&tmp).ok(); let _c = EnvGuard::unset("WOW_CAPTURE"); diff --git a/crates/benilla-app/src/ui_text/engine/mod.rs b/crates/benilla-app/src/ui_text/engine/mod.rs index b0a3af17..51a65561 100644 --- a/crates/benilla-app/src/ui_text/engine/mod.rs +++ b/crates/benilla-app/src/ui_text/engine/mod.rs @@ -795,8 +795,9 @@ mod differential_tests { .chars() .filter_map(|c| e.char_cell(face, ppem, c)) .flat_map(|c| c.glyphs.iter()) - .map(|g| (g.advance.floor() + step_extra) / dpi) - .sum(); + .map(|g| g.advance.floor() + step_extra) + .sum::() + / dpi; assert_eq!( got, by_glyph, "{family} @{ppem} dpi {dpi} extra {step_extra}: {text:?} — the \ diff --git a/crates/benilla-app/src/ui_text/layout/measure.rs b/crates/benilla-app/src/ui_text/layout/measure.rs index f6af1205..0215f44d 100644 --- a/crates/benilla-app/src/ui_text/layout/measure.rs +++ b/crates/benilla-app/src/ui_text/layout/measure.rs @@ -73,11 +73,16 @@ pub(super) fn resolve(e: &mut TextEngine, font: &FontSpec) -> Resolved { /// The caller must have ensured `text`'s metrics; a character that has not been is skipped rather /// than shaped, because this half is deliberately `&`-only. pub(super) fn measure_line_width(e: &TextEngine, r: Resolved, step_extra: f32, text: &str) -> f32 { - let dpi = e.dpi(); - text.chars() + // Summed in DEVICE px and divided once, because that is exactly what the render pen does + // (decision 1644 — it walks in device px so the line's origin can round once). A sum of + // quotients and a quotient of sums are not the same `f32`, and "measure == render, to the bit" + // is a property this module is asked to keep, not one it gets to approximate. + let steps: f32 = text + .chars() .filter_map(|c| e.char_cell(r.face, r.ppem, c)) - .map(|c| (c.floor_sum + step_extra * c.glyphs.len() as f32) / dpi) - .sum() + .map(|c| c.floor_sum + step_extra * c.glyphs.len() as f32) + .sum(); + steps / e.dpi() } /// The width law itself, for the differential test that pins it to a shaped sum. @@ -385,16 +390,18 @@ pub(crate) fn line_advances(e: &mut TextEngine, text: &str, font: FontSpec) -> V let mut written = vec![false; text.len() + 1]; written[0] = true; - let mut x = 0.0f32; + // The same device-px pen the render walks (1644), so a caret boundary is the same number the + // glyph before it was drawn at. + let mut x_dev = 0.0f32; for (off, ch) in drawn.char_indices() { if let Some(c) = e.char_cell(r.face, r.ppem, ch) { for g in &c.glyphs { - x += client_step(g.advance, r.step_extra, dpi); + x_dev += client_step(g.advance, r.step_extra); } } // This character's end boundary in DRAWN bytes → the RAW boundary it sits on. let end = bounds[(off + ch.len_utf8()).min(drawn.len())]; - cum[end] = x; + cum[end] = x_dev / dpi; written[end] = true; } // Forward-fill the unwritten slots (cluster interiors, markup bytes): each carries the diff --git a/crates/benilla-app/src/ui_text/layout/mod.rs b/crates/benilla-app/src/ui_text/layout/mod.rs index 71213ae6..520f1f39 100644 --- a/crates/benilla-app/src/ui_text/layout/mod.rs +++ b/crates/benilla-app/src/ui_text/layout/mod.rs @@ -81,9 +81,10 @@ pub(crate) struct FontSpec<'a> { /// only-negative kerning scaled by the face's advance-adjust, then rounds). This extra tracking is /// look-defining — it is why real client text reads wider/denser than the raw font metrics. /// -/// **Applied in PHYSICAL device pixels** (the ppem the face is actually rasterized at), then divided -/// back to logical — because that is where the real client's law lives: `FT_advance >> 6` floors the -/// device-pixel advance and the `+1`/`+1` biases are whole *device* pixels. Flooring in logical px +/// **Answered in PHYSICAL device pixels** (the ppem the face is actually rasterized at); the caller +/// divides back to logical **once**, at the end of the line (decision 1644) — because that is where +/// the real client's law lives: `FT_advance >> 6` floors the device-pixel advance and the `+1`/`+1` +/// biases are whole *device* pixels. Flooring in logical px /// instead (the pre-fix shortcut) over-tracks on any window where `dpi ≠ 1`: at retina a /// 6.385px-logical (12.77px-physical) ARIALN digit steps `(⌊12.77⌋+2)/2 = 7.0px`, not the logical /// `⌊6.385⌋+2 = 8.0px` — a full logical pixel of extra gap *per digit*, proportionally huge on small @@ -92,8 +93,8 @@ pub(crate) struct FontSpec<'a> { /// Deliberate v1 simplification (module doc): kerning is dropped entirely (the client applies only /// *negative* pair kerns and rounds the sum — at UI sizes the rounded contribution is almost always /// 0). -fn client_step(raw_physical_advance: f32, step_extra: f32, dpi: f32) -> f32 { - (raw_physical_advance.floor() + step_extra) / dpi +fn client_step(raw_physical_advance: f32, step_extra: f32) -> f32 { + raw_physical_advance.floor() + step_extra } /// The step-law bias for a font's outline flag: the base `+1` for everyone, and one more `+1` for @@ -236,17 +237,26 @@ fn layout_text_quads_inner( let e = &*e; let dpi = e.dpi(); let sheet = e.sheet_image(); - // Snap a logical coordinate to the nearest DEVICE pixel (a multiple of `1/dpi`), so glyph edges - // land on device pixels — the same integer-device-pixel snap the real client does, which keeps - // the cell crisp instead of resampled. A cell is an integer count of device texels, so the - // quad's far edge is then automatically device-aligned too. + // **The line rounds ONCE, and the glyphs are integer device offsets from it** (decision 1644). // - // Note what this now buys that it could not before: every glyph on a line shares one `pen_y` - // and an integer physical `bearing_top`, so they all round by the same residual and the line is - // FLAT. Under the old size ladder a rescale ran after this and multiplied each already-rounded - // position by a non-integer factor, giving every letter its own sub-pixel phase — which is what - // "the letters aren't on a line" was (decision 1342). - let snap = |v: f32| (v * dpi).round() / dpi; + // Glyph edges must land on device pixels — the same integer-device-pixel placement the real + // client does — or the cell is resampled instead of blitted. Everything a glyph contributes to + // its own position is *already* an integer count of device texels: swash's `bearing_x` / + // `bearing_top`, the shaper's `y_off`, and the step law's `floor(advance) + extra`. So the pen + // walks in DEVICE px, the line's origin is rounded once, and each glyph adds its integers to + // that one rounded number. + // + // The obvious spelling — `snap(pen + per_glyph_offset)` with `snap(v) = (v·dpi).round()/dpi` — + // is the same thing in exact arithmetic and NOT the same thing in `f32`, which is what B232 + // was. At a fractional scale factor (Windows 150 %, a fractional Wayland scale) `bearing/dpi` + // is inexact, so a sum that is mathematically a half-pixel tie lands either side of the tie + // depending on the glyph's own bearing: the tall letters round one way, the x-height letters + // the other, and the word visibly steps. It fires only where the tie actually falls (~2 % of + // seats at dpi 1.5, ~4 % at 1.75 — measured), which is why it read as "one UI scale is broken + // and the next is fine" rather than as anything systematic. Rounding the ORIGIN instead of the + // sum removes the tie from the per-glyph term entirely: a line cannot be anything but flat. + // (The predecessor defect, 1342's: a rescale ran *after* the snap and multiplied each rounded + // position by a non-integer factor, giving every letter its own sub-pixel phase.) // The render lays the same lines the measure counted, or a MIDDLE-justified block seats // against a height it does not have ([`fontstring_lines`], decision 1343). @@ -325,10 +335,12 @@ fn layout_text_quads_inner( let mut pen_y = block_top + baseline_in_cell; // One glyph laid at a line-relative x-origin of 0, pending the line's justification shift. + // Both offsets are **device px, and integral** — the pen's own walk plus swash's bearings. struct PendingGlyph { - x_rel: f32, - /// Vertical offset from the line baseline (`pen_y`). - y_rel: f32, + /// Horizontal offset from the line's rounded origin. + x_dev: f32, + /// Vertical offset from the line's rounded baseline. + y_dev: f32, uv: Rect, px_w: f32, px_h: f32, @@ -347,12 +359,14 @@ fn layout_text_quads_inner( // This line's hyperlink x-ranges (line-origin space), one entry per distinct link — a // link's runs are contiguous, so min/max of its runs' pen extents is its span. let mut line_links: Vec<(std::sync::Arc, f32, f32)> = Vec::new(); - let mut pen_x = 0.0f32; + // The pen in DEVICE px: every step is `floor(advance) + extra`, an integer, so the walk + // is exact and the logical width is one division at the end rather than a sum of quotients. + let mut pen_dev = 0.0f32; for run in line { if run.text.is_empty() { continue; } - let run_x0 = pen_x; + let run_x0 = pen_dev / dpi; // The pen walks CHARACTERS, not a shaped buffer. The client's law has no kerning and no // neighbour term (`ComputeStep 0x5ca2d0`), so a run's glyph sequence is the // concatenation of its characters' — which is exactly what the cache holds, and exactly @@ -373,15 +387,15 @@ fn layout_text_quads_inner( for g in &cc.glyphs { if let Some(info) = e.cell(r.face, r.ppem, r.radius, g.glyph_id) { pending.push(PendingGlyph { - x_rel: pen_x + info.bearing_x / dpi, - y_rel: g.y_off / dpi - info.bearing_top / dpi - thick_rise, + x_dev: pen_dev + info.bearing_x, + y_dev: g.y_off - info.bearing_top, uv: info.uv, px_w: info.px_w / dpi, px_h: info.px_h / dpi, color, }); } - pen_x += client_step(g.advance, r.step_extra, dpi); + pen_dev += client_step(g.advance, r.step_extra); } char_index += 1; } @@ -390,22 +404,27 @@ fn layout_text_quads_inner( .iter_mut() .find(|(a, _, _)| std::sync::Arc::ptr_eq(a, info)) { - Some((_, _, x1)) => *x1 = pen_x, - None => line_links.push((info.clone(), run_x0, pen_x)), + Some((_, _, x1)) => *x1 = pen_dev / dpi, + None => line_links.push((info.clone(), run_x0, pen_dev / dpi)), } } } // Second pass: shift the whole line by the justification offset and emit. - let line_width = pen_x; + let line_width = pen_dev / dpi; let origin_x = match justify.h { JustifyH::Left => rect.min.x, JustifyH::Center => rect.min.x + (rect.width() - line_width) * 0.5, JustifyH::Right => rect.max.x - line_width, }; + // The line's two rounded device anchors — computed once, shared by every glyph on it. + // `thick_rise` is a LOGICAL px (the composite cell's net blit shift), so it rides into the + // round with the baseline rather than being applied to each glyph after it. + let origin_dev_x = (origin_x * dpi).round(); + let baseline_dev_y = ((pen_y - thick_rise) * dpi).round(); for g in &pending { - let gx = snap(origin_x + g.x_rel); - let gy = snap(pen_y + g.y_rel); + let gx = (origin_dev_x + g.x_dev) / dpi; + let gy = (baseline_dev_y + g.y_dev) / dpi; quads.push(UiQuad { rect: Rect::new(gx, gy, gx + g.px_w, gy + g.px_h), z_key, @@ -567,6 +586,83 @@ mod measure_fits_render { } } + /// **One line is ONE baseline — at every DPI, size and seat.** + /// + /// The emit pass's whole claim (decision 1342) is that a line is flat *by construction*: one + /// `pen_y`, an integer physical `bearing_top` per glyph, so every letter rounds by the same + /// residual. That is true in exact arithmetic and it was NOT true in `f32` — `snap(pen_y − + /// bt/dpi)` rounds a per-glyph sum, and at a **fractional** scale factor (Windows 150 %, + /// a fractional Wayland/KDE scale) `bt/dpi` is inexact, so a value that is mathematically a + /// half-pixel tie lands either side of it depending on the glyph's own bearing. B232's + /// "the letters are not vertically aligned" is that tie, one device pixel wide, and it can + /// only be seen at the sizes and seats where the tie actually falls — which is why one UI + /// scale showed it and the next did not. + /// + /// The invariant is read off the quads: FRIZQT's hinted raster puts **every** cell bottom + /// exactly on the baseline (`px_h == bearing_top` for every glyph at every ppem 8..64 — + /// measured, engine sweep), so one baseline means one `rect.max.y`. + #[test] + fn one_line_is_one_baseline_at_every_dpi() { + let Some(mut e) = test_engine(1.0) else { + eprintln!("skipping: no install / font chain"); + return; + }; + // Integer DPIs are the ones this machine can capture; the fractional ones are what the + // reporters run, and they are exactly where the tie lives. + for dpi in [1.0f32, 1.25, 1.5, 1.75, 2.0, 2.5, 3.0] { + e.set_dpi_for_test(dpi); + for base in [10.0f32, 12.0, 13.0, 14.0, 16.0, 18.0, 20.0] { + let s = spec(base * ERA); + // A quarter-pixel walk over the seat: a frame at 90 % UI scale seats its text at + // every fractional offset there is. + for step in 0..320 { + let top = step as f32 * 0.25; + let rect = Rect::new(17.5, top, 217.5, top + 20.0); + let quads = layout_text_quads( + &mut e, + "Combat", + rect, + [1.0; 4], + Justify { + h: JustifyH::Left, + v: JustifyV::Middle, + }, + 0, + s, + ); + let lo = quads + .iter() + .map(|q| q.rect.max.y) + .fold(f32::INFINITY, f32::min); + let hi = quads + .iter() + .map(|q| q.rect.max.y) + .fold(f32::NEG_INFINITY, f32::max); + assert!( + (hi - lo) * dpi < 1e-3, + "dpi {dpi}, height {base}, seat {top}: the letters of \"Combat\" \ + straddle {:.2} device px of baseline", + (hi - lo) * dpi + ); + // The same law horizontally: a glyph's left edge is the line's one rounded + // origin plus integers, so it is on the device grid too (a letter off it is + // resampled — the "uneven stems" half of the same report). + for q in &quads { + for (axis, dev) in + [("top", q.rect.min.y * dpi), ("left", q.rect.min.x * dpi)] + { + assert!( + (dev - dev.round()).abs() < 1e-3, + "dpi {dpi}, height {base}, seat {top}: a glyph {axis} landed at \ + {dev} device px — off the device grid" + ); + } + } + } + } + } + } + /// **The pen and the measure are the same number, to the bit.** The emit pass walks characters /// and steps; `measure_line_width` sums the same per-character steps. Nothing rounds between /// them, so this is equality, not a tolerance — and any future divergence (a re-introduced diff --git a/crates/benilla-app/src/video.rs b/crates/benilla-app/src/video.rs index 741429fb..fc24e3f1 100644 --- a/crates/benilla-app/src/video.rs +++ b/crates/benilla-app/src/video.rs @@ -283,25 +283,35 @@ fn check_window_pinned( let Ok(window) = windows.single() else { return; }; - let got = UVec2::new( - window.resolution.width() as u32, - window.resolution.height() as u32, - ); + // `$WOW_DPI` re-denominates `$WOW_WIN` in PHYSICAL px (that knob's doc says so: the capture is + // the framebuffer that player's GPU would scan out), so the size this compares has to follow + // it. Comparing the logical size under an override refused every non-1× run outright — it read + // `want` against `want/dpi`. + let res = &window.resolution; + let got = match requested_dpi() { + Some(_) => UVec2::new(res.physical_width(), res.physical_height()), + None => UVec2::new(res.width() as u32, res.height() as u32), + }; if got == want { return; } + let unit = if requested_dpi().is_some() { + "physical" + } else { + "logical" + }; let capturing = std::env::var_os("WOW_CAPTURE").is_some() || std::env::var_os("WOW_CAPTURE_UI").is_some(); if !capturing { warn!( - "window: asked for {}x{} logical, got {}x{} — the window manager clamped it to the \ + "window: asked for {}x{} {unit}, got {}x{} — the window manager clamped it to the \ display. Harmless here; it would invalidate a capture.", want.x, want.y, got.x, got.y ); return; } error!( - "window: REFUSING this capture — asked for {}x{} logical, got {}x{}. The window manager \ + "window: REFUSING this capture — asked for {}x{} {unit}, got {}x{}. The window manager \ clamped the request to the display this window opened on, so the image would not be the \ size the scenario is denominated in and any diff against it would be meaningless. Use a \ size that fits the current display (or move the window to a bigger one) and re-run.", diff --git a/crates/benilla-assets/src/blp.rs b/crates/benilla-assets/src/blp.rs index a3d0b59b..37c9bb3a 100644 --- a/crates/benilla-assets/src/blp.rs +++ b/crates/benilla-assets/src/blp.rs @@ -5,7 +5,8 @@ //! - [`BlpVariant::WorldArt`] — tiling world/model albedo: `Rgba8Unorm` (the RE'd gamma-space //! invariant — the GPU does *not* linearize albedo on sample, so shader math stays in WoW's //! gamma/byte space), the BLP's **authored** mip pyramid used verbatim (the real 1.12 client cannot -//! regenerate mips — it uploads the stored ones), repeat + trilinear + anisotropic sampling. +//! regenerate mips — it uploads the stored ones), repeat + the process filter policy +//! ([`crate::tex_filter`]). //! - [`BlpVariant::Sprite`] — emissive billboards (sun/moon discs): `Rgba8UnormSrgb`, clamp, mip 0. //! - [`BlpVariant::Cursor`] — the OS cursor image: `Rgba8UnormSrgb`, single mip. //! @@ -26,7 +27,7 @@ use crate::gpu_blp::{for_upload, UploadChain}; /// Which on-GPU form a BLP decodes to. See the module docs. #[derive(Default, Clone, Copy, PartialEq, Eq, Debug, Serialize, Deserialize)] pub enum BlpVariant { - /// Tiling world/model albedo — `Rgba8Unorm`, authored mips, repeat + anisotropic. The default. + /// Tiling world/model albedo — `Rgba8Unorm`, authored mips, repeat + the filter policy. The default. #[default] WorldArt, /// Emissive billboard (celestial disc) — `Rgba8UnormSrgb`, clamp, mip 0 only. @@ -117,8 +118,8 @@ impl AssetLoader for BlpImageLoader { /// `wow-5875-re/system/lighting`). Every decoded lane below uploads as this. const GAMMA_BYTES: TextureFormat = TextureFormat::Rgba8Unorm; -/// World/model albedo: the BLP's authored mip pyramid laid in verbatim, repeat + trilinear + -/// anisotropic. (Port of the old `repeat_texture_authored`.) +/// World/model albedo: the BLP's authored mip pyramid laid in verbatim, repeat + the process +/// filter policy ([`crate::tex_filter`]). (Port of the old `repeat_texture_authored`.) fn world_art_image(upload: UploadChain, wrap: (bool, bool)) -> Image { let UploadChain { chain, format } = upload; let levels = chain.mips.len() as u32; @@ -157,13 +158,18 @@ fn world_art_image(upload: UploadChain, wrap: (bool, bool)) -> Image { ImageAddressMode::ClampToEdge } }; + // The mip filter and the anisotropy are NOT this lane's to choose: the reference forces both + // from two process globals at every `TextureCreate` (`0x449ae0`), and what a virgin install + // gets from `hwDetect` is trilinear with anisotropy OFF. See [`crate::tex_filter`] — this used + // to read `Linear` / `8`, which is mode 5 shipped as if it were the default. + let filter = crate::tex_filter::tex_filter(); image.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: mode(wrap.0), address_mode_v: mode(wrap.1), mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, - mipmap_filter: ImageFilterMode::Linear, - anisotropy_clamp: 8, + mipmap_filter: filter.mipmap_filter(), + anisotropy_clamp: filter.anisotropy_clamp(), ..Default::default() }); image @@ -265,7 +271,10 @@ fn point_sprite_image(upload: UploadChain) -> Image { address_mode_v: ImageAddressMode::ClampToEdge, mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, - mipmap_filter: ImageFilterMode::Linear, + // A mip chain means the policy owns the mip filter here too — the reference's override is + // unconditional across every texture it creates, with no per-class exemption. The aniso + // stays absent by the argument above, and the policy agrees in modes 3 and 4. + mipmap_filter: crate::tex_filter::tex_filter().mipmap_filter(), ..Default::default() }); image diff --git a/crates/benilla-assets/src/lib.rs b/crates/benilla-assets/src/lib.rs index 14c59368..6fe74995 100644 --- a/crates/benilla-assets/src/lib.rs +++ b/crates/benilla-assets/src/lib.rs @@ -48,6 +48,8 @@ pub use blp::{BlpImageLoader, BlpLoaderSettings, BlpVariant}; /// Whether this run's GPU can eat WoW's stored DXT blocks, and the chain form that follows. mod gpu_blp; pub use gpu_blp::{bc_supported, for_upload, publish_bc_support, BlpGpuSupportPlugin, UploadChain}; +mod tex_filter; +pub use tex_filter::{publish_tex_filter, tex_filter, TexFilterSetting, ANISO_RANGE}; mod m2; pub use m2::{ EmitterBillboard, M2Model, M2ModelLoader, ModelEmitter, ModelLight, ModelRibbon, PortraitCamera, diff --git a/crates/benilla-assets/src/terrain.rs b/crates/benilla-assets/src/terrain.rs index 86dbf6bd..576bd725 100644 --- a/crates/benilla-assets/src/terrain.rs +++ b/crates/benilla-assets/src/terrain.rs @@ -137,13 +137,19 @@ pub(crate) fn layer_array_image( image.data = Some(rgba); image.texture_descriptor.mip_level_count = mip_levels; image.data_order = TextureDataOrder::LayerMajor; + // Ground is the surface the filter policy costs the most on — a tiling texture at a grazing + // angle is the anisotropy axis's worst case, and it fills most of the frame. The reference + // does not let this lane choose: both values come from the process globals + // ([`crate::tex_filter`]) — trilinear with anisotropy off, which is what `hwDetect` hands a + // virgin install (1645). + let filter = crate::tex_filter::tex_filter(); image.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, address_mode_v: ImageAddressMode::Repeat, mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, - mipmap_filter: ImageFilterMode::Linear, - anisotropy_clamp: 8, + mipmap_filter: filter.mipmap_filter(), + anisotropy_clamp: filter.anisotropy_clamp(), ..default() }); image diff --git a/crates/benilla-assets/src/tex_filter.rs b/crates/benilla-assets/src/tex_filter.rs new file mode 100644 index 00000000..5086e01d --- /dev/null +++ b/crates/benilla-assets/src/tex_filter.rs @@ -0,0 +1,307 @@ +//! The frame's **texture filter policy** — one process-global pair, exactly as the reference holds +//! it, and the divergence that pair was hiding. +//! +//! ## What the reference actually does +//! +//! The real 1.12.1 client does not let a texture choose how it is filtered. Every +//! `TextureCreate` (`0x449d90`) runs the caller's request through **`0x449ae0`** with a force flag +//! that is a literal `1` at every observed call site, and `0x449ae0` **overwrites** it from two +//! globals: filter bits 0-2 from `[0x835250]`, aniso bits 9-13 from +//! `(mode == 5 ? [0x835254] : 1)`. Their static `.data` values are **`[0x835250] = 3`** and +//! **`[0x835254] = 1`**, and a full image-wide caller census of their only two writers +//! (`0x449ac0` / `0x449ad0`) finds exactly three call sites, all inside the CVar callbacks below. +//! WMO, M2/character and terrain textures all share the one policy; there is no per-draw filter +//! override anywhere. (wow-re `system/models/scratch/wmo-texture-sampling.md` §2, VERIFIED off +//! `0x449ae0`/`0x59f170`/`0x58a980`/`0x6c4c20` + the registration block `0x6885b0`-`0x688840`.) +//! +//! Mode 3 — the static `.data` value — is `GL_LINEAR_MIPMAP_NEAREST` / `GL_LINEAR`: bilinear with +//! nearest-mip select, anisotropy off. Two CVars move it, and only upward: +//! +//! - **`trilinear`** (reg `0x688608`, registered default `"0"`) — nonzero sets mode 4, +//! `GL_LINEAR_MIPMAP_LINEAR`. +//! - **`anisotropic`** (reg `0x6887d0`, registered default **`"1"`**) — a value >1 sets mode 5 +//! (trilinear *plus* real aniso N, clamped to the device cap) and **outranks** `trilinear`. +//! +//! ## …and why the registrar's string is not what the client renders at +//! +//! **`hwDetect` overwrites it before the first frame.** `hwDetect` registers `"1"`, and +//! `DetectHardware 0x641260` → `0x639a60` then `CVar::Set`s **sixteen** video CVars from the +//! matched `VideoHardware.dbc` row, self-clears to `"0"`, and persists — so the registrar's string +//! is only ever what an install renders at if the hardware table has nothing to say. +//! +//! It always has something to say. Every GPU this client runs on now is unlisted in a 2004 table, +//! so the row comes from the **fallback** scan (`0x641610`, from record index 1, vendor `0xFFFF` +//! against a D3D9 capability tier from `CGxDevice::DeviceAdapterInfer 0x58bc90`), whose reachable +//! set is exactly three rows: **168** (tier 0), **169** (tier 1), **170** (tier 2). Tier 2 needs +//! HW T&L, >2 simultaneous textures and PS ≥ 1.1 — which any D3D9 part saturates. +//! +//! **`trilinear` is 1 on rows 169 and 170, at both CPU tiers, and carries no CPU bias term.** So +//! the mode a real machine gets from a virgin install is **4**, not 3, and that is what benilla +//! registers (decision 1645, superseding 1642's `"0"`). Measured, not only derived: the reference +//! client's own `WoW/Logs/gx.log` on the director's machine reads `VID: 106b` (unlisted) → +//! `DeviceAdapterInfer … DID: 2` → `DetectHardware(): videoID: 170`. +//! +//! **`anisotropic` is NOT one of the sixteen** (verified by scanning `[0x639a60, 0x639b80)`: +//! sixteen record-pointer reads, `0xc7f2e4` absent). It keeps its registered `"1"` on every path, +//! so anisotropy really is off out of the box — 1642's load-bearing half stands. +//! +//! ## One correction to how wide the policy reaches +//! +//! 1642 said there is no per-texture filter override anywhere in the reference. That is **too +//! broad**. `applyGlobal` is a per-*call* argument to `0x449ae0`, and while terrain (`0x6c4c77`), +//! WMO (the same wrapper `0x6c4c20`) and M2 (`0x71da72`) all pass `1` — which is what this module +//! needs and is confirmed — six subsystems pass `0` and keep their own mode: weather sprites +//! (hard-coded mode 4), the minimap, glue/loading, Lua texture widgets and the sky. None of them +//! is a lane this module builds a sampler for, so the code is right; the claim was not. +//! +//! ## The divergence this module closes +//! +//! benilla hardcoded `mipmap_filter: Linear` + `anisotropy_clamp: 8` at every sampler it built — +//! mode 5 at aniso 8, on every terrain layer array, model albedo, WMO albedo and static-GX array +//! in the game, with no CVar and no way to turn it off. That is the *maximum* of what the +//! reference will do on request, shipped as the thing it does before anybody asks. +//! +//! It is [`decisions/1624`]'s mistake one sampler over, and it was made the same way. `farclip` +//! shipped at 777 because that was "what most players' `Config.wtf` had"; the filtering shipped at +//! trilinear+aniso because an apitrace of the repo's reference install showed trilinear+aniso16 — +//! and that install's `Config.wtf` carries `SET anisotropic "16"`. Both read a *set* value as a +//! *registered* one. Verifying that a mechanism exists is not verifying that it is the default. +//! +//! The cost is not incidental. Mode 3 takes one bilinear tap per sample. Mode 5 at aniso 8 takes up +//! to **sixteen** — eight along the anisotropy axis, doubled by the trilinear mip blend — and the +//! worst case for the anisotropy axis is a tiling surface at a grazing angle, which is what ground +//! is from a third-person camera. On the GPU-bound Steam Deck frame behind 1624 (B329) the texture +//! path is where the frame already lives. +//! +//! ## Why a process global here too +//! +//! Not imitation: the same constraint the reference had. A sampler is baked into the [`Image`] +//! asset at load time, and the lanes that build one are an async `AssetLoader` with no world +//! access ([`crate::blp`]) and ordinary systems ([`crate::world_assets`], `static_gx`) — neither +//! can read the other's resource. This is exactly the shape [`crate::gpu_blp`]'s `bc_supported` +//! already has, for exactly the same reason. +//! +//! **Latched, like `gxMultisample`** (decision 1629): read once at boot and never again, because a +//! sampler already baked into an uploaded texture cannot be changed without rebuilding it — which +//! is also why the reference's own UI says "enabled upon restart". The CVar holds the pending +//! value; the textures keep what they were born with. + +use std::sync::OnceLock; + +use bevy::image::ImageFilterMode; +use bevy::prelude::*; +use bevy::render::render_resource::FilterMode; + +/// The reference's `anisotropic` clamp — its callback parses 1-16 before clamping to the device +/// cap (`0x689110`). +pub const ANISO_RANGE: std::ops::RangeInclusive = 1..=16; + +/// The two globals, as one value: `trilinear` and `anisotropic` resolved into the policy every +/// sampler in the process takes. +/// +/// The precedence is the reference's own, not a choice: `anisotropic >= 2` writes mode 5 at +/// `0x68919f` and `trilinear` writes mode 4 at `0x688d0f` *only* when the aniso option bit is +/// clear (`0x688d0d js` skips it), so aniso outranks trilinear whichever order the callbacks run. +#[derive(Resource, Clone, Copy, PartialEq, Eq, Debug)] +pub struct TexFilterSetting { + /// `trilinear` — mode 4 when set and aniso is off. + pub trilinear: bool, + /// `anisotropic` — 1 is off; >=2 is mode 5 at that level. + pub aniso: u32, +} + +impl Default for TexFilterSetting { + /// What a virgin install actually renders at — `trilinear` **on**, `anisotropic` off — with + /// `$WOW_TRILINEAR` / `$WOW_ANISO` overriding **session-only**, the same law `$WOW_MSAA` and + /// `$WOW_FARCLIP` run under: this is the A/B lever for pricing the filter policy on one machine + /// in one session, and a value pinned into `config.toml` would make a measurement sticky. + /// + /// `trilinear` is `true` here and not the registrar's `"0"` because `hwDetect` sets it on both + /// fallback rows a real GPU can reach — see the module doc, and decision 1645. + fn default() -> Self { + let env_flag = |k: &str| { + std::env::var(k) + .ok() + .map(|v| !matches!(v.as_str(), "" | "0" | "off")) + }; + Self { + trilinear: env_flag("WOW_TRILINEAR").unwrap_or(true), + aniso: std::env::var("WOW_ANISO") + .ok() + .and_then(|v| v.parse::().ok()) + .map_or(1, |v| v.clamp(*ANISO_RANGE.start(), *ANISO_RANGE.end())), + } + } +} + +impl TexFilterSetting { + /// The reference's filter mode 0-5 (`[0x835250]`) this policy resolves to. Only 3, 4 and 5 are + /// reachable from the two CVars; the lower three are set by nothing in the shipped image. + pub fn mode(self) -> u8 { + if self.aniso >= 2 { + 5 + } else if self.trilinear { + 4 + } else { + 3 + } + } + + /// The mip filter this policy asks for: mode 3 selects the nearer mip, 4 and 5 blend the two. + pub fn mipmap_filter(self) -> ImageFilterMode { + match self.mode() { + 3 => ImageFilterMode::Nearest, + _ => ImageFilterMode::Linear, + } + } + + /// The same, for the lanes that build a raw wgpu [`SamplerDescriptor`]( + /// bevy::render::render_resource::SamplerDescriptor) instead of an [`Image`]'s. + pub fn gpu_mipmap_filter(self) -> FilterMode { + match self.mode() { + 3 => FilterMode::Nearest, + _ => FilterMode::Linear, + } + } + + /// The max anisotropy — `(mode == 5 ? [0x835254] : 1)`, `0x449afc`. + /// + /// wgpu additionally requires all three filters to be `Linear` before it will accept a clamp + /// above 1, which mode 5 satisfies by construction and modes 3/4 never reach. + pub fn anisotropy_clamp(self) -> u16 { + if self.mode() == 5 { + self.aniso.clamp(*ANISO_RANGE.start(), *ANISO_RANGE.end()) as u16 + } else { + 1 + } + } +} + +static POLICY: OnceLock = OnceLock::new(); + +/// Publish the resolved policy for the process. Called once, from the CVar load +/// (`benilla_app::cvars::load_config`, the `CvarLoad` set) — the earliest moment `config.toml` has +/// been folded in, and before any system that can request a texture has run. +pub fn publish_tex_filter(setting: TexFilterSetting) { + let _ = POLICY.set(setting); +} + +/// The policy every sampler in the process takes. +/// +/// Unpublished — the world viewer, a headless test, or a texture somehow requested ahead of +/// `CvarLoad` — falls back to [`TexFilterSetting::default`], which is what a virgin install +/// renders at. A race can therefore only ever produce the faithful value, never a wrong one. +pub fn tex_filter() -> TexFilterSetting { + *POLICY.get_or_init(TexFilterSetting::default) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// **What benilla actually ships** — `hwDetect`'s row-170 outcome, not the registrar's string: + /// trilinear on, anisotropy off, mode 4 (decision 1645). If this ever reads mode 5, we are + /// shipping the divergence 1642 removed; if it reads mode 3, we are back to reading the + /// registrar's string as if `hwDetect` did not exist. + /// + /// `Default` is the assertion's subject deliberately: it is the value every unpublished lane + /// falls back to, so this pins the fallback and the shipped default in one place. It reads the + /// env, hence the guard — the levers are session-only and a set one must not fail the suite. + #[test] + fn what_ships_is_trilinear_with_anisotropy_off() { + if std::env::var_os("WOW_TRILINEAR").is_some() || std::env::var_os("WOW_ANISO").is_some() { + return; // an A/B session owns the value; the levers are tested below on literals + } + let shipped = TexFilterSetting::default(); + assert!( + shipped.trilinear, + "hwDetect sets trilinear on rows 169 and 170" + ); + assert_eq!( + shipped.aniso, 1, + "anisotropic is not one of hwDetect's sixteen" + ); + assert_eq!(shipped.mode(), 4); + assert_eq!(shipped.mipmap_filter(), ImageFilterMode::Linear); + assert_eq!(shipped.anisotropy_clamp(), 1); + } + + /// The registrar's own string, which is a real thing that is simply not what a machine gets: + /// `[0x835250] = 3` with `trilinear "0"` is the state a client would render at if the hardware + /// table said nothing. Kept as a test because the mode-3 arm is still reachable — any run whose + /// `config.toml` or `$WOW_TRILINEAR` says 0 takes it. + #[test] + fn the_registrars_own_string_is_still_mode_three() { + let registrar = TexFilterSetting { + trilinear: false, + aniso: 1, + }; + assert_eq!(registrar.mode(), 3); + assert_eq!(registrar.mipmap_filter(), ImageFilterMode::Nearest); + assert_eq!(registrar.gpu_mipmap_filter(), FilterMode::Nearest); + assert_eq!(registrar.anisotropy_clamp(), 1); + } + + /// `trilinear 1` alone is mode 4: the mip blend arrives, the aniso does not. + #[test] + fn trilinear_alone_is_mode_four() { + let s = TexFilterSetting { + trilinear: true, + aniso: 1, + }; + assert_eq!(s.mode(), 4); + assert_eq!(s.mipmap_filter(), ImageFilterMode::Linear); + assert_eq!(s.anisotropy_clamp(), 1); + } + + /// `anisotropic >= 2` is mode 5 and **outranks** `trilinear` in both directions — the mip + /// blend comes with it whether or not `trilinear` was ever set. + #[test] + fn aniso_outranks_trilinear_in_both_directions() { + for trilinear in [false, true] { + let s = TexFilterSetting { + trilinear, + aniso: 8, + }; + assert_eq!(s.mode(), 5, "aniso 8 is mode 5 with trilinear={trilinear}"); + assert_eq!(s.mipmap_filter(), ImageFilterMode::Linear); + assert_eq!(s.anisotropy_clamp(), 8); + } + } + + /// `anisotropic 1` is off, not "aniso at one sample" — the reference's own reading of the + /// registered string, and the reason the default is not mode 5. + #[test] + fn aniso_one_is_off() { + let s = TexFilterSetting { + trilinear: false, + aniso: 1, + }; + assert_eq!(s.mode(), 3); + } + + /// wgpu rejects a clamp above 1 unless every filter is `Linear`; modes 3 and 4 must therefore + /// never report one, whatever the CVar holds. + #[test] + fn a_clamp_above_one_only_ever_rides_all_linear_filters() { + for s in [ + TexFilterSetting { + trilinear: false, + aniso: 1, + }, + TexFilterSetting { + trilinear: true, + aniso: 1, + }, + TexFilterSetting { + trilinear: true, + aniso: 16, + }, + ] { + if s.anisotropy_clamp() > 1 { + assert_eq!(s.mipmap_filter(), ImageFilterMode::Linear); + assert_eq!(s.gpu_mipmap_filter(), FilterMode::Linear); + } + } + } +} diff --git a/crates/benilla-assets/src/world_assets/images.rs b/crates/benilla-assets/src/world_assets/images.rs index 8bd21151..7e296e58 100644 --- a/crates/benilla-assets/src/world_assets/images.rs +++ b/crates/benilla-assets/src/world_assets/images.rs @@ -74,14 +74,18 @@ pub fn repeat_texture_authored(upload: crate::gpu_blp::UploadChain, wrap: (bool, ImageAddressMode::ClampToEdge } }; + // The reference forces the mip filter and the anisotropy from two process globals at every + // `TextureCreate`, and a virgin install lands on trilinear with anisotropy OFF + // ([`crate::tex_filter`]). This read `Linear` / `8` — mode 5 — under the comment "the vanilla + // look", which was the look of an install with `SET anisotropic "16"` in its `Config.wtf`. + let filter = crate::tex_filter::tex_filter(); image.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: mode(wrap.0), address_mode_v: mode(wrap.1), - // Trilinear + anisotropic: smooth, sharp-where-it-should-be, no shimmer — the vanilla look. mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, - mipmap_filter: ImageFilterMode::Linear, - anisotropy_clamp: 8, + mipmap_filter: filter.mipmap_filter(), + anisotropy_clamp: filter.anisotropy_clamp(), ..default() }); image @@ -143,16 +147,22 @@ pub fn liquid_frame_array(frames: Vec) -> Image { dimension: Some(TextureViewDimension::D2Array), ..default() }); + // **The apitrace that pinned this was read one step too far.** WoW.20 showed the ripple + // sampler at LINEAR_MIPMAP_LINEAR + MAX_ANISOTROPY 16 and it was recorded here as "VERIFIED + // the reference ripple sampler" — but that capture came off the repo's reference install, + // whose `Config.wtf` carries `SET anisotropic "16"`. It proves the *mechanism* (the global + // reaches the liquid sampler like every other), not the *default*, which is aniso off. There + // is no per-texture filter override anywhere in the reference, so water takes the same policy + // as ground ([`crate::tex_filter`]) — and at `anisotropic 16` this lane is byte-identical to + // what it was. + let filter = crate::tex_filter::tex_filter(); image.sampler = ImageSampler::Descriptor(ImageSamplerDescriptor { address_mode_u: ImageAddressMode::Repeat, address_mode_v: ImageAddressMode::Repeat, - // Trilinear + 16× anisotropic — VERIFIED the reference ripple sampler (apitrace WoW.20: tex 442, - // LINEAR_MIPMAP_LINEAR + MAX_ANISOTROPY 16). This is what turns the unmipped sparkle into the - // reference's sparse, smooth, elongated wave-streaks at the grazing water surface. mag_filter: ImageFilterMode::Linear, min_filter: ImageFilterMode::Linear, - mipmap_filter: ImageFilterMode::Linear, - anisotropy_clamp: 16, + mipmap_filter: filter.mipmap_filter(), + anisotropy_clamp: filter.anisotropy_clamp(), ..default() }); image diff --git a/crates/benilla-world/src/static_gx/render.rs b/crates/benilla-world/src/static_gx/render.rs index 1ceec2c2..43c2cba2 100644 --- a/crates/benilla-world/src/static_gx/render.rs +++ b/crates/benilla-world/src/static_gx/render.rs @@ -228,21 +228,25 @@ fn init_pipelines(mut commands: Commands, render_device: Res) { ), ); // TWO samplers per array — repeat and clamp, both matching the BLP loader's model albedo - // sampler exactly (linear tri-filtered, ANISOTROPY 8 — `blp.rs`; the aniso is load-bearing - // for parity, oblique minification reads visibly softer without it). The shader selects by - // the vertex word's wrap bits: a shared array cannot carry per-layer address modes. The - // rare MIXED-wrap batch (repeat one axis, clamp the other) keeps the repeat sampler plus - // the shader's half-texel inset clamp on its clamped axis — an approximation confined to - // that class (decision 0763's silhouette concern, honoured per axis). + // sampler exactly. Parity with `blp.rs` was always the rule here; what changed is that + // `blp.rs` no longer picks the numbers. The mip filter and the anisotropy are the process + // filter policy's (`benilla_assets::tex_filter`), which the reference forces onto every + // texture it creates from `[0x835250]`/`[0x835254]` — so this lane stays in step by asking + // the same question rather than by copying the same literals. The shader selects by the + // vertex word's wrap bits: a shared array cannot carry per-layer address modes. The rare + // MIXED-wrap batch (repeat one axis, clamp the other) keeps the repeat sampler plus the + // shader's half-texel inset clamp on its clamped axis — an approximation confined to that + // class (decision 0763's silhouette concern, honoured per axis). + let filter = benilla_assets::tex_filter(); let make = |label: &'static str, mode: AddressMode| { render_device.create_sampler(&SamplerDescriptor { label: Some(label), min_filter: FilterMode::Linear, mag_filter: FilterMode::Linear, - mipmap_filter: FilterMode::Linear, + mipmap_filter: filter.gpu_mipmap_filter(), address_mode_u: mode, address_mode_v: mode, - anisotropy_clamp: 8, + anisotropy_clamp: filter.anisotropy_clamp(), ..Default::default() }) }; diff --git a/crates/benilla-world/src/view.rs b/crates/benilla-world/src/view.rs index 23217307..ae161a4a 100644 --- a/crates/benilla-world/src/view.rs +++ b/crates/benilla-world/src/view.rs @@ -257,6 +257,31 @@ pub struct MsaaFormats { pub formats: Vec<(u32, u32, u32)>, } +impl MsaaFormats { + /// `requested`, stepped DOWN to the nearest count this device actually offers. + /// + /// **The single expression of "the device has the final say"** (decision 1643). It used to + /// exist only inside [`MsaaSupportPlugin::finish`], which runs once, before the first update — + /// i.e. against whatever [`MsaaSetting::default`] seeded, and therefore *before* + /// `config.toml` had been folded in. The doc two paragraphs up already claimed that "the list + /// a player picks from and the ceiling a typed value is clamped to cannot disagree"; a typed + /// value was in fact never clamped to it at all, and a saved `gxMultisample = 8` opened on a + /// GPU that stops at 4 handed the camera an 8 — the 2026-08-26 render-thread kill, one launch + /// later and on a machine the setting was never chosen on. + /// + /// **An empty list is no opinion, not a refusal.** Headless, or before the adapter exists, + /// there is no device to be wrong about and `requested` passes through — the same early-out + /// `finish` takes when there is no `RenderAdapter`. Clamping to 1 there would turn every + /// device-less test and every worldview boot into a silent "multisampling off". + pub fn clamp(&self, requested: u32) -> u32 { + if self.formats.is_empty() { + return requested; + } + let counts: Vec = self.formats.iter().map(|&(_, _, n)| n).collect(); + clamp_to_supported(requested, &counts) + } +} + /// Colour and depth bit counts for the dropdown's label, derived from the formats we actually /// render into rather than typed: `MULTISAMPLING_FORMAT_STRING` is /// `"%d-bit color %d-bit depth %dx multisample"`, and a number in it should be true. @@ -312,7 +337,10 @@ impl Plugin for MsaaSupportPlugin { let Some(requested) = app.world().get_resource::().map(|m| m.samples) else { return; }; - let granted = clamp_to_supported(requested, &supported); + // Through `MsaaFormats::clamp`, not `clamp_to_supported` directly: this seed clamp and the + // per-write clamp in `cvars::apply_to_knobs` must be the same rule, or the pair drifts + // exactly the way the missing half already did (1643). + let granted = app.world().resource::().clamp(requested); if granted == requested { debug!("msaa: {requested}x accepted (this GPU offers {supported:?})"); return; @@ -666,6 +694,32 @@ mod msaa_tests { assert_eq!(MsaaSetting { samples: 1 }.level(), Msaa::Off); } + /// [`MsaaFormats::clamp`] — the one expression of "the device has the final say" (1643), + /// and both of its halves matter. + /// + /// A device list steps a request DOWN to something the GPU will actually take, which is what + /// stops a saved `gxMultisample` from reaching the camera as a wgpu validation error. An + /// EMPTY list is no opinion rather than a refusal: headless, and before the adapter exists, + /// there is no device to be wrong about — clamping to 1 there would turn every device-less + /// run into a silent "multisampling off", including the one this crate's own tests run in. + #[test] + fn the_device_list_steps_down_and_an_empty_one_has_no_opinion() { + let apple = MsaaFormats { + formats: vec![(32, 32, 1), (32, 32, 2), (32, 32, 4)], + }; + assert_eq!(apple.clamp(4), 4, "a count it offers is untouched"); + assert_eq!(apple.clamp(3), 2, "down to the nearest offered, never up"); + assert_eq!(apple.clamp(8), 4, "the 2026-08-26 render-thread kill"); + assert_eq!(apple.clamp(16), 4); + assert_eq!(apple.clamp(1), 1); + + let headless = MsaaFormats::default(); + assert!(headless.formats.is_empty()); + for n in [1, 2, 4, 8, 16] { + assert_eq!(headless.clamp(n), n, "no adapter, no opinion"); + } + } + #[test] fn a_sample_count_rounds_down_never_up() { let level = |n| MsaaSetting { samples: n }.level(); diff --git a/crates/benilla-world/src/world_plugins.rs b/crates/benilla-world/src/world_plugins.rs index f3342a0c..626af726 100644 --- a/crates/benilla-world/src/world_plugins.rs +++ b/crates/benilla-world/src/world_plugins.rs @@ -246,6 +246,10 @@ impl Plugin for WorldFoundation { // ONCE, at the camera's spawn, because the reference's own flag says latched. .init_resource::() .init_resource::() + // The process texture filter policy (`trilinear` / `anisotropic`) — read ONCE, at the + // CVar load, for the same reason: a sampler is baked into an uploaded texture and the + // reference's own UI says "enabled upon restart". + .init_resource::() // ...and the clamp that keeps it inside what this GPU will actually accept. Runs in // plugin `finish()`, so it lands between the render adapter appearing and `Startup` // reading the setting — see `view::MsaaSupportPlugin`.