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

Fix resizing behaviour of hello-triangle example #2543

Merged
merged 3 commits into from
Mar 21, 2022

Conversation

FrankenApps
Copy link
Contributor

On macos the window is not updated until requested explicitly.

Connections
Issue was first raised in #2144.

Description
The window extended and the surface was resized accordingly, but the window was never updated with the new render.

Testing
Tested on Intel macbook pro.

On macos the window is not updated until requested explicitly.
@FrankenApps FrankenApps changed the title Fix resizing behaviour of hello-triangle example. Fix resizing behaviour of hello-triangle example Mar 20, 2022
@cwfitzgerald
Copy link
Member

Do the other examples need to be fixed too? In a way this feels like a winit bug, but I'm happy to accept this workaround.

@FrankenApps
Copy link
Contributor Author

@cwfitzgerald I am not sure if it is a winit Issue. They do recommend calling window.request_redraw() and also claim that it integrates with OS issued redraw requests, so I do not think that this should lead to additional redraws on other platforms.

There was one more example hello-windows that needed to be fixed, so I did this too.

The other examples with a winit window use the framework which renders at a given target framerate nevertheless (see here) so it does not really matter.

@cwfitzgerald
Copy link
Member

My thinking this is a winit issue is because a resize should always retrigger a redraw without the need to manually do it.

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) March 21, 2022 01:12
@cwfitzgerald cwfitzgerald merged commit 36877c3 into gfx-rs:master Mar 21, 2022
@FrankenApps FrankenApps deleted the patch-1 branch March 21, 2022 15:34
@FrankenApps
Copy link
Contributor Author

FrankenApps commented Mar 21, 2022

@cwfitzgerald I think it depends. If winits' goal is to abstract away all platform / os behaviour, then its probably an issue. However if thats not the goal and somehow apple decided to not fire a redraw request automatically when a window is resized on macos (I tried to look up the actual behaviour, but was not successful), then not.

Btw. I think the linked Issue should be closed. Somehow I did not reference it correctly.

@cwfitzgerald
Copy link
Member

Alright, I poked them on their matrix to see what they say.

@cwfitzgerald
Copy link
Member

@FrankenApps alright, just had a chat with them, they do expect that resizing should automatically trigger a redraw, so you should file a bug against winit if you have the time.

@FrankenApps
Copy link
Contributor Author

@cwfitzgerald I was in the process of opening an Issue on their repo, but now I think we might be on to something weird here and I am not sure if the bug is on their side (I would say most like yes, but I am not sure).

So I created a minimal example for filing a bug report for winit, but I discovered, that everything behaved as expected on my macos machine:

use winit::{
    event::{Event, WindowEvent},
    event_loop::{ControlFlow, EventLoop},
    window::Window,
};

fn main() {
    let event_loop = EventLoop::new();
    let _window = Window::new(&event_loop).unwrap();

    event_loop.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Wait;
        match event {
            Event::WindowEvent {
                event: WindowEvent::Resized(_),
                ..
            } => {
                println!("Resized event fired.")
            }
            Event::RedrawRequested(_) => {
                println!("Redraw requested.")
            }
            Event::WindowEvent {
                event: WindowEvent::CloseRequested,
                ..
            } => *control_flow = ControlFlow::Exit,
            _ => {}
        }
    });
}

results in an output like this:

Redraw requested.
Redraw requested.
Resized event fired.
Resized event fired.
Redraw requested.
Resized event fired.
Redraw requested.
Resized event fired.
Redraw requested.
...

So I compared this minimal version to the hello-triangle example and noted some small differences. At first I thought the problem might be related to pollster or that we are running inside an async function and pass the window around. But it turns out that is not the problem. Instead the window stops firing redraw requests on resize automatically after let _surface = unsafe { instance.create_surface(&window) }; is executed.

Here is a minimal version, with which I can reproduce the Issue on macos:

use winit::{
    event::{Event, WindowEvent},
    event_loop::{ControlFlow, EventLoop},
    window::Window,
};

fn main() {
    let event_loop = EventLoop::new();
    let window = Window::new(&event_loop).unwrap();

    let instance = wgpu::Instance::new(wgpu::Backends::all());

    // Comment out the next line to see, that it works without creating
    // a surface first.
    let _surface = unsafe { instance.create_surface(&window) };

    event_loop.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Wait;
        match event {
            Event::WindowEvent {
                event: WindowEvent::Resized(_),
                ..
            } => {
                println!("Resized event fired.")
            }
            Event::RedrawRequested(_) => {
                println!("Redraw requested.")
            }
            Event::WindowEvent {
                event: WindowEvent::CloseRequested,
                ..
            } => *control_flow = ControlFlow::Exit,
            _ => {}
        }
    });
}

it produces the following output when run:

Redraw requested.
Resized event fired.
Resized event fired.
Resized event fired.
Resized event fired.
...

I am unfamiliar with what you are doing with the window in create_surface() and if there might be any Issue there, so I decided to report back here first.

@Liamolucko
Copy link
Contributor

I wonder if this is related to rust-windowing/winit#2189? Apparently using Gl/Metal in a view causes the AppKit equivalent of request_redraw not to work, so that might be preventing resizes from triggering redraws too.

@cwfitzgerald
Copy link
Member

Yeah I definitely would consider this a bug because of what @Liamolucko said, they likely need to automatically add a request redraw on their own for the resize events. The impression I got from chatting with them is they definitely want it to fire every time no matter what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants