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

MacOS can't wake up during resize, breaks timeout schemes #219

Closed
willglynn opened this issue Jul 11, 2017 · 24 comments · Fixed by #913
Closed

MacOS can't wake up during resize, breaks timeout schemes #219

willglynn opened this issue Jul 11, 2017 · 24 comments · Fixed by #913
Labels
C - waiting on author Waiting for a response or another PR D - hard Likely harder than most tasks here DS - macos P - high Vital to have S - platform parity Unintended platform differences
Milestone

Comments

@willglynn
Copy link

As the comment describes, nextEventMatchingMask:untilDate:inMode:dequeue: blocks during a resize and does not return events until the resize is complete. That includes not returning events sent via postEvent:atStart:, which means Proxy::wakeup() doesn't actually cause run_forever() to wake up.

This property is especially unfortunate with glutin_window, which implements its own fn wait_event_timeout(&mut self, timeout: Duration) by calling run_forever() and interrupting when the timeout expires. Since it can't interrupt, a wait_event_timeout() that wants to yield 60 fps to e.g. drive a render loop will simply hang entirely until the user is done resizing.

I've been experimenting with ways to get nextEventMatchingMask: to be less thorny, but another complimentary approach would be to expose a new run_until_timeout() call in addition to poll_events() and run_forever() -- or perhaps to deprecate poll_events() and run_forever() in favor of run_until_timeout(Some(0)) and run_until_timeout(None). A timeout-aware API would let glutin_window pass its deadline all the way down to nextEventMatchingMask:untilDate:, avoid spawning needless timeout threads, and sidestep some of the resize-related blocking issues all at once.

@mitchmindtree
Copy link
Contributor

mitchmindtree commented Jul 11, 2017

As far as I understand, this modal resize event loop is an unfortunate wart stemming from macos history 😿 I've spent quite a bit of time looking into solutions but haven't personally had much luck - hopefully your endeavour turns out better than mine!

One of the particularly frustrating things about this is that, to properly handle live-resize a winit application's drawing must be abstracted in a way that allows it to be called when Resized events are received on top of the regular place at which drawing is done in the application. This becomes particularly tedious when the drawing needs to be timed (e.g. don't draw more than 60fps) which is in most cases as it is common for large numbers of Resized events to be emitted at once.

All that said, the current situation is much better than pre #126 days, where it was impossible to handle live resize without registering a totally separate callback function that could only share data with the main application via global state 😱 Hopefully we can work out some way to improve things even further though.

I'm not sure about adding a timeout method, or whether it will even help much with our macos problem. In theory, this should already be easy for users to do by using a timer to call EventsLoopProxy::wakeup, interrupting the run_forever method - unfortunately it's only macos that has problems with this due to this problem of blocking on resize that you've already outlined. I'm skeptical that specifying a timeout for nextEventMatchingMask will actually help in this case, as from what I remember the current implementation of poll_events is basically this with a timeout of 0 and it still blocks during resize.

@willglynn
Copy link
Author

willglynn commented Jul 11, 2017

All that said, the current situation is much better than pre #126 days

That's why I'm looking at this now and not earlier :-)

I'm not sure about adding a timeout method, or whether it will even help much with our macos problem.

A timeout-capable API would remove this pattern:

let events_loop_proxy = self.events_loop.create_proxy();
thread::spawn(move || {
    thread::sleep(timeout);
    events_loop_proxy.wakeup().ok();
});
{
    let ref mut events = self.events;
    self.events_loop.run_forever(|ev| {
        events.push_back(ev);
        glutin::ControlFlow::Break
    });
}

That's pistoncore-glutin_window implementing a timeout-oriented API on top of winit's timeout-less API which is implemented on top of e.g. MacOS's timeout-oriented API.

Letting the caller express timeouts seems intrinsically useful: if it's a timed event loop, the caller wants to either a) receive a message from the windowing system or b) get control again without a message so it can start another draw cycle. Every platform should have a way of accommodating this, and it's likely better than unconditionally spawning a thread for every message received. If not, maybe the platform should be responsible for implementing timeouts by spawning a thread and waking itself.

"Receive with timeout" is also a more fundamental approach than the current APIs: poll_events() is a special case where you want to receive any messages that are available right now without blocking (zero timeout), and run_forever() is a special case where you want to keep running until you want to receive a message (infinite timeout).

I'm skeptical that specifying a timeout for nextEventMatchingMask will actually help in this case, as from what I remember the current implementation of poll_events is basically this with a timeout of 0 and it still blocks during resize.

Yeah… the situation is worse than the comment in the code, and a timeout here doesn't help as much as I initially thought.

