Skip to content

Commit e06b225

Browse files
Fix: Window position creeps between executions on scaled monitors (#4443)
<!-- Please read the "Making a PR" section of [`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md) before opening a Pull Request! * Keep your PR:s small and focused. * The PR title is what ends up in the changelog, so make it descriptive! * If applicable, add a screenshot or gif. * If it is a non-trivial addition, consider adding a demo for it to `egui_demo_lib`, or a new example. * Do NOT open PR:s from your `master` branch, as that makes it hard for maintainers to add commits to your PR. * Remember to run `cargo fmt` and `cargo clippy`. * Open the PR as a draft until you have self-reviewed it and run `./scripts/check.sh`. * When you have addressed a PR comment, mark it as resolved. Please be patient! I will review your PR, but my time is limited! --> * Closes <#4442> * Refactors active monitor detection so it can be called from multiple locations. Compare this gif to the one on the issue report. ![egui_window_position_no_creep](https://github.com/emilk/egui/assets/45777186/8e05d4fb-266e-48b9-9223-b65f16500a99) ### Investigation notes - [`WindowSettings.inner_position_pixels` and `WindowSettings.outer_position_pixels`](https://github.com/emilk/egui/blob/master/crates/egui-winit/src/window_settings.rs#L8-L12) are stored in physical/pixel coordinates. - `ViewportBuilder::with_position` expects to be passed a position in _logical_ coordinates. - Prior to this PR, the position was being passed from `WindowSettings` to `with_position` [without any scaling](https://github.com/emilk/egui/blob/master/crates/egui-winit/src/window_settings.rs#L61-L68). This was the root cause of the issue. - The fix is to first convert the position to logical coordinates, respecting the scaling factor of the active monitor. This requires us to first determine the active monitor, so I factored out some of the logic in [`clamp_pos_to_monitor`](https://github.com/emilk/egui/blob/master/crates/egui-winit/src/window_settings.rs#L130) to find the active monitor.
1 parent 66d2b3f commit e06b225

File tree

2 files changed

+38
-7
lines changed

2 files changed

+38
-7
lines changed

crates/eframe/src/native/epi_integration.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,11 @@ pub fn viewport_builder<E>(
3838
}
3939
window_settings.clamp_position_to_monitors(egui_zoom_factor, event_loop);
4040

41-
viewport_builder = window_settings.initialize_viewport_builder(viewport_builder);
41+
viewport_builder = window_settings.initialize_viewport_builder(
42+
egui_zoom_factor,
43+
event_loop,
44+
viewport_builder,
45+
);
4246
window_settings.inner_size_points()
4347
} else {
4448
if let Some(pos) = viewport_builder.position {

crates/egui-winit/src/window_settings.rs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ impl WindowSettings {
5050
self.inner_size_points
5151
}
5252

53-
pub fn initialize_viewport_builder(
53+
pub fn initialize_viewport_builder<E>(
5454
&self,
55+
egui_zoom_factor: f32,
56+
event_loop: &winit::event_loop::EventLoopWindowTarget<E>,
5557
mut viewport_builder: ViewportBuilder,
5658
) -> ViewportBuilder {
5759
crate::profile_function!();
@@ -64,7 +66,15 @@ impl WindowSettings {
6466
self.outer_position_pixels
6567
};
6668
if let Some(pos) = pos_px {
67-
viewport_builder = viewport_builder.with_position(pos);
69+
let monitor_scale_factor = if let Some(inner_size_points) = self.inner_size_points {
70+
find_active_monitor(egui_zoom_factor, event_loop, inner_size_points, &pos)
71+
.map_or(1.0, |monitor| monitor.scale_factor() as f32)
72+
} else {
73+
1.0
74+
};
75+
76+
let scaled_pos = pos / (egui_zoom_factor * monitor_scale_factor);
77+
viewport_builder = viewport_builder.with_position(scaled_pos);
6878
}
6979

7080
if let Some(inner_size_points) = self.inner_size_points {
@@ -127,12 +137,12 @@ impl WindowSettings {
127137
}
128138
}
129139

130-
fn clamp_pos_to_monitors<E>(
140+
fn find_active_monitor<E>(
131141
egui_zoom_factor: f32,
132142
event_loop: &winit::event_loop::EventLoopWindowTarget<E>,
133143
window_size_pts: egui::Vec2,
134-
position_px: &mut egui::Pos2,
135-
) {
144+
position_px: &egui::Pos2,
145+
) -> Option<winit::monitor::MonitorHandle> {
136146
crate::profile_function!();
137147

138148
let monitors = event_loop.available_monitors();
@@ -142,7 +152,7 @@ fn clamp_pos_to_monitors<E>(
142152
.primary_monitor()
143153
.or_else(|| event_loop.available_monitors().next())
144154
else {
145-
return; // no monitors 🤷
155+
return None; // no monitors 🤷
146156
};
147157

148158
for monitor in monitors {
@@ -159,6 +169,23 @@ fn clamp_pos_to_monitors<E>(
159169
}
160170
}
161171

172+
Some(active_monitor)
173+
}
174+
175+
fn clamp_pos_to_monitors<E>(
176+
egui_zoom_factor: f32,
177+
event_loop: &winit::event_loop::EventLoopWindowTarget<E>,
178+
window_size_pts: egui::Vec2,
179+
position_px: &mut egui::Pos2,
180+
) {
181+
crate::profile_function!();
182+
183+
let Some(active_monitor) =
184+
find_active_monitor(egui_zoom_factor, event_loop, window_size_pts, position_px)
185+
else {
186+
return; // no monitors 🤷
187+
};
188+
162189
let mut window_size_px =
163190
window_size_pts * (egui_zoom_factor * active_monitor.scale_factor() as f32);
164191
// Add size of title bar. This is 32 px by default in Win 10/11.

0 commit comments

Comments
 (0)