rust-windowing / winit

Window handling library in pure Rust
https://docs.rs/winit/
Apache License 2.0
4.74k stars 890 forks source link

Erroneous `WindowEvent::Resized `sent on app startup #2094

Open aloucks opened 2 years ago

aloucks commented 2 years ago

Why is the window being resized to near-full-screen and then back to the original value on application startup?

WindowEvent { window_id: WindowId(WindowId(0x7606be)), event: Resized(PhysicalSize { width: 1904, height: 990 }) }

This can be reproduced with the window example:

$ cargo run --example window
    Finished dev [unoptimized + debuginfo] target(s) in 0.08s
     Running `target\debug\examples\window.exe`
NewEvents(Init)
WindowEvent { window_id: WindowId(WindowId(0x7606be)), event: Focused(true) }
WindowEvent { window_id: WindowId(WindowId(0x7606be)), event: Resized(PhysicalSize { width: 1904, height: 990 }) }
WindowEvent { window_id: WindowId(WindowId(0x7606be)), event: Resized(PhysicalSize { width: 128, height: 128 }) }
WindowEvent { window_id: WindowId(WindowId(0x7606be)), event: Resized(PhysicalSize { width: 128, height: 128 }) }

Platform: Windows 10

This behavior can also be observed in the wgpu examples (which report vulkan validation errors if you have debug logging enabled).

filnet commented 2 years ago

I am seeing the same behavior after switching from 0.25 to 0.26.

A workaround is to ignore resize events when the start cause in NewEvents is StartCause::Init.

filnet commented 2 years ago

Seems related to https://github.com/rust-windowing/winit/commit/3ecbea3c39c1f8d33048a855caa359b1a01cf488. I did not confirm that though...

filnet commented 2 years ago

I am getting:

Resized(PhysicalSize { width: 2858, height: 1490 })
Resized(PhysicalSize { width: 1920, height: 1080 })
Resized(PhysicalSize { width: 1920, height: 1080 })

With a native physical size of 3840 x 2160.

The first resize seems random and has no obvious relation to the native size. The 2858 / 3840 ratio is 0.7442 which cannot be explained by, for example, the dpi scale factor (1.5 in my case).

filnet commented 2 years ago

PS : the first size might come from Windows 10 default window size.

msiglreith commented 2 years ago

My initial assumption would be that due to moving parts from WM_CREATE to WM_NCCREATE there are some additional events in between which we should filter out,

aloucks commented 2 years ago

It's definitely related to splitting the window creation across WM_CREATE and WM_NCCREATE.

https://github.com/rust-windowing/winit/blob/438d286fd5d5b0c007836bf88766ce920576c73c/src/platform_impl/windows/window.rs#L810-L821

The Resized events have the correct values if I move win.set_visible(attributes.visible) below win.set_inner_size(dimensions). However, I don't know what side effects this may incur.

I'm still a little surprised by the fact that there are still multiple Resized events being emitted during startup.

aloucks commented 2 years ago

I thought that the comment above set_visible may be related to borderless windows, but borderless seems to also be broken (setting .with_decorations(false) in the window examples shows nothing)

maroider commented 2 years ago

The calls to set_visible and set_inner_size are where they are because of #1994 (which fixed a regression introduced by #1933)

msiglreith commented 2 years ago

Decorations appear fine when drawing something on the screen, which the window example doesn't do. We could probably force decorations during the non-client area process and disable it on client area creation step.

zxz149res commented 2 years ago

I'm using wgpu (with Vulkan backend) with winit on Windows.

let event_loop = EventLoop::new();
// the following line will call CreateWindowA with CW_USEDEFAULT as width and height. Windows will choose an initial size
let window = WindowBuilder::new().with_inner_size(winit::dpi::PhysicalSize{width: 800, height: 600}).build(&event_loop).unwrap();
 // so Windows thinks the window inner size is (2862, 1510), but winit thinks it (800, 600). (i'm using 3840x2160 monitor with scale factor 1.25)
setup_wgpu(); // vulkan will complains here because of the inconsistency
// event_loop.run() will resize the window on WM_CREATE / WM_NCCREATE
event_loop.run(call_back);

