macOS: winit emits a MouseInput event after resizing

[sodiumjoe]

repro example:

extern crate winit;

fn main() {
    let mut events_loop = winit::EventsLoop::new();

    let _window = winit::WindowBuilder::new()
        .with_title("A fantastic window!")
        .build(&events_loop)
        .unwrap();

    events_loop.run_forever(|event| {
        match event {
            winit::Event::WindowEvent { event: winit::WindowEvent::Resized(x, y), .. } => {
                println!("resized: {},{}", x, y);
                winit::ControlFlow::Continue
            }
            winit::Event::WindowEvent { event: winit::WindowEvent::MouseInput { state, .. }, .. } => {
                println!("mouse input: {:?}", state);
                winit::ControlFlow::Continue
            }
            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => {
                winit::ControlFlow::Break
            },
            _ => winit::ControlFlow::Continue,
        }
    });
}

[sodiumjoe]

For some reason, after resizing, on mouse up, appkit::NSLeftMouseUp fires, and appkit::NSLeftMouseDown never fires. Seems like it might have something to do with resize blocking the event loop.

[sodiumjoe]

So, with the printlns added here: https://github.com/tomaka/winit/compare/master...sodiumjoe:463-resize?expand=1

the output looks like this on resize:

WindowEvent { window_id: WindowId(Id(140514968137152)), event: CursorEntered { device_id: DeviceId(DeviceId) } }
resizing
WindowEvent { window_id: WindowId(Id(140514968137152)), event: Resized(2040, 1526) }
resizing
WindowEvent { window_id: WindowId(Id(140514968137152)), event: Resized(2040, 1524) }
resizing
WindowEvent { window_id: WindowId(Id(140514968137152)), event: Resized(2040, 1522) }
WindowEvent { window_id: WindowId(Id(140514968137152)), event: MouseInput { device_id: DeviceId(DeviceId), state: Pressed, button: Left, modifiers: ModifiersState { shift: false, ctrl: false, alt: false, logo: false } } }
WindowEvent { window_id: WindowId(Id(140514968137152)), event: TouchpadPressure { device_id: DeviceId(DeviceId), pressure: 0.0, stage: 1 } }
WindowEvent { window_id: WindowId(Id(140514968137152)), event: TouchpadPressure { device_id: DeviceId(DeviceId), pressure: 0.000030517578, stage: 1 } }
WindowEvent { window_id: WindowId(Id(140514968137152)), event: TouchpadPressure { device_id: DeviceId(DeviceId), pressure: 0.002029419, stage: 1 } }
WindowEvent { window_id: WindowId(Id(140514968137152)), event: TouchpadPressure { device_id: DeviceId(DeviceId), pressure: 0.0050354004, stage: 1 } }

It looks like the event loop blocks when the resize starts. So while resizing all the event_loop events queue up inside the nextEventMatchingMask_untilDate_inMode_dequeue_. Then once the resizing finishes, the event loop runs again and emits all the events that were queued.

So it looks like the initial MouseInput { state: Pressed } is there, but for some reason the MouseInput { state: Released } isn't in the queue. I have no idea why.

One way to address this might be to discard all the queued events during resize with this. I guess the window would have to notify the event loop when a resize happens.

[sodiumjoe]

