Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Erroneous WindowEvent::Resized sent on app startup #2094

Open
aloucks opened this issue Dec 13, 2021 · 35 comments
Open

Erroneous WindowEvent::Resized sent on app startup #2094

aloucks opened this issue Dec 13, 2021 · 35 comments
Labels
B - bug Dang, that shouldn't have happened DS - windows DS - x11

Comments

@aloucks
Copy link
Contributor

aloucks commented Dec 13, 2021

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
Copy link
Contributor

filnet commented Dec 13, 2021

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
Copy link
Contributor

filnet commented Dec 13, 2021

Seems related to 3ecbea3. I did not confirm that though...

@filnet
Copy link
Contributor

filnet commented Dec 13, 2021

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
Copy link
Contributor

filnet commented Dec 13, 2021

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

@msiglreith msiglreith added DS - windows B - bug Dang, that shouldn't have happened labels Dec 13, 2021
@msiglreith
Copy link
Member

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
Copy link
Contributor Author

aloucks commented Dec 14, 2021

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

pub unsafe fn on_create(&mut self) {
let win = self.window.as_mut().expect("failed window creation");
let attributes = self.attributes.clone();
// Set visible before setting the size to ensure the
// attribute is correctly applied.
win.set_visible(attributes.visible);
let dimensions = attributes
.inner_size
.unwrap_or_else(|| PhysicalSize::new(800, 600).into());
win.set_inner_size(dimensions);

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
Copy link
Contributor Author

aloucks commented Dec 14, 2021

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
Copy link
Member

maroider commented Dec 14, 2021

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

@msiglreith
Copy link
Member

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
Copy link

zxz149res commented Jan 12, 2022

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
Copy link
Contributor

dhardy commented Mar 14, 2022

I see the same behaviour (basically) on Wayland:

  • screen is 3840 × 2160 at 150% (but because of stupid reasons, Wayland clients see scale factor 200%)
  • max window size is set to 968, 1423 (physical px) using WindowBuilder
  • immediately on construction, size reported by window.inner_size() is this maximum
  • immediately after, window receives WindowEvent::Resized(PhysicalSize { width: 1936, height: 2846 })
  • on first Alt+Tab, window receives WindowEvent::Resized(PhysicalSize { width: 968, height: 1424 })

@kchibisov
Copy link
Member

Could you provide WAYLAND_DEBUG logs?

@dhardy
Copy link
Contributor

dhardy commented Mar 14, 2022

Scale factors 100% and 150%:
logs.zip

@kchibisov
Copy link
Member

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
Copy link
Contributor

dhardy commented Mar 15, 2022

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
Copy link
Member

@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
Copy link
Contributor

dhardy commented Mar 15, 2022

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
Copy link
Member

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
Copy link
Member

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

@dhardy
Copy link
Contributor

dhardy commented Mar 15, 2022

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
Copy link
Contributor

dhardy commented Mar 15, 2022

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
Copy link
Member

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
Copy link
Member

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

@dhardy
Copy link
Contributor

dhardy commented Mar 15, 2022

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
Copy link
Member

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
Copy link
Member

kchibisov commented Mar 16, 2022

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
Copy link
Contributor

dhardy commented Mar 16, 2022

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
Copy link
Contributor

dhardy commented Mar 16, 2022

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
Copy link
Member

@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
Copy link
Contributor

dhardy commented Mar 16, 2022

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

@LPGhatguy
Copy link
Contributor

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
Copy link

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
Copy link

schell commented Nov 2, 2022

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
Copy link

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
Copy link

rocket-matt commented Jan 18, 2023

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;
    }

);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B - bug Dang, that shouldn't have happened DS - windows DS - x11
Development

No branches or pull requests