From 86aae77e5e4ac03dfe74e79b50c497258964e871 Mon Sep 17 00:00:00 2001 From: goring <67476208+goring@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:34:00 +0000 Subject: [PATCH] gpui: Add scale-factor simulation to `TestWindow` (#63719) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > **AI disclosure:** I used Claude (Anthropic) to write this change, its test, and draft this PR description, after my own app shipped a rendering bug that only reproduces on 1x displays — every test built on gpui's test platform renders at a fixed 2.0 scale factor, so the bug was invisible to an otherwise thorough offscreen test suite. I reviewed the diff, ran the tests myself, and understand and take responsibility for the change, per Zed's [AI Policy](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#ai-policy). # Objective - `TestWindow::scale_factor()` has returned a hardcoded `2.0` since the test platform's inception (`75f0670dd`, 2024-01-03), with no way for a test to change it. Rendering behavior that only appears on 1x displays — device-grid rounding, pixel snapping — can't be exercised: an error of one device pixel is the full width of a hairline at 1x and half a logical pixel at 2x, so this class of bug passes every test and ships. - `Window::set_scale_factor` (`24058c865`, #61348, 2025-08) exists but doesn't cover this: it overrides only the `Window`'s cached copy, so the next `bounds_changed` re-reads the platform window and silently reverts the override to 2.0. ## Solution - Back the value with `TestWindowState`, still defaulting to `2.0`, so existing tests are unchanged. - Add `TestWindow::simulate_scale_factor_change`, which stores the new factor and reports it through the resize callback — the same channel a real DPI change reaches the window on — plus the matching wrappers on `TestAppContext`, `VisualTestContext`, and `TestAppWindow`, mirroring the `simulate_resize` family. - Because the platform window owns the value, it survives later bounds changes (unlike `set_scale_factor`); the test pins that. ## Testing - Added `test_simulate_scale_factor_change` in `crates/gpui/src/window.rs`: asserts the unchanged 2.0 default, that a simulated change propagates to `Window::scale_factor()` through the resize callback, and that it persists across a subsequent `simulate_resize`. - Reverting `scale_factor()` to the hardcoded `2.0` while keeping the test reproduces the failure (see Showcase), so the test pins the behaviour rather than passing vacuously. - `cargo test -p gpui --lib` — 299 passed, 0 failed: ``` running 299 tests ... test window::tests::test_simulate_scale_factor_change ... ok ... test result: ok. 299 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.23s ``` - Tested on macOS. The change is confined to test-support code (`platform/test/`, test contexts), which is platform independent, but I have not built the Linux or Windows platform crates. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [ ] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase Before — the new test against the hardcoded `2.0`: ``` thread 'window::tests::test_simulate_scale_factor_change' panicked at crates/gpui/src/window.rs:7230:9: assertion `left == right` failed left: 2.0 right: 1.0 test result: FAILED. 0 passed; 1 failed ``` After — see the full run above. Release Notes: - N/A --------- Co-authored-by: Smit Barmase --- crates/gpui/src/app/test_app.rs | 38 ++++++++++++++++++++++++ crates/gpui/src/app/test_context.rs | 15 ++++++++++ crates/gpui/src/platform/test/window.rs | 15 +++++++++- crates/gpui/src/window.rs | 39 +++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 1 deletion(-) diff --git a/crates/gpui/src/app/test_app.rs b/crates/gpui/src/app/test_app.rs index d241863883..bf265fd7bf 100644 --- a/crates/gpui/src/app/test_app.rs +++ b/crates/gpui/src/app/test_app.rs @@ -481,6 +481,24 @@ impl TestAppWindow { self.background_executor.run_until_parked(); } + /// Simulate the window moving to a display with a different scale factor. + pub fn simulate_scale_factor_change(&mut self, scale_factor: f32) { + let window_id = self.handle.window_id(); + let test_window = { + let mut app = self.app.borrow_mut(); + app.windows + .get_mut(window_id) + .and_then(|window| window.as_mut()) + .and_then(|window| window.platform_window.as_test()) + .cloned() + }; + // The resize callback needs to borrow the app again synchronously. + if let Some(mut test_window) = test_window { + test_window.simulate_scale_factor_change(scale_factor); + } + self.background_executor.run_until_parked(); + } + /// Force a redraw of the window. pub fn draw(&mut self) { let mut app = self.app.borrow_mut(); @@ -557,6 +575,26 @@ mod tests { app.update(|cx| cx.shutdown()); } + #[test] + fn test_simulate_scale_factor_change() { + let mut app = TestApp::new(); + let mut window = app.open_window(|_, _| crate::EmptyView); + let viewport_size = window.update(|_, window, _| { + assert_eq!(window.scale_factor(), 2.0); + window.viewport_size() + }); + + window.simulate_scale_factor_change(1.0); + + window.update(|_, window, _| { + assert_eq!(window.scale_factor(), 1.0); + assert_eq!(window.viewport_size(), viewport_size); + }); + + drop(window); + app.update(|cx| cx.shutdown()); + } + #[test] fn test_entity_creation() { let mut app = TestApp::new(); diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index 1dceee09d7..a34a779007 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -403,6 +403,16 @@ impl TestAppContext { self.test_window(window_handle).simulate_resize(size); } + /// Simulates the window moving to a display with a different scale factor. + pub fn simulate_window_scale_factor_change( + &self, + window_handle: AnyWindowHandle, + scale_factor: f32, + ) { + self.test_window(window_handle) + .simulate_scale_factor_change(scale_factor); + } + /// Returns true if there's an alert dialog open. pub fn expect_restart(&self) -> oneshot::Receiver<(Option, Vec)> { let (tx, rx) = futures::channel::oneshot::channel(); @@ -884,6 +894,11 @@ impl VisualTestContext { self.simulate_window_resize(self.window, size) } + /// Simulates the window moving to a display with a different scale factor. + pub fn simulate_scale_factor_change(&self, scale_factor: f32) { + self.simulate_window_scale_factor_change(self.window, scale_factor) + } + /// debug_bounds returns the bounds of the element with the given selector. pub fn debug_bounds(&mut self, selector: &'static str) -> Option> { self.update(|window, _| window.rendered_frame.debug_bounds.get(selector).copied()) diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index 38926b649c..5a07bc5c01 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -46,6 +46,7 @@ pub(crate) struct TestWindowState { text_input_configurations: Vec, text_input_state_changes: Vec, is_fullscreen: bool, + scale_factor: f32, appearance: WindowAppearance, external_drag_files: Vec<(PathBuf, bool)>, start_external_drag_result: bool, @@ -110,6 +111,8 @@ impl TestWindow { text_input_configurations: Vec::new(), text_input_state_changes: Vec::new(), is_fullscreen: false, + // Preserve the test platform's historical 2x default. + scale_factor: 2.0, appearance: WindowAppearance::Light, external_drag_files: Vec::new(), start_external_drag_result: false, @@ -160,6 +163,16 @@ impl TestWindow { self.0.lock().resize_callback = Some(callback); } + /// Simulates a display scale change through the resize callback, preserving logical bounds. + pub fn simulate_scale_factor_change(&mut self, scale_factor: f32) { + let size = { + let mut lock = self.0.lock(); + lock.scale_factor = scale_factor; + lock.bounds.size + }; + self.simulate_resize(size); + } + pub(crate) fn simulate_active_status_change(&self, active: bool) { let mut lock = self.0.lock(); let Some(mut callback) = lock.active_status_change_callback.take() else { @@ -241,7 +254,7 @@ impl PlatformWindow for TestWindow { } fn scale_factor(&self) -> f32 { - 2.0 + self.0.lock().scale_factor } fn appearance(&self) -> WindowAppearance { diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index cf8eded704..606623b5b0 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -7452,6 +7452,45 @@ mod tests { .unwrap(); } + #[test] + fn test_scale_factor_change_preserves_bounds_and_survives_resize() { + let mut cx = TestAppContext::single(); + let window = cx.add_window(|_, _| EmptyView); + let handle: AnyWindowHandle = window.into(); + let window_state = |cx: &mut TestAppContext| { + cx.update_window(handle, |_, window, _| { + ( + window.scale_factor(), + window.bounds(), + window.viewport_size(), + ) + }) + .unwrap() + }; + + let (scale_factor, mut expected_bounds, _) = window_state(&mut cx); + assert_eq!(scale_factor, 2.0); + + for (scale_factor, resized_size) in [ + (1.0, size(px(800.), px(600.))), + (1.25, size(px(640.), px(480.))), + (2.0, size(px(1024.), px(768.))), + ] { + cx.simulate_window_scale_factor_change(handle, scale_factor); + assert_eq!( + window_state(&mut cx), + (scale_factor, expected_bounds, expected_bounds.size) + ); + + cx.simulate_window_resize(handle, resized_size); + expected_bounds.size = resized_size; + assert_eq!( + window_state(&mut cx), + (scale_factor, expected_bounds, resized_size) + ); + } + } + /// Platforms that stop requesting frames for idle windows (currently web) /// rely on the frame waker firing whenever frame demand arises; a demand /// source that skips the waker shows up there as a window that silently