From 4340c50f60c450664ee306c55ad5b656303bb15d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 11 Jul 2026 12:58:58 +0200 Subject: [PATCH] fix(render): honour render_scale from the first frame on every host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Renderer::new_impl` sizes the depth/HDR/G-buffer targets to the full surface, but the pass chain runs at `render_extent()` (surface * render_scale, 0.5 by default). `resize()` is the only thing that reconciles the two, and until now the only caller that ran it at boot was the Windows crate, which patched itself in-place. Every other host boots through `attach_engine` (macOS, iOS/iPadOS, tvOS, Linux, Android, visionOS) and never resizes on frame 1: those platforms poll the OS size and only resize when it *changes*, which is false on a plain windowed launch. They rendered against targets that ignored render_scale — post-FX silently dropped out, and the depth-snapshot copy spans a partial depth texture, which wgpu rejects outright once a scene-reading translucent material is in view (the fatal first-frame validation error the Windows fix was chasing). Verified on an M1 Max: before this change the macOS boot path rendered visibly differently from the resized Windows path (mean 1.076/255, max 191, bloom/glow highlights missing); render_extent reported 128x128 against 256x256 targets. Fix it at the root instead: `Renderer::new_impl` now runs `resize()` before returning, so construction-time targets are always correct and every platform inherits it. The Windows band-aid is removed — it was masking the bug for one platform and hiding it for the rest. The golden suite structurally cannot catch this: its harness calls `set_taa_enabled(false)` right after construction, which internally resizes and repairs the invariant before anything renders. So add tests/render_targets.rs, which asserts render_target_extent() == render_extent() straight out of the constructor (the real frame-1 state) and that it survives set_render_scale / resize / set_taa_enabled. Needs one new accessor, `Renderer::render_target_extent()`. All 7 goldens pass unchanged, which is the evidence this is pixel-neutral wherever a resize already happened and only changes behaviour where it was genuinely broken. Also files EN-024: iOS/iPadOS/tvOS report pixels as the logical size (macOS reports points), which makes the physical-resolution glyph work inert on Apple mobile and diverges game-facing coordinates. Left as a decision, not folded in here — both resolutions are breaking. --- docs/tickets.md | 40 ++++++++++++++ native/shared/src/renderer/mod.rs | 27 +++++++++- native/shared/tests/render_targets.rs | 76 +++++++++++++++++++++++++++ native/windows/src/lib.rs | 14 ++--- 4 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 native/shared/tests/render_targets.rs diff --git a/docs/tickets.md b/docs/tickets.md index 44ee957..87abfb6 100644 --- a/docs/tickets.md +++ b/docs/tickets.md @@ -817,3 +817,43 @@ building base and ≥8-10% luma in shaded receivers. `setSsgiEnabled(false)` on SW adapters banks the ~1.6 ms until this lands; needs a backend query FFI or the game reading the new boot log. +--- + +## EN-024 — iOS/iPadOS/tvOS report pixels as the logical size 🔴 needs decision + +**Why.** macOS passes points as logical and pixels as physical +(`native/macos/src/lib.rs:361`, via `backingScaleFactor`). iOS/iPadOS +and tvOS pass **pixels for both**: `bloom_attach_native` sets +`logical_w = physical_w = width` (`native/ios/src/lib.rs:770-773`) and +the orientation-change poll resizes with `resize(pw, ph, pw, ph)` +(`native/ios/src/lib.rs:828`); tvOS mirrors it. Touch input is then +scaled points→pixels to match (`native/tvos/src/lib.rs:267`), so the +convention is at least self-consistent — a deliberate-looking choice, +not an obvious slip. + +Two consequences, both real: + +1. **The physical-resolution glyph work is inert on iOS.** The shared + text renderer derives `dpi = surface_config.width / logical_width` + (`native/shared/src/text_renderer.rs:284-285`). With logical == + physical that is always 1.0, so the "rasterize glyphs at physical + resolution" change (60ba704) does nothing on Apple mobile — glyphs + rasterize at 1× of a pixel-sized font instead of at the backing + scale. Text is not blurry (sizes are already in pixels) but the + crispness win never lands, and 3× devices get 3×-smaller text than + the same source produces on macOS. +2. **Game-facing coordinates differ across Apple targets.** `screenWidth` + and 2D HUD coordinates are points on macOS and pixels on iOS, so the + same layout code does not carry across. + +**Decision needed (breaking either way).** Either (a) switch iOS/tvOS to +the macOS convention — pass points as logical, pixels as physical, and +stop pre-scaling touches — which fixes both consequences but changes +game-facing coordinates and will break existing iOS games' layout; or +(b) keep pixels and document the divergence explicitly, accepting that +`text_renderer`'s dpi path is dead on Apple mobile. Deliberately not +folded into the boot-path render-target fix (PR for EN-024's sibling +bug), which was a straight bug — this one is a product call. + +**Found by** the macOS/iOS parity audit, 2026-07-11. + diff --git a/native/shared/src/renderer/mod.rs b/native/shared/src/renderer/mod.rs index ada0a5c..e490c58 100644 --- a/native/shared/src/renderer/mod.rs +++ b/native/shared/src/renderer/mod.rs @@ -6082,7 +6082,7 @@ impl Renderer { ..Default::default() }); - Self { + let mut renderer = Self { headless_target: if surface.is_none() { Some(device.create_texture(&wgpu::TextureDescriptor { label: Some("headless_swapchain"), @@ -6530,7 +6530,21 @@ impl Renderer { composite_ldr_rt_b: None, composite_ldr_rt_b_view: None, post_pass_depth_sampler, - } + }; + + // The targets above are sized to the full surface, but the pass + // chain runs at `render_extent()` (surface * render_scale, 0.5 by + // default) and `resize` is the only thing that reconciles the two. + // Run it once here rather than trusting the host to: every + // `attach_engine` platform (macOS/iOS/tvOS/Linux/Android) resizes + // only when the OS size *changes*, so it never fires on frame 1 — + // those hosts rendered against targets that ignored render_scale, + // dropping post-FX and failing the depth-snapshot copy outright + // with a scene-reading translucent material in view. + let (pw, ph) = (renderer.surface_config.width, renderer.surface_config.height); + let (lw, lh) = (renderer.logical_width, renderer.logical_height); + renderer.resize(pw, ph, lw, lh); + renderer } /// Q1: Set up a render target override. The next end_frame will render to @@ -10732,6 +10746,15 @@ impl Renderer { self.surface_config.height } + /// Actual size of the render-resolution targets (depth / HDR / + /// G-buffer). Must always equal `render_extent()`: the pass chain + /// computes viewports and dispatches from the latter and writes into + /// the former. Pinned by `tests/render_targets.rs`. + pub fn render_target_extent(&self) -> (u32, u32) { + let s = self.depth_texture.size(); + (s.width, s.height) + } + pub fn surface_format(&self) -> wgpu::TextureFormat { self.surface_config.format } diff --git a/native/shared/tests/render_targets.rs b/native/shared/tests/render_targets.rs new file mode 100644 index 0000000..7348f37 --- /dev/null +++ b/native/shared/tests/render_targets.rs @@ -0,0 +1,76 @@ +//! Render-target sizing invariant. +//! +//! The pass chain computes viewports and compute dispatches from +//! `render_extent()` (surface * render_scale) and writes into the +//! depth/HDR/G-buffer targets. Those two must agree from the very first +//! frame. +//! +//! The golden suite cannot cover this: its harness calls +//! `set_taa_enabled(false)` immediately after construction, which +//! internally resizes and so always repairs the invariant before +//! anything renders. A real host does not — every `attach_engine` +//! platform (macOS/iOS/tvOS/Linux/Android) boots and then only resizes +//! when the OS-reported window size *changes*, which is false on frame +//! 1. That left those platforms rendering against targets sized to the +//! full surface while the passes ran at half extent: post-FX silently +//! dropped out, and the depth-snapshot copy failed validation outright +//! whenever a scene-reading translucent material was in view. + +use bloom_shared::renderer::Renderer; + +fn try_renderer() -> Option { + let instance = wgpu::Instance::new(wgpu::InstanceDescriptor { + backends: wgpu::Backends::all(), + ..wgpu::InstanceDescriptor::new_without_display_handle() + }); + let adapter = + pollster::block_on(instance.request_adapter(&wgpu::RequestAdapterOptions::default())) + .ok()?; + if adapter.get_info().device_type == wgpu::DeviceType::Cpu { + return None; + } + let (device, queue) = pollster::block_on(adapter.request_device(&wgpu::DeviceDescriptor { + required_limits: adapter.limits(), + ..Default::default() + })) + .ok()?; + Some(Renderer::new_headless(device, queue, 256, 256)) +} + +/// Straight out of the constructor — no resize, no TAA toggle, which is +/// exactly what a freshly attached host looks like on frame 1. +#[test] +fn render_targets_match_render_extent_at_construction() { + let Some(r) = try_renderer() else { + eprintln!("no GPU adapter — skipping"); + return; + }; + assert_eq!( + r.render_target_extent(), + r.render_extent(), + "render targets disagree with render_extent straight out of the \ + constructor — the boot path is not honouring render_scale" + ); +} + +/// The invariant must survive the scale/TAA setters and an explicit +/// resize, not just hold at construction. +#[test] +fn render_targets_track_render_extent_through_changes() { + let Some(mut r) = try_renderer() else { + eprintln!("no GPU adapter — skipping"); + return; + }; + + r.set_render_scale(1.0); + assert_eq!(r.render_target_extent(), r.render_extent(), "after set_render_scale(1.0)"); + + r.set_render_scale(0.5); + assert_eq!(r.render_target_extent(), r.render_extent(), "after set_render_scale(0.5)"); + + r.resize(640, 480, 640, 480); + assert_eq!(r.render_target_extent(), r.render_extent(), "after resize"); + + r.set_taa_enabled(false); + assert_eq!(r.render_target_extent(), r.render_extent(), "after set_taa_enabled(false)"); +} diff --git a/native/windows/src/lib.rs b/native/windows/src/lib.rs index 9c647ad..cfd6e52 100644 --- a/native/windows/src/lib.rs +++ b/native/windows/src/lib.rs @@ -675,15 +675,11 @@ unsafe fn init_engine_for_hwnd( }; surface.configure(&device, &surface_config); - let mut renderer = Renderer::new(device, queue, surface, surface_config, logical_w, logical_h); - // Route initial target creation through the same resize path a - // WM_SIZE takes. Without this, windowed mode (which never gets a - // resize, unlike the borderless-fullscreen transition) keeps - // construction-time render targets that ignore render_scale — - // the depth-snapshot copy then spans a partial depth texture, - // which wgpu rejects (fatal validation error on the first frame - // with a scene-reading translucent material in view). - renderer.resize(phys_w, phys_h, logical_w, logical_h); + // `Renderer::new` routes initial target creation through `resize` + // itself, so the construction-time targets already honour + // render_scale — no explicit resize needed here (it used to live + // here, which left every non-Windows host with the bug). + let renderer = Renderer::new(device, queue, surface, surface_config, logical_w, logical_h); let _ = ENGINE.set(EngineState::new(renderer)); }