Resizing does block on _nextEventMatchingEventMask:untilDate:inMode:dequeue:, but not where poll_events()/run_forever() tries to receive a message. After receiving the message that starts a resize (I think it's a general mouse down event?), both poll_event() and run_forever():

  • call ns_event_to_event(),
  • which calls -[NSApplication(NSEvent) sendEvent:],
  • which calls -[NSWindow(NSEventRouting) sendEvent:],
  • which calls -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:],
  • which calls -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:],
  • which calls -[NSThemeFrame mouseDown:],
  • which calls -[NSThemeFrame handleMouseDown:],
  • which calls -[NSTitledFrame attemptResizeWithEvent:],
  • which calls -[NSWindow(NSWindowResizing) _resizeWithEvent:],
  • which calls -[NSView _getNextResizeEventFromMask:invalidatingLiveResizeCacheIfNecessary:],
  • which calls -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]

That's bad. For resize events, sendEvent: wanders into an internal runloop that doesn't return until the resize is complete. This is where we get stuck.

A timeout on the outer loop won't fix that, but let's say the caller told us when they needed control again.

How would that help? Well, I don't think we can avoid this inner runloop, but a runloop observer could punt control back to user code before that inner runloop goes to sleep or after it wakes. We should be able to wake up the event loop with a timer, as @mitchmindtree point outs, even if the timer message gets ignored by the mask and therefore not delivered in a timely fashion.

How would that help? Well… let's say we somehow manage to get back to winit from deep inside this context, and we see that the user's timeout has expired. That tells us that we need to return from the poll_events() or run_until_timeout() call immediately. We can't unwind the stack back to winit's caller, so we need to avoid piling on top of it in the first place. Spawn a thread, right? Well, we also need to be on the caller's thread. That leaves stack switching: call sendEvent: in a coroutine which either a) runs to completion or b) yields when a timeout expires.

This would let winit's event loop return to the user on time even though sendEvent: hasn't yet returned. The user gets control, draws something, calls the event loop again. Event loop sees there's still a sendEvent: call pending, and instead of working the normal runloop again, it resumes the coroutine which stack switches back to the runloop observer callback in the middle of -[NSTitledFrame attemptResizeWithEvent:]'s filtered runloop. Maybe sendEvent: returns and life goes back to normal, or maybe it doesn't and we suspend the coroutine again and repeat.

@willglynn
Copy link
Author

I have a coroutines branch that accomplishes most of what I described.

First, I started with refactoring: the MacOS event loop now has a CocoaEvent enum that tracks Received from Cocoa -> Sending back to Cocoa -> Complete (and possibly translated into an events::Event) lifecycle. I also refactored the "send event" activity into a struct that possibly requires work() to be called several times, and made the general event loop aware of this. The basic implementation calls sendEvent: as usual.

Second, I added an optional Mac dependency on context which can provide suitable coroutine glue. As of this point, winit uses the basic mod send_event implementation by default, but --feature context will pull in context and use mod send_event_context instead. I then got it calling sendEvent: from inside the coroutine, but still blocking as usual.

Third, I added an observer to the default runloop triggering on kCFRunLoopBeforeWaiting | kCFRunLoopAfterWaiting. As of now, this observer unconditionally context switches back out of the coroutine, punting all the way back to the usual MacOS event loop code -- even during a resize. Concept: proved.

Next steps:

  • As of right now wakeup() can't actually wake up the run loop, which I think will mean adding a timer… but again, this is use case. If the caller needs to return by a specific time, they should be able to indicate that when they enter the event loop, instead of waiting for that time to elapse and then trying to rouse the event loop from slumber. Should I open a new issue for that?
  • Tear apart the MacOS event loop and reassemble it out of a chain of generators. There's too many ways events can get queued and buffered and such, and still too much logic in poll_events() and run_forever(), even after this refactoring.
  • Optimize the coroutine implementation. Right now, each sendEvent: operation gets its own stack and its own runloop observer, both of which are torn down when it returns. This is wasteful.

@willglynn
Copy link
Author

Tear apart the MacOS event loop and reassemble it out of a chain of generators.

That's not exactly what I ended up doing, but oh man does this approach feel better. willglynn@12cd9f3:

poll_events() and run_forever() now delegate event production to get_event(), a new function whose job it is to either a) produce Some(Event) by a Timeout or b) return None. This dramatically simplifies the entire event loop.

MacOS in particular has multiple ways that Events can be produced: wakeup(), AppDelegate callbacks, and a Cocoa event receive -> send -> translate cycle. The first two are handled by an event queue; wakeup() now posts an Event::Awakened to the queue, just like the callbacks.

The CocoaEvent cycle is handled either via the usual blocking [NSApp sendEvent:] call or via a coroutine-based event dispatcher. The coroutine yields back to get_event() on a regular basis, even while it's in the middle of sending events. This allows get_event() to return an event to the calling application even if it is still in the middle of a sendEvent: call.

