Skip to content

Commit

Permalink
Fix initial size of windows on multi-monitor systems
Browse files Browse the repository at this point in the history
  • Loading branch information
emilk committed Nov 30, 2023
1 parent 37244e3 commit 13df4ba
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 42 deletions.
28 changes: 15 additions & 13 deletions crates/eframe/src/native/glow_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ use egui::{
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
use egui_winit::{
apply_viewport_builder_to_new_window, create_winit_window_builder, process_viewport_commands,
EventResponse,
};

use crate::{
native::{epi_integration::EpiIntegration, winit_integration::create_egui_context},
Expand Down Expand Up @@ -744,7 +740,7 @@ impl GlowWinitRunning {
return EventResult::Exit;
}

let mut event_response = EventResponse {
let mut event_response = egui_winit::EventResponse {
consumed: false,
repaint: false,
};
Expand Down Expand Up @@ -823,7 +819,7 @@ impl GlutinWindowContext {
let display_builder = glutin_winit::DisplayBuilder::new()
// we might want to expose this option to users in the future. maybe using an env var or using native_options.
.with_preference(glutin_winit::ApiPrefence::FallbackEgl) // https://github.com/emilk/egui/issues/2520#issuecomment-1367841150
.with_window_builder(Some(create_winit_window_builder(
.with_window_builder(Some(egui_winit::create_winit_window_builder(
egui_ctx,
event_loop,
viewport_builder.clone(),
Expand All @@ -849,7 +845,7 @@ impl GlutinWindowContext {
.map_err(|e| crate::Error::NoGlutinConfigs(config_template_builder.build(), e))?
};
if let Some(window) = &window {
apply_viewport_builder_to_new_window(window, &viewport_builder);
egui_winit::apply_viewport_builder_to_window(egui_ctx, window, &viewport_builder);
}

let gl_display = gl_config.display();
Expand Down Expand Up @@ -981,12 +977,18 @@ impl GlutinWindowContext {
window
} else {
log::trace!("Window doesn't exist yet. Creating one now with finalize_window");
let window = glutin_winit::finalize_window(
let window_builder = egui_winit::create_winit_window_builder(
&self.egui_ctx,
event_loop,
create_winit_window_builder(&self.egui_ctx, event_loop, viewport.builder.clone()),
&self.gl_config,
)?;
apply_viewport_builder_to_new_window(&window, &viewport.builder);
viewport.builder.clone(),
);
let window =
glutin_winit::finalize_window(event_loop, window_builder, &self.gl_config)?;
egui_winit::apply_viewport_builder_to_window(
&self.egui_ctx,
&window,
&viewport.builder,
);
viewport.info.minimized = window.is_minimized();
viewport.info.maximized = Some(window.is_maximized());
viewport.window.insert(Rc::new(window))
Expand Down Expand Up @@ -1235,7 +1237,7 @@ fn initialize_or_update_viewport<'vp>(
viewport.egui_winit = None;
} else if let Some(window) = &viewport.window {
let is_viewport_focused = focused_viewport == Some(ids.this);
process_viewport_commands(
egui_winit::process_viewport_commands(
egu_ctx,
&mut viewport.info,
delta_commands,
Expand Down
18 changes: 3 additions & 15 deletions crates/eframe/src/native/wgpu_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ use egui::{
};
#[cfg(feature = "accesskit")]
use egui_winit::accesskit_winit;
use egui_winit::{
apply_viewport_builder_to_new_window, create_winit_window_builder, process_viewport_commands,
};

use crate::{
native::{epi_integration::EpiIntegration, winit_integration::EventResult},
Expand Down Expand Up @@ -779,12 +776,8 @@ impl Viewport {

let viewport_id = self.ids.this;

match create_winit_window_builder(egui_ctx, event_loop, self.builder.clone())
.build(event_loop)
{
match egui_winit::create_window(egui_ctx, event_loop, &self.builder) {
Ok(window) => {
apply_viewport_builder_to_new_window(&window, &self.builder);

windows_id.insert(window.id(), viewport_id);

if let Err(err) = pollster::block_on(painter.set_window(viewport_id, Some(&window)))
Expand Down Expand Up @@ -840,12 +833,7 @@ fn create_window(
)
.with_visible(false); // Start hidden until we render the first frame to fix white flash on startup (https://github.com/emilk/egui/pull/3631)

let window = {
crate::profile_scope!("WindowBuilder::build");
create_winit_window_builder(egui_ctx, event_loop, viewport_builder.clone())
.build(event_loop)?
};
apply_viewport_builder_to_new_window(&window, &viewport_builder);
let window = egui_winit::create_window(egui_ctx, event_loop, &viewport_builder)?;
epi_integration::apply_window_settings(&window, window_settings);
Ok((window, viewport_builder))
}
Expand Down Expand Up @@ -1056,7 +1044,7 @@ fn initialize_or_update_viewport<'vp>(
viewport.egui_winit = None;
} else if let Some(window) = &viewport.window {
let is_viewport_focused = focused_viewport == Some(ids.this);
process_viewport_commands(
egui_winit::process_viewport_commands(
egui_ctx,
&mut viewport.info,
delta_commands,
Expand Down
98 changes: 85 additions & 13 deletions crates/egui-winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,26 @@ fn process_viewport_command(
}
}

/// Build and intitlaize a window.
///
/// Wrapper around `create_winit_window_builder` and `apply_viewport_builder_to_window`.
pub fn create_window<T>(
egui_ctx: &egui::Context,
event_loop: &EventLoopWindowTarget<T>,
viewport_builder: &ViewportBuilder,
) -> Result<Window, winit::error::OsError> {
crate::profile_function!();

let window_builder =
create_winit_window_builder(egui_ctx, event_loop, viewport_builder.clone());
let window = {
crate::profile_scope!("WindowBuilder::build");
window_builder.build(event_loop)?
};
apply_viewport_builder_to_window(egui_ctx, &window, viewport_builder);
Ok(window)
}

pub fn create_winit_window_builder<T>(
egui_ctx: &egui::Context,
event_loop: &EventLoopWindowTarget<T>,
Expand All @@ -1253,6 +1273,8 @@ pub fn create_winit_window_builder<T>(

// We set sizes and positions in egui:s own ui points, which depends on the egui
// zoom_factor and the native pixels per point, so we need to know that here.
// We don't know what monitor the window will appear on though, but
// we'll try to fix that after the window is created in the vall to `apply_viewport_builder_to_window`.
let native_pixels_per_point = event_loop
.primary_monitor()
.or_else(|| event_loop.available_monitors().next())
Expand Down Expand Up @@ -1297,7 +1319,7 @@ pub fn create_winit_window_builder<T>(
// wayland:
app_id: _app_id,

mouse_passthrough: _, // handled in `apply_viewport_builder_to_new_window`
mouse_passthrough: _, // handled in `apply_viewport_builder_to_window`
} = viewport_builder;

let mut window_builder = winit::window::WindowBuilder::new()
Expand Down Expand Up @@ -1330,31 +1352,31 @@ pub fn create_winit_window_builder<T>(
})
.with_active(active.unwrap_or(true));

if let Some(inner_size) = inner_size {
if let Some(size) = inner_size {
window_builder = window_builder.with_inner_size(PhysicalSize::new(
pixels_per_point * inner_size.x,
pixels_per_point * inner_size.y,
pixels_per_point * size.x,
pixels_per_point * size.y,
));
}

if let Some(min_inner_size) = min_inner_size {
if let Some(size) = min_inner_size {
window_builder = window_builder.with_min_inner_size(PhysicalSize::new(
pixels_per_point * min_inner_size.x,
pixels_per_point * min_inner_size.y,
pixels_per_point * size.x,
pixels_per_point * size.y,
));
}

if let Some(max_inner_size) = max_inner_size {
if let Some(size) = max_inner_size {
window_builder = window_builder.with_max_inner_size(PhysicalSize::new(
pixels_per_point * max_inner_size.x,
pixels_per_point * max_inner_size.y,
pixels_per_point * size.x,
pixels_per_point * size.y,
));
}

if let Some(position) = position {
if let Some(pos) = position {
window_builder = window_builder.with_position(PhysicalPosition::new(
pixels_per_point * position.x,
pixels_per_point * position.y,
pixels_per_point * pos.x,
pixels_per_point * pos.y,
));
}

Expand Down Expand Up @@ -1391,6 +1413,7 @@ pub fn create_winit_window_builder<T>(
}

/// Applies what `create_winit_window_builder` couldn't
#[deprecated = "Use apply_viewport_builder_to_window instead"]
pub fn apply_viewport_builder_to_new_window(window: &Window, builder: &ViewportBuilder) {
if let Some(mouse_passthrough) = builder.mouse_passthrough {
if let Err(err) = window.set_cursor_hittest(!mouse_passthrough) {
Expand All @@ -1399,6 +1422,55 @@ pub fn apply_viewport_builder_to_new_window(window: &Window, builder: &ViewportB
}
}

/// Applies what `create_winit_window_builder` couldn't
pub fn apply_viewport_builder_to_window(
egui_ctx: &egui::Context,
window: &Window,
builder: &ViewportBuilder,
) {
if let Some(mouse_passthrough) = builder.mouse_passthrough {
if let Err(err) = window.set_cursor_hittest(!mouse_passthrough) {
log::warn!("set_cursor_hittest failed: {err}");
}
}

if let Some(monitor) = window.current_monitor() {
// In `create_winit_window_builder` we didn't know
// on what monitor the window would appear, so we didn't know
// how to translate egui ui point to native physical pixels.
// Now we do know:

let native_pixels_per_point = monitor.scale_factor() as f32;
let zoom_factor = egui_ctx.zoom_factor();
let pixels_per_point = zoom_factor * native_pixels_per_point;

if let Some(size) = builder.inner_size {
window.set_inner_size(PhysicalSize::new(
pixels_per_point * size.x,
pixels_per_point * size.y,
));
}
if let Some(size) = builder.min_inner_size {
window.set_min_inner_size(Some(PhysicalSize::new(
pixels_per_point * size.x,
pixels_per_point * size.y,
)));
}
if let Some(size) = builder.max_inner_size {
window.set_max_inner_size(Some(PhysicalSize::new(
pixels_per_point * size.x,
pixels_per_point * size.y,
)));
}
if let Some(pos) = builder.position {
let pos = PhysicalPosition::new(pixels_per_point * pos.x, pixels_per_point * pos.y);
window.set_outer_position(pos);
}
} else {
log::debug!("Couldn't figure out what monitor the new window is on, so the window size will likely be wrong.");
}
}

// ---------------------------------------------------------------------------

/// Short and fast description of an event.
Expand Down
4 changes: 3 additions & 1 deletion crates/egui/src/viewport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ pub struct ViewportBuilder {
/// This is wayland only. See [`Self::with_app_id`].
pub app_id: Option<String>,

/// The desired outer position of the window.
pub position: Option<Pos2>,
pub inner_size: Option<Vec2>,
pub min_inner_size: Option<Vec2>,
Expand Down Expand Up @@ -506,7 +507,8 @@ impl ViewportBuilder {
self
}

/// This will probably not work as expected!
/// The initial "outer" position of the window,
/// i.e. where the top-left corner of the frame/chrome should be.
#[inline]
pub fn with_position(mut self, pos: impl Into<Pos2>) -> Self {
self.position = Some(pos.into());
Expand Down

0 comments on commit 13df4ba

Please sign in to comment.