@Osspial @paulrouget @francesca64 @mitchmindtree @swiftcoder @RobSaunders (pinging y'all because you're either well represented in git blame on src/platform/macos/{events_loop,window}.rs or I've interacted with you in the past on macos issues, apologies if this is inappropriate)

I'd be willing to take a stab at this, but would appreciate any guidance. Discarding all events queued during resize seems reasonable to me, since presumably those events would only originate from the window as opposed to other application events, but I'm new to objective-c and rust. I also don't want to sink too much time into this if the approach is just wrong.

Alternatively, if anyone has ideas about a better approach, I'm all ears. Thanks!

[swiftcoder]

I'm actually a little surprised that this is happening, because winit is looking for events in NSDefaultRunLoopMode and window resize should be happening running in NSEventTrackingRunLoopMode.

That said, a simple hack would be to eat a single MouseUp event in WindowDelegate.window_did_resize(...), which might do the trick. Or we could potentially rework the whole event loop to work with live window resize, and get smooth rendering during resize working as well.

[sodiumjoe]

@swiftcoder

I'm actually a little surprised that this is happening, because winit is looking for events in NSDefaultRunLoopMode and window resize should be happening running in NSEventTrackingRunLoopMode.

I think the issue is that the resize event doesn't come from that event loop, it comes from the WindowDelegate.window_did_resize, which is out of band of the event loop (they eventually both called user_callback.call_with_event directly).

I think this is why the hack wouldn't work either, because the events are sourced from different places. I can try keeping a has_unhandled_resize_event boolean in UserCallback in order to eat the next MouseUp event in call_with_event, but that seems like a significantly nastier hack than what you suggested. I'll try it out and report back.

I also have tried googling around for why the event is emitted, and why the MouseDown isn't, but my google-fu is failing me there (and my unfamiliarity with cocoa is not helping).

Live window resize looks like it would involve a pretty significant refactor, and possibly an api change?

[swiftcoder]

Ok, there's something a little more subtle going on here. If I take your example, and press and hold on the resize affordance (without moving the mouse), I see both the press and the release event one after another, but neither appears until I let go of the button.

If I drag, I get the resize events, then the Press, but the Release never shows up.

[swiftcoder]

I see what's happening. We do actually receive the mouse down event, but then we call NSApp.sendEvent(...) with it, which dispatches to NSWindow.sendEvent(...), which if you are over a resize handle, enters a modal tracking loop... the tracking loop blocks until the user releases the mouse.

If the mouse wasn't moved, we'll deliver the mouse down when we are unblocked, and immediately receive the mouse up, and all is well.

However, if the mouse was dragged, NSWindow's tracking loop discards to mouse up event (along with all the mouse moved events). We are unblocked, we deliver the original mouse up event, and there's no mouse down in the queue any more, so things are not good.

We can solve this by marking the event for discarding in windowDidResize. See #466 for a working fix.

[sodiumjoe]

@swiftcoder awesome, thanks! I wouldn't have been able to figure this out for a while, if at all.

[francesca64]

Oh wow, I wasn't expecting this to resolve so painlessly. Great work, @swiftcoder!

In regard to changing the API to make live window resize possible, there's been more discussion lately about changing the way the event loop works (#459) which is a fairly recurring area of concern (#231). Therefore, if you have any case to make for why the current API should change, then I encourage being vocal, since that will help lead us to a decisive stance on what's needed.

@sodiumjoe I certainly don't mind being pinged (though that's not saying much, since I get notified either way), but I'd like to take this opportunity to ask if you mind being pinged. I can see that you've been putting care into winit and the macOS backend, and it would be a big help if you could review PRs and generally represent the interests of Mac users.

[sodiumjoe]

@francesca64 I've found my way into contributing to winit by way of alacritty, mostly trying to scratch my own itches and also trying to learn rust. Out of necessity, I've had to learn a little bit of obj-c and c along the way 😅 . All that to say, I'm very inexperienced at basically everything in winit.

That said, I'm happy to contribute what time I can find (I have a full time job and young children), and I don't mind being pinged at all. If nothing else, I can help test/verify issues and fixes.

[swiftcoder]

A prettier fix somewhere down the road might be to pull mouse down events off the Window, instead of the global event queue. If we overrode mouseDown: and mouseUp: on the window's container view, that would give us mouse events after the window has handled resizing, instead of before.

It would limit us to only handling those events within the window bounds, so we'd need to return to the existing method when capturing the mouse... maybe not prettier after all. Hmm.

[sodiumjoe]

what is the release process for winit/glutin?

[tomaka]

@sodiumjoe You can ask to publish a new versi9j by submitting a PR that bumps the changelog and the version in Cargo.toml.

[francesca64]

Re-opening as per #470

[sodiumjoe]

seems like at this point we should consider this blocked on #459 and #231

[francesca64]

#561 appears to fix this.