This is what I mean by a timeout-centric API being fundamental. poll_events() and run_forever() are now:

pub fn poll_events<F>(&mut self, mut callback: F)
    where F: FnMut(Event),
{
    // Return as many events as we can without blocking
    while let Some(event) = self.get_event(Timeout::Now) {
        callback(event);
    }
}

pub fn run_forever<F>(&mut self, mut callback: F)
    where F: FnMut(Event) -> ControlFlow
{
    // Get events until we're told to stop
    while let Some(event) = self.get_event(Timeout::Forever) {
        // Send to the app
        let control_flow = callback(event);

        // Do what it says
        match control_flow {
            ControlFlow::Break => break,
            ControlFlow::Continue => (),
        }
    }
}

So far I've implemented only Timeout::Now and Timeout::Forever, but Timeout::Deadline would be a natural and useful extension.

@willglynn
Copy link
Author

As of right now wakeup() can't actually wake up the run loop, which I think will mean adding a timer…

Per the earlier changeset, wakeup() actually enqueues an event, so now it's enqueue_event() that's responsible for attempting to wake the runloop.

willglynn@1ffc1bf adds a shared CFRunLoopTimer which can be trigger()ed from any thread. The timer callback does nothing by default, but when paired with --feature context, it yields from event sending coroutine back to get_event().

Experimentation suggested that the timer callback alone was insufficient, as was a timer + runloop observer, but the combination of timer, runloop observer, and explicitly waking the event loop seems at least minimally passable. My crude instrumentation shows enqueue_event() -> get_event() latencies reliably in the 4-6 ms range during a resize. That still seems high, but now AppDelegate callbacks and wakeup() calls are actually getting delivered during a resize, instead of blocking forever until the user stops dragging.

@willglynn
Copy link
Author

From #231, @mitchmindtree said:

@willglynn I just tested the window.rs example with your coroutines branch on my macOS machine and it has re-introduced this problem now that you have re-implemented run_forever in terms of get_event.

Are you running with --features context? I'm able to receive resize events during a resize.

@mitchmindtree
Copy link
Contributor

mitchmindtree commented Jul 17, 2017

Ah my mistake, I did not! I just ran the window.rs example briefly again, this time with --features "context" and it does seem to emit the events correctly - nice work!

I just had a brief read through the diff for your coroutines branch. I've never done any stack switching before and so I'm yet to wrap my head around it fully (I think reading through this context crate might help), but this solution seems promising.

My crude instrumentation shows enqueue_event() -> get_event() latencies reliably in the 4-6 ms range during a resize

Would you mind elaborating on this a little more? Where is this delay coming from?

@willglynn
Copy link
Author

Would you mind elaborating on this a little more? Where is this delay coming from?

Short answer: I don't know.

My coroutines branch adds callbacks on:

  • Runloop before sleep
  • Runloop after wake
  • Timer fired

When triggered from the coroutine, these callbacks yield back to the "normal" stack frame, which jumps back to get_event(). Once there, we check the event queue for pending events, check the timeout to see if we need to return, and eventually yield back to the coroutine -- which from the coroutine's perspective means we finish executing the callback.

The runloop observer keeps us from blocking indefinitely, and the timer is something we can trigger across threads, so we have all the pieces.

Shared::enqueue_event() is thread safe and intended to be invoked from anywhere. It adds the event to the queue (which get_event() will check), and then it needs to get back to get_event(), so it tells the timer to fire right now and calls CFRunLoopWakeUp(CFRunLoopGetMain()). We're then at the mercy of the usual event loop, waiting for it to reach one of our callbacks and yield.

That said, I realized something else: in the case where we're trying to post an event from inside the coroutine -- say because the resize event loop triggered our NSWindowDelegate callback -- we can yield back to get_event() directly. There's no reason to wait for anything at all.

This totally works, and it drops the event delivery time to 11-14 µs using the same unscientific measuring protocol.

@willglynn
Copy link
Author

I just had a brief read through the diff for your coroutines branch. I've never done any stack switching before and so I'm yet to warp my head around it fully (I think reading through this context crate might help), but this solution seems promising.

The code in my branch isn't at all a clear introduction to the concept, but it's what I produced after a series of incremental refactorings. My plan for the last action item ("optimization") involves moving the EventLoop boundary even further, and it's easier to explain, so I'll do that now.

The problem

A basic Cocoa runloop looks like:

loop {
    event = receive_event_from_cocoa(distant_future);
    forward_event_to_cocoa(event);
    match event {
        appkit::NSLeftMouseDown => {
            emit(WindowEvent::MouseInput{});
        },
        appkit::NSLeftMouseUp => {
            emit(WindowEvent::MouseInput{});
        },}
}

extern fn resize_callback() {
    emit(WindowEvent::Resized{});
}

