gpui: Add scale-factor simulation to TestWindow (#63719)
> **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 <heysmitbarmase@gmail.com>
This commit is contained in:
1 parent
d977c888b2
commit
86aae77e5e
4 files changed
+106
-1
No files matched your search
@@ -481,6 +481,24 @@ impl<V: 'static + Render> TestAppWindow<V> {
|
||||
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();
|
||||
|
||||
@@ -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<PathBuf>, Vec<std::ffi::OsString>)> {
|
||||
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<Bounds<Pixels>> {
|
||||
self.update(|window, _| window.rendered_frame.debug_bounds.get(selector).copied())
|
||||
|
||||
@@ -46,6 +46,7 @@ pub(crate) struct TestWindowState {
|
||||
text_input_configurations: Vec<TextInputConfiguration>,
|
||||
text_input_state_changes: Vec<TextInputStateChange>,
|
||||
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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user