The inner size is inconsistent between Window::new() and event_loop.run(). I can see the window has a big initial size and then flashes to 800x600 and Vulkan validation layer complains about it.

I propose winit to set window size on CreateWindowA() instead of postpone it until event_loop.run().

winit call CreateWindowExW with CW_USEDEFAULT

dhardy commented 2 years ago

I see the same behaviour (basically) on Wayland:

kchibisov commented 2 years ago

Could you provide WAYLAND_DEBUG logs?

dhardy commented 2 years ago

Scale factors 100% and 150%: logs.zip

kchibisov commented 2 years ago

Are you sure that you've reproduced your issue in log with scale 1.5? Could you provide interleaved log from winit and from WAYLAND_DEBUG=1 during you reproducing the issue? I have a guess what the problem is, but this particular log looks fine to me.

dhardy commented 2 years ago

Hopefully this is better? log.gz

Not sure what you mean by the winit log since it doesn't appear to have one (here I enabled trace level but minimised the noisy stuff):

export RUST_LOG=trace,wgpu_core=warn,wgpu_hal=warn,kas_text=warn,kas_wgpu=debug,kas=info,naga=error
kchibisov commented 2 years ago

@dhardy Seems like gnome( I guess it's gnome) decided to resize you, nothing we can do here?

[ 650698.724] wl_keyboard@16.modifiers(28615, 8, 0, 0, 0)
[ 650698.857] zwp_text_input_v3@18.enter(wl_surface@19)
[ 650698.911]  -> zwp_text_input_v3@18.enable()
[ 650698.931]  -> zwp_text_input_v3@18.commit()
[ 650698.961] wl_keyboard@16.modifiers(28625, 0, 0, 0, 0)
[ 650699.039] xdg_toplevel@23.configure(484, 712, array[4])
[ 650699.141] xdg_surface@22.configure(28627)
[ 650699.178]  -> xdg_surface@22.ack_configure(28627)
[ 650699.349]  -> xdg_surface@22.set_window_geometry(0, 0, 484, 712)
[ 650700.392] wl_buffer@55.release()
[ 650703.346]  -> wl_surface@19.attach(wl_buffer@55, 0, 0)
dhardy commented 2 years ago

KDE/kwin actually. The real question is not so much why the compositor resized the window on Alt+Tab but why it resized it after construction:

Scale factor: 2
Min inner window size: Size(272, 196)
Max inner window size: Size(968, 1423)
[ 648039.378]  -> wl_compositor@4.create_surface(new id wl_surface@19)
[ 648039.424]  -> wl_registry@2.bind(5, "xdg_wm_base", 2, new id [unknown]@20)
[ 648039.467]  -> wl_shm@6.create_pool(new id wl_shm_pool@21, fd 21, 4096)
[ 648039.501]  -> xdg_wm_base@20.get_xdg_surface(new id xdg_surface@22, wl_surface@19)
[ 648039.520]  -> xdg_surface@22.get_toplevel(new id xdg_toplevel@23)
[ 648039.533]  -> wl_surface@19.commit()
[ 648039.543]  -> xdg_toplevel@23.set_min_size(2, 1)
[ 648039.552]  -> xdg_surface@22.set_window_geometry(0, 0, 968, 1423)
[ 648039.580]  -> wl_compositor@4.create_surface(new id wl_surface@24)
[ 648039.592]  -> wl_seat@7.get_pointer(new id wl_pointer@25)
[ 648039.622]  -> zxdg_decoration_manager_v1@5.get_toplevel_decoration(new id zxdg_toplevel_decoration_v1@26, xdg_toplevel@23)
[ 648039.641]  -> zxdg_toplevel_decoration_v1@26.unset_mode()
[ 648039.645]  -> wl_surface@19.commit()
[ 648039.652]  -> xdg_toplevel@23.set_min_size(272, 196)
[ 648039.661]  -> xdg_toplevel@23.set_max_size(968, 1423)
[ 648039.669]  -> xdg_toplevel@23.set_min_size(272, 196)
[ 648039.677]  -> xdg_toplevel@23.set_max_size(968, 1423)
[ 648039.686]  -> xdg_toplevel@23.set_title("Widget Gallery")
[ 648039.716]  -> wl_display@1.sync(new id wl_callback@27)
[ 648040.749] wl_display@1.delete_id(27)
[ 648040.759] wl_keyboard@16.repeat_info(25, 600)
[ 648040.779] wl_keyboard@16.keymap(1, fd 21, 66099)
[ 648042.148] zxdg_toplevel_decoration_v1@26.configure(2)
[ 648042.172] zxdg_toplevel_decoration_v1@26.configure(2)
[ 648042.179] wl_callback@27.done(28588)
[ 648042.184] xdg_toplevel@23.configure(0, 0, array[0])
[ 648042.203] xdg_surface@22.configure(28589)
[ 648042.213]  -> xdg_surface@22.ack_configure(28589)
[2022-03-15T07:57:39Z INFO  kas_wgpu::window] Constucted new window with size Size(968, 1423)

The Size(..) values are all in physical pixels, but what about xdg_surface@22.set_window_geometry? In one case it has width 968 and the other 484. Also xdg_toplevel@23.set_max_size(968, 1423). I don't know whether Wayland uses physical or logical pixels here?

kchibisov commented 2 years ago

On Wayland all windows are created at scale factor one, and then got resized/rescaled to other scale factor. The resize is expected. See https://gitlab.freedesktop.org/wayland/wayland/-/issues/133.

kchibisov commented 2 years ago

Max size and min size are in window geometry coordinates. So they are logical.

dhardy commented 2 years ago

But the behaviour I see is that the window is too large on construction, but the correct size after Alt+Tab. Hence xdg_surface@22.set_window_geometry(0, 0, 968, 1423) and xdg_toplevel@23.set_max_size(968, 1423) are both wrong if these are logical coordinates... except, going by what you say, these are "logical coordinates with scale factor 1", even though winit already knows the scale factor is 2. Confusing.

dhardy commented 2 years ago

In fact, the KAS toolkit "cheats" by looking up the scale factor from the previous window constructed or (initially) from the list of available screens, thus it calculates the size requirements (using physical pixels internally) using scale factor 2. It sounds like Wayland assumes the window size will be calculated in logical pixels.

Thus the issue is probably that winit accepts size requests through WindowBuilder in physical pixels but doesn't handle these correctly. (I'm not sure how it should, to be honest, given that Wayland apparently doesn't appear to let windows address raw pixels.)

kchibisov commented 2 years ago

You don't know scale factor your window will use even if you probe all monitors. You can't just assume that it's 2, you should use 1, and wait for ScaleFactorChanged.

kchibisov commented 2 years ago

But yeah, wayland is entirely in logical pixels, only buffer size is physical.

dhardy commented 2 years ago

You don't know scale factor your window will use even if you probe all monitors. You can't just assume that it's 2, you should use 1, and wait for ScaleFactorChanged.

No, but I can guess. If I remember correctly, I wrote this code to make the initial size correct on X11. Can't I get both right?

The way I see it, it's currently undefined (or improperly documented) how sizes passed in physical or logical pixels (dependant on the platform) are handled before the window's scale factor is known.

kchibisov commented 2 years ago

If you provide size in logical pixels you should get correct window size regarding on window scale factor? Since on X11 I bet it can know that it's 2.

The problem is that creating window in physical pixels as well as using physical pixels is unreliable and overly complicated, I'd say.

kchibisov commented 2 years ago

The thing winit's Wayland backend is doing makes sense to me, since it converts your size to logical size with a scale factor surface has right now. If you've passed the physical size computed for scale factor 2 you're out of luck, since winit knows scale factor 1, not 2.

And logical is required, since Wayland's window API is entirely in logical pixels.

dhardy commented 2 years ago

So, on X11, using my scale-factor-detection methodology and physical pixels:

Scale factor: 1.5
Min inner window size: Size(221, 166)
Max inner window size: Size(727, 1114)
Resized to: PhysicalSize { width: 727, height: 1114 }

And the result looks correct.

Instead using scale-factor=1 to calculate sizes and passing as logical sizes:

Initial scale factor: 1
Min inner window size: Size(135, 97)
Max inner window size: Size(484, 707)
Resized to: PhysicalSize { width: 726, height: 1061 }
Resized to: PhysicalSize { width: 484, height: 707 }

The window that appears is too small and does not ever get resized.


So, possibly the X11 implementation should be doing the scaling, or at least sending ScaleFactorChanged immediately to fix this?

But from my perspective it's wrong, as I've been been arguing here: neither winit nor a compositor can accurately calculate the scaled size since e.g. if a line of text would be 20.6 pixels tall, that needs to be rounded to an integer, then that integer size used to calculate the y-position of the next line, etc., thus when a window is sized to its contents, only the toolkit can calculate the correct window size.

dhardy commented 2 years ago

Apparently I made a mistake above: after the window is constructed, the render scale factor is known, and the toolkit can re-calculate layout using that immediately:

Initial scale factor: 1
Min inner window size: Size(135, 97)
Max inner window size: Size(484, 707)
New scale factor: 1.5
Resized to: PhysicalSize { width: 726, height: 1061 }
Resized to: PhysicalSize { width: 559, height: 994 }

The result looks acceptable, though the size is not exactly what was requested... edit: the resize happens because once the layout is recalculated, the toolkit calculates a different (smaller) ideal size for some reason, and calls self.window.set_max_inner_size(..).

kchibisov commented 2 years ago

@dhardy if you follow the logic wrt startup size and initial DPI are we doing in alacritty will it work for you (https://github.com/alacritty/alacritty/blob/589c1e9c6b8830625162af14a9a7aee32c7aade0/alacritty/src/display/mod.rs#L227) ?

dhardy commented 2 years ago

Yes, that's about what I just came up with.

LPGhatguy commented 2 years ago

Hit this issue in our project, ended up working around it with roughly this code:

let mut is_init = false;

event_loop.run(move |event, _, control_flow| {
    match event {
        Event::WindowEvent {
            event: WindowEvent::Resized(size),
            ..
        } => {
            if is_init {
                return;
            }

            if size.width != 0 || size.height != 0 {
                actually_do_resize(size.width, size.height);
            }
        }

        Event::NewEvents(cause) => {
            if cause == StartCause::Init {
                is_init = true;
            } else {
                is_init = false;
            }
        }

        // ...
    }
});

This fixes the issue on Windows 10 at least.

hannobraun commented 1 year ago

I'm seeing this issue on Gnome/X11. In my case, I'm getting another resize event with the correct size right after, so the following fix works:

let mut new_size = None;

event_loop.run(move |event, _, control_flow| {
    match event {
        Event::WindowEvent {
            event: WindowEvent::Resized(size),
            ..
        } => {
            new_size = Some(size);
        }
        Event::RedrawRequested(_) => {
            if let Some(size) = new_size.take() {
                renderer.handle_resize(size);
            }

            // ...
        }
        _ => {}
    }
});
schell commented 1 year ago

I am also seeing this on Windows 10. I'm building my window:

 let window_builder = WindowBuilder::new()
        .with_inner_size(PhysicalSize {
            width: 800,
            height: 600,
        })
        //...

and then I see three resize events after running the event loop:

2022-11-03 10:07:59.7764337 INFO example - window resized to: PhysicalSize { width: 2858, height: 1490 }
2022-11-03 10:07:59.7776071 INFO example - window resized to: PhysicalSize { width: 800, height: 600 }
2022-11-03 10:07:59.77843 INFO example - window resized to: PhysicalSize { width: 800, height: 600 }
rocket-matt commented 1 year ago

Is there any movement on this bug on Windows? The workaround above doesn't seem to work for me on Windows 11. I am getting the NewEvents(StartCause::init) event before the erroneous resize event.

I suspect the original size is coming from using CW_USEDEFAULT before setting to the requested size. Why can't the requested size be given on the CreateWindow call?

rocket-matt commented 1 year ago

The current work around I have is something like this:

#[cfg(windows)]
let mut is_init = false;
#[cfg(not windows)]
let mut is_init = true;

event_loop.run(...

    // Within WindowEvent::Resized:
    if is_init {
        // Do your resize
    } else {
        is_init = true;
    }

);