We need to map this to winit's poll_events() API, in which the user owns the loop {} and in which we need to return promptly. We can call receive_event_from_cocoa() with a deadline that's already in the past, and if we don't get any events, we're done. Sketch:

fn poll_events(callback) {
    while let Some(event) = dequeue_event_nonblocking() {
        callback(event);
    }
    
    global_callback = Some(callback);
    
    while let Some(event) = receive_event_from_cocoa(distant_past) {
        forward_event_to_cocoa(event);
        match event {
            appkit::NSLeftMouseDown => {
                callback(WindowEvent::MouseInput{});
            },
            appkit::NSLeftMouseUp => {
                callback(WindowEvent::MouseInput{});
            },}
    }
    
    global_callback = None;
}

extern fn resize_callback() {
    if let Some(callback) = global_callback {
        callback(WindowEvent::Resized{});
    } else {
        enqueue_event(WindowEvent::Resized{});
    }
}

This is roughly what master does now. The problem: forward_event_to_cocoa(event) delegates to other components, at least one of which can enter its own internal runloop. This is fine for normal Cocoa apps, since the run inside the loop {} -- but winit runs inside its caller's loop, so entering an internal loop is bad.

The call stack at this point looks like:

poll_events()
  forward_event_to_cocoa()
    -[NSApplication(NSEvent) sendEvent:]
      -[NSWindow(NSEventRouting) sendEvent:]
        -[NSWindow(NSEventRouting) _reallySendEvent:isDelayedEvent:]
          -[NSWindow(NSEventRouting) _handleMouseDownEvent:isDelayedEvent:]
            -[NSThemeFrame mouseDown:]
              -[NSThemeFrame handleMouseDown:]
                -[NSTitledFrame attemptResizeWithEvent:]
                  -[NSWindow(NSWindowResizing) _resizeWithEvent:]
                    -[NSView _getNextResizeEventFromMask:invalidatingLiveResizeCacheIfNecessary:]
                      <internal event loop>
                        resize_callback()

We can get callbacks from within the internal event loop, but we can't return from forward_event_to_cocoa(), and therefore we can't return poll_events(), which means poll_events() will block until that inner loop returns.

The fix that isn't

Okay, so let's split this into two threads: one for the caller's code, one for Cocoa runloop.

fn poll_events(callback) {
    while let Some(event) = dequeue_event_nonblocking() {
        callback(event)
    }
}

fn runloop() {
    loop {
        event = receive_event_from_cocoa(distant_future);
        forward_event_to_cocoa(event);
        match event {
            appkit::NSLeftMouseDown => {
                enqueue_event(WindowEvent::MouseInput{});
            },
            appkit::NSLeftMouseUp => {
                enqueue_event(WindowEvent::MouseInput{});
            },}
    }
}

extern fn resize_callback() {
    enqueue_event(WindowEvent::Resized{});
}

thread::spawn(|| { runloop() });

Brilliant. Now it doesn't matter if we're in the outer loop or an inner loop, since the only way runloop() communicates with EventLoops::poll_events() is via a queue or channel or whatever. poll_events() will never block, and we're done.

Problem: receive_event_from_cocoa() and forward_event_to_cocoa() can't be called anywhere but on the main thread, which belongs to the winit's caller, which has its own loop which we don't control.

winit has no choice but to execute a runloop because forward_event_to_cocoa() can enter an internal runloop… but also winit's API puts the user in charge of the runloop and assumes that's the only one. That's a big conflict.

The fix that is

The realization is that while we do need two separate threads of execution -- one of them needs to stay in a loop while the other one returns from a function -- we do not need two separate OS threads. We need only the ability to enter and leave runloop()'s call stack at will, which is something we can achieve.

fn poll_events() {
    while let Some(event) = yield_to_runloop(Timeout::Now) {
        emit(event)
    }
}

static timeout: Timeout;

fn runloop() {
    timeout = yield_to_caller(None);
    
    loop {
        event = receive_event(timeout);
        forward_event(event);
        match event {
            appkit::NSLeftMouseDown => {
                timeout = yield_to_caller(WindowEvent::MouseInput{});
            },
            appkit::NSLeftMouseUp => {
                timeout = yield_to_caller(WindowEvent::MouseInput{});
            },}
    }
}

fn resize_callback() {
    yield_to_caller(WindowEvent::Resized{})
}

When poll_events() wants to give control to the runloop, it calls yield_to_runloop(Timeout), which transfers execution into that context. Every time the runloop gets an event, it calls yield_to_caller(Some(Event)). If the runloop detects that the timeout is elapsed, it calls yield_to_caller(None).

At the machine level, this is a user-mode context switch -- context restores the processor state as of the moment the callee was last suspended, while also providing callee with the processor state of the caller as of the moment it transferred execution. This lets the callee resume() its caller, and lets the caller resume() the callee, ad infinitum, cooperatively sharing execution of a single OS thread as coroutines.

The magic is that the runloop() coroutine has a separate call stack, which means it can suspend itself and context switch back to e.g. poll_events() from anywhere. This is the key.

If resize_callback() gets invoked, it can yield back to poll_events() and let the user handle the resize event, even if it's deep inside Cocoa components. The next time the user calls poll_events(), execution resumes inside resize_callback() exactly where it left off. If the runloop learns that forward_event() entered an internal loop due to -[NSWindow(NSWindowResizing) _resizeWithEvent:] and gets a callback that the thread is about to block, it can yield back to poll_events() without returning from _resizeWithEvent:.

Right now in my coroutines branch, message sending lives in a coroutine, which we create and destroy for each message. I plan to push the entire Cocoa runloop into a single long-lived coroutine: stream of Timeouts come in, stream of Option<Event>s come out. This will be simpler, cleaner, and more efficient than the way this branch uses coroutines now.

@tomaka
Copy link
Contributor

tomaka commented Jul 17, 2017

So you're context switching by using inline assembly? That looks like dark magic.

@willglynn
Copy link
Author

willglynn commented Jul 17, 2017

Well… I'm context switching using context, which context switches using boost.context, which context switches using inline assembly. If you'd prefer, I could trade this out for an implementation that FFIs to swapcontext(), which was defined by POSIX and is provided by the MacOS X libc.

(Edit: LLVM is getting coroutine intrinsics, so it stands to reason we'll get them in Rust eventually.)

As I see it, the facts are that a) the MacOS platform has unavoidable loops and b) the winit API assumes it doesn't. The options are:

  1. Use coroutines to marry both APIs,
  2. Move winit to the bottom of the call stack, drop poll_events(), drop run_forever()'s ability to return, and make the user do everything inside a callback, or
  3. Accept that poll_events() can sometimes block forever even though it's not supposed to

@jwilm
Copy link
Contributor

jwilm commented Jul 17, 2017

I'm context switching using context, which context switches using boost.context

There's also libfringe which provides this without the boost dependency. libfringe is of course implemented using inline assembly as well.

In any case, the coroutine solution seems rather clever for the Cocoa run loop problem. It would be nice to not worry about macOS-specific details and have the control flow friendly event fetching API.

@willglynn
Copy link
Author

@jwilm: Yep, and coroutine, and others. I selected context and suggested FFI to the platform's swapcontext() as an alternative in order to avoid requiring nightly Rust.

@tomaka
Copy link
Contributor

tomaka commented Jul 17, 2017

I still have zero knowledge of how cocoa behaves in the first place, but I'm really dubious about this because you have to make sure that whatever the user does and whatever cocoa does this can't trigger an undefined behavior.

@tomaka
Copy link
Contributor

tomaka commented Jul 17, 2017

I'm not even sure that context switching are compatible with safe Rust in the first place. We would need an expert to analyse that.

@willglynn
Copy link
Author

willglynn commented Jul 17, 2017

I'll make the case that this is less crazy than it sounds.

There's no concurrent multithreading. It's not possible for the user's code on the main thread and the runloop coroutine to concurrently mutate anything because they're not running at the same. We'd context switch into the runloop coroutine from get_event() and context switch back to that same place with 100% determinism.

There's no shared state between the runloop and the rest of the main thread. The runloop can keep everything in its private stack, except for events which it can push onto a mutex-protected queue, which is what it does now. That mutex can't deadlock on the main thread because there's exactly one call site where the main thread can enter the runloop coroutine.

"How do we panic?" is a valid question, given that panicing unwinds the stack and we're on a different stack. Note however that I'm not suggesting we run the user's code in a coroutine, only winit's Cocoa event loop code, which shrinks the problem considerably -- all we have to do is deliver events to a queue and context switch back. That's safer than the current approach, where the runloop produces events and delivers them straight from Objective C into the user's callback, which lets the user panic! and unwind into FFI.

@willglynn
Copy link
Author

willglynn commented Jul 18, 2017

I refactored a bunch of event-handling concerns, ripped out my SendEvent/CocoaEvent state machine stuff, and pushed a basic Cocoa runloop into a Runloop which works the way I describe in "The problem". I then transformed that Runloop to run as a coroutine , selectable via --features context.

I haven't yet hooked up the timer or runloop observer again, but cargo run --example window --features context gets resize events as they happen (like master) and poll_events() can return during a resize (unlike master).

@willglynn
Copy link
Author

There's now a timer and a runloop observer which helps the resize situation considerably. This brings my observed worst-case poll_events() call duration down from 978 ms to 25 ms, and it keeps the typical mid-resize call duration in the single digit milliseconds or better.

(When not resizing, call durations vary from 14-50 µs with a median of 26 µs. Switching contexts each trip into and out of the runloop does not seem to be a performance problem.)

It's probably worth making the timer fire only during a resize event, since I think that's the only time we need it, and that would reduce the non-resize overhead. That would also let us make it fire ridiculously fast during a resize to further constrain the amount of time the inner event loop can spend on other things without yielding.

@francesca64 francesca64 added P - high Vital to have S - platform parity Unintended platform differences labels May 6, 2018
@francesca64 francesca64 added this to the EventsLoop 2.0 milestone May 6, 2018
@OneSadCookie
Copy link

I really don't think this should be required any more with the Event Loop 2.0 stuff? If you need control back during the live resize to allow the user to render, then an NSTimer in NSEventTrackingRunLoopMode should provide that. (And in general, running rendering off a high-frequency timer is probably sane in all run loop modes).

@iugo-eric
Copy link

Right now during a window resize, alacritty window contents are unresponsive until after the resize is complete. Is this why? (See alacritty/alacritty#2283)

parasyte added a commit to parasyte/pixels that referenced this issue May 29, 2021
- Fixes #163
- Reimplements window resize
  - Still has problems on macOS; Resizing with the drag handle blocks the main loop. See:
    - glfw/glfw#1251
    - rust-windowing/winit#219
parasyte added a commit to parasyte/pixels that referenced this issue May 29, 2021
- Fixes #163
- Reimplements window resize
  - Still has problems on macOS; Resizing with the drag handle blocks the main loop. See:
    - glfw/glfw#1251
    - rust-windowing/winit#219
tmfink pushed a commit to tmfink/winit that referenced this issue Jan 5, 2022
tmfink pushed a commit to tmfink/winit that referenced this issue Jan 5, 2022
madsmtm added a commit to madsmtm/winit that referenced this issue Jun 11, 2022
* refactor(windows): `begin_resize_drag` now similar to gtk's (rust-windowing#200)

* refactor(windows): `begin_resize_drag` now similart to gtk's

* fix

* feat(linux): skipping taskbar will now also skip pager (rust-windowing#198)

* refactor(linux): clean dummy device_id (rust-windowing#195)

* refactor(linux): clean dummy device_id

* fmt

* feat(linux): allow resizing undecorated window using touch (rust-windowing#199)

* refactor(windows): only skip taskbar if needed when `set_visible` is called (rust-windowing#196)

* fix: increase borderless resizing inset (rust-windowing#202)

* fix: increase borderless resizing inset

* update some comments

* Replace winapi with windows crate bindings shared with WRY (rust-windowing#206)

* fix(deps): update rust crate libayatana-appindicator to 0.1.6 (rust-windowing#190)

Co-authored-by: Renovate Bot <[email protected]>

* Add Windows crate and webview2-com-sys bindings

* Initial port to webview2-com-sys

* Finish conversion and remove winapi

* Fix renamed lint warning

* Fix all match arms referencing const variables

* Put back the assert instead of expect

* Point to the published version of webview2-com-sys

* Cleanup slightly weird BOOL handling

* Replace mem::zeroed with Default::default

* Add a summary in .changes

* Remove extra projects not in config.json

* Fix clippy warnings

* Update to 32-bit compatible webview2-com-sys

* Better fix for merge conflict

* Fix clippy errors on Windows

* Use path prefix to prevent variable shadowing

* Fix Windows clippy warnings with nightly toolchain

* Fix Linux nightly/stable clippy warnings

* Fix macOS nightly/stable clippy warnings

* Put back public *mut libc::c_void for consistency

* Re-run cargo fmt

* Move call_default_window_proc to util mod

* Remove unnecessary util::to_wstring calls

* Don't repeat LRESULT expression in match arms

* Replace bitwise operations with util functions

* Cleanup more bit mask & shift with util fns

* Prefer from conversions instead of as cast

* Implement get_xbutton_wparam

* Use *mut libc::c_void for return types

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>

* fix(keyboard): add mapping for space key on Windows (rust-windowing#209)

* fix(keyboard): add mapping for space key on Windows

* change file

* feat: impl Clone for EventLoopWindowTarget (rust-windowing#211)

* chore: add `on_issue_closed.yml` (rust-windowing#214)

* Update tray dependency version (rust-windowing#217)

* Delete on_issue_closed.yml (rust-windowing#221)

* refactor(linux): event loop (rust-windowing#233)

* Use crossbeam::channel

* Fix crossbeam channel import

* Add check on poll event

* Fix deadlock when unregistering shortcut on Linux (rust-windowing#230)

* Add fullscreen monitor selection support on Linux (rust-windowing#235)

* Add fullscreen monitor support on Linux

* Add change file

* Remove todo on videomode

* Fix clippy

* Update to 2021 edition (rust-windowing#236)

* Update to 2021 edition

* Fix clippy

* Add run_return on Linux (rust-windowing#237)

* Add run_return on Linux

* Add main context

* Add run_return trait on Linux (rust-windowing#238)

* Fix: rust-windowing#239 Update webview2-com and windows crates (rust-windowing#240)

* Replace webivew2-com-sys with prebuilt windows

* Use windows utility instead of direct GetLastError

* Bump windows version and add changelog

* Run cargo fmt

* Restore inverted matches macro

* Scope constants in match arms

* Fix inverted null check

* Update src/platform_impl/windows/util.rs

Co-authored-by: Amr Bashir <[email protected]>

* Use env_logger instead of simple_logger (rust-windowing#241)

* Use env_logger instead of simple_logger

* Make clippy happy

* Cherry pick commits to next (rust-windowing#244)

* feat(macos): Add `unhide_application` method, closes rust-windowing#182 (rust-windowing#231)

* feat(macos): Add `unhide_application` method

* Update src/platform/macos.rs

Co-authored-by: Amr Bashir <[email protected]>

* Reanme to `show_application()`

* Remove broken doc link

Co-authored-by: Amr Bashir <[email protected]>

* feat: Allow more strings to parse to keycode (rust-windowing#229)

* feat: support accelerator key strings `,` `-` `.` `Space` `Tab` and `F13`-`F24` (rust-windowing#228)

* feat(macOS): support more accelerator key strings

* Move function keys together

* Add `,` `-` `.` `Space` `F20-F24` for Windows

* Remove support for accelerators not found in `winapi`

* Add `,` `-` `.` `Space` `F13-F24` for Linux

* Update .changes

* Add the rest for Windows

* Add the rest for Linux

* Add the rest on macOS

* Update accelerator-strings.md

* Fix git comments

Co-authored-by: Kasper <[email protected]>
Co-authored-by: Amr Bashir <[email protected]>

* Add redraw events on Linux (rust-windowing#245)

* Add redraw events on Linux

* Update doc of RequestRedraw Event

* Add change file

* Fix missing menu bar on borderless window (rust-windowing#247)

Credit goes to irh's work on winit commit f2de847

* refactor: improve `set_skip_taskbar` impl on Windows (rust-windowing#250)

* fix: emit errors on parsing an invalid accelerator for string, closes rust-windowing#135 (rust-windowing#252)

* chore: update comment

* fix(linux): fix focus events not firing properly (rust-windowing#253)

* fix(linux): fix focus events not firing properly

* add changelog

* chore: update focus events error message

* chore: fmt

* fix: revert windows-rs 0.28 version bump

* fix(linux): fix native menu items (rust-windowing#256)

* chore: remove examples commited by accident

* Update `ReceivedImeText` (rust-windowing#251)

* Allow receiving text without Ime on Windows

* Avoid panic todo

* Receive text without ime on mac

* Fix CursorMoved event on Linux

* Add ReceivedImeText on Linux

This only add Simple IME from GTK for now. We should add the actual IME
from system in the future.

* Fix redraw event that causes inifinite loop (rust-windowing#260)

* Fix redraw event that causes inifinite loop

* Refactor event loop

* Remove unused function

* Add doc comment on linux's run_return

* Ignore doc test on run_return

* Add non blocking iteration on Linux (rust-windowing#261)

* Docs: SystemTrayExtWindows::remove() is gone (rust-windowing#262)

Fix docs following rust-windowing#153

* Fix busy loop on Linux (rust-windowing#265)

* Update windows crate to 0.29.0 (rust-windowing#266)

* Update to windows 0.29.0

* Add change description

* Remove clippy check (rust-windowing#267)

* refactor(windows): align util function with win32 names

* chore: update PR template

* fix(linux): fire resized & moved events on min/maximize, closes rust-windowing#219 (rust-windowing#254)

* feat(linux): implement `raw_window_handle()` (rust-windowing#269)

* chore(deps): update to raw-window-handle 0.4

* add linux raw-window-handle support

* update macos/ios/android

* fix ios

* Fix core-video-sys dependency (rust-windowing#274)

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

* Fix some invalid msg_send! usage (rust-windowing#276)

* Revert "Fix some invalid msg_send! usage (rust-windowing#276)" (rust-windowing#277)

This reverts commit a3a2e0cfc49ddfa8cdf65cf9870fb8e3d45b4bc0.

* Revert "The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#275)" (rust-windowing#279)

This reverts commit 6f9c468f26ddb60e29be2139397bfaf3b30eab1e.

* The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. (rust-windowing#280)

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

Co-authored-by: madsmtm <[email protected]>

* Fix some invalid msg_send! usage (rust-windowing#278)

Co-authored-by: madsmtm <[email protected]>

* Add exit code to ControlFlow::Exit (rust-windowing#281)

* Add exit code to ControlFlow::Exit

* Cargo fmt

* Add change files

Co-authored-by:  multisn8 <[email protected]>

* Add new_any_thread to Unix event loop (rust-windowing#282)

* Update windows crate to 0.30.0 (rust-windowing#283)

* Update windows crate to 0.30.0

* Simplify new-type usage

* Fix boxing in GWL_USERDATA

* Make sure everyone is using Get/SetWindowLongPtrW

* build the system_tray module when "ayatana" feature is enabled (rust-windowing#285)

Without those cfg feature checks, the "ayatana" feature does
actually not enable anything.

* Fix click events missing whe tray has menu (rust-windowing#291)

* Fix click events missing whe tray has menu

* Add change file

* Fix crash when tray has no menu (rust-windowing#294)

* chore: update pull request commit exmple

* fix(windows): send correct position for system tray events, closes rust-windowing#295 (rust-windowing#300)

* fix(windows): revert maximized state handling to winit impl, closes rust-windowing#193 (rust-windowing#299)

* fix(windows): revet maximized state handling to winit impl, closes rust-windowing#193

* add chanefile [skip ci]

* fix: `MenuItem::Quit` on Windows (rust-windowing#303)

* fix: `MenuItem::Close` on Windows

* use `PostQuitMessage` instead

Co-authored-by: amrbashir <[email protected]>

* feat: v1 audit by Radically Open Security (rust-windowing#304)

* Update to gtk 0.15 (rust-windowing#288)

* Update to gtk 0.15

* Fix picky none on set_geometry_hint

* Fix CursorMoved position

Co-authored-by: Amr Bashir <[email protected]>
Co-authored-by: Bill Avery <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
Co-authored-by: Kasper <[email protected]>
Co-authored-by: amrbashir <[email protected]>
Co-authored-by: Jay Oster <[email protected]>
Co-authored-by: madsmtm <[email protected]>
Co-authored-by: multisn8 <[email protected]>
Co-authored-by: Aurélien Jacobs <[email protected]>
Co-authored-by: Lucas Fernandes Nogueira <[email protected]>
@CrabNejonas
Copy link

CrabNejonas commented Oct 9, 2023

For posterity and because this ended up being one of the few resources about this, I'm gonna post this here too:
There is a way for nextEventMatchingMask:untilDate:inMode:dequeue to not block during resizing by switching into the eventTracking run loop mode and intercepting the mouse down event on the resize areas. The obvious downside here is that one would have to implement the window resizing themselves using the mouse drag events.
As @madsmtm mentioned on matrix though, aside from being more work implementing this it has other downsides e.g. potential compatibility problems and so on since this is essentially relying on undocumented quirks in appkit (but then again, what is not on macOS am I right?)

@eliasnaur
Copy link

@CrabNejonas I'd like to implement resizing myself to avoid this very issue of nextEventMatchingMask blocking during resize. However, I don't understand what you mean by "switching into the eventTracking run loop mode" and how to determine whether a button down is within the "resize areas".

There is a way for nextEventMatchingMask:untilDate:inMode:dequeue to not block during resizing by switching into the eventTracking run loop mode and intercepting the mouse down event on the resize areas.

I assume a CFRunLoopObserver can be used to detect when the NSWindow enters its nested run loop (in eventTracking mode), but that's too late to do anything, except maybe sending a synthetic button up event.

@CrabNejonas
Copy link

CrabNejonas commented Dec 14, 2023

I assume a CFRunLoopObserver can be used to detect when the NSWindow enters its nested run loop (in eventTracking mode), but that's too late to do anything, except maybe sending a synthetic button up event.

The way I have implemented this is through testing for a click on the "resize areas" when a NSLeftMouseDown event is received. If you check for directly after nextEventMatchingMask that happens before it is dispatched to the NSView (or whatever downstream component handles the inner loop I don't remember in detail rn) in case the click hit the resize area I simply don't forward it (i.e. call sendEvent only for all other events) that way as far as the rest of macOS code is concerned there never was a resize to begin with.

Then you just switch the mode for nextEventMatchingMask to the event tracking one and do the resizing yourself

@eliasnaur
Copy link

eliasnaur commented Dec 14, 2023

Thanks, @CrabNejonas for the help. Your idea sounds good, but I've been unable to figure out how to robustly determine whether a button press is bound for the resize areas. How do you do that? If I have to hard-code the areas, I risk getting them wrong and a button press slipping through to the NSWindow inner resize loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - waiting on author Waiting for a response or another PR D - hard Likely harder than most tasks here DS - macos P - high Vital to have S - platform parity Unintended platform differences
9 participants