Skip to content

Update the macOS backend to the new futures-compatible API.#132

Merged
tomaka merged 14 commits intorust-windowing:masterfrom
mitchmindtree:macos_event_loop
Feb 14, 2017
Merged

Update the macOS backend to the new futures-compatible API.#132
tomaka merged 14 commits intorust-windowing:masterfrom
mitchmindtree:macos_event_loop

Conversation

@mitchmindtree
Copy link
Copy Markdown
Contributor

@mitchmindtree mitchmindtree commented Feb 3, 2017

This is a follow up to the new API introduced in #126.

Summary of Changes

  • A new module was added for defining the events_loop. Most event handling code from the old mod.rs has been moved into this module. The NSEventToEvent function has been moved to a private ns_event_to_event method on the EventsLoop itself.
  • The global static mut key modifier trackers (SHIFT_PRESSED, ALT_PRESSED, etc) have been removed in favour a Modifiers struct which is owned by the EventsLoop instance.
  • The event module was removed in favour of moving the one function that was inside of it (used for converting cocoa key codes to VirtualKeyCodes) to the new events_loop module.
  • Window, WindowDelegate, DelegateState and related items have been moved from mod.rs into a new window.rs module.
  • The old PollEventsIterator and WaitEventsIterators have been removed in favour of the new EventsLoop::poll_events and EventsLoop::run_forever methods respectively.
  • WindowProxy was removed in favour of the new API. Its logic for waking up the waiting event loop was moved to the EventsLoop::interrupt method.
  • Add a test that will only compile if the EventsLoop is Send + Sync. This should make it clearer to contributors that the backend that they're working on must be Send + Sync.

Fixes

unsafe code

Unsafe code was necessary in order to share the user-given callback between each of the window delegate callbacks. A *mut FnMut(Event) is stored behind a Mutex in order to do this. I've explained the behaviour in detailed comments on the UserCallback and its methods. If someone could review the new UserCallback type and its usage, that would be greatly appreciated!

WIP todo:

  • Implement interrupted properly. At the moment it causes run_forever to return ASAP after the next event, however it should actually wake-up the "awaiting event" function.
  • Ensure that multi-window handling is working properly and that events are correctly emitted for each window (should fix Multiwindow example is broken on OS X #56).
  • Work out how to ensure Send + Sync are safely impld for EventsLoop without using unsafe impl. Currently neither are implemented as the EventsLoop's callback field is not Send or Sync:
    the trait `std::marker::Send` is not implemented for `std::ops::FnMut(winit::Event) + 'static
    
    Edit: I think I've fixed this by wrapping the user callback in a UserCallback type and impling Send and Sync for it.
  • Ensure that new windows become the "main" window.
  • Fix mouse Enter/Leave events. At the moment, Enter is only triggered when the mouse is over the resize bar and Leave triggers when leaving the resize bar 😖. This seems to require setting up tracking areas which seems like a more involved fix. Going to leave this for a future PR.

@tomaka feel free to merge #126 in the meantime if this PR is not yet ready. In that case, I'll just close this and re-open against master.

This is a follow up to the new API introduced in rust-windowing#20.

This also fixes the issue where window resize events would not be
emitted until the end of the resize. This PR fixese rust-windowing#39 by ensuring that
the user callback given to either `EventsLoop::poll_events` or
`EventsLoop::run_forever` can be called by each window delegate's resize
callback directly.
invariants

This also removes the need for "box"ing the callback in favour of
storing a raw `*mut` pointer. We can do this by ensuring that we never
store the pointer for longer than the lifetime of the user callback,
which is the duration of a call to `poll_events` or `run_forever`.

Also removes old commented out event code from the window module.
Fix issue where key window would lose all mouse events once mouse left
that window.

Make sure that only window under mouse receives mouse scroll wheel
events.
the NSApplication is in focus.

This NSEvent produces an undocumented NSEventType value `21` that has no
associated variant within the cocoa-rs crate's `NSEventType` enum, thus
causing a segfault when attemptingt to match on the value.

This commit adds a check for `21` to avoid the segfault.

This fixes rust-windowing#104.
@mitchmindtree mitchmindtree changed the title [WIP] Update the macOS backend to the new futures-compatible API. Update the macOS backend to the new futures-compatible API. Feb 5, 2017
@mitchmindtree
Copy link
Copy Markdown
Contributor Author

OK @tomaka I think this should be ready for review.

cc @paulrouget @jdm @pcwalton @davll @jwilm - sorry to bother, you are the only other macOS winit contributors that I'm aware of!

fn new(state: DelegateState) -> WindowDelegate {
// Box the state so we can give a pointer to it
let mut state = Box::new(state);
let state_ptr: *mut DelegateState = &mut *state;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this in some of the old code. This should probably use Box::into_raw, otherwise I'm pretty sure state gets freed at the end of this scope when drop is called on the Box. Seems to me that this has just been "working" by pure chance that the memory at the address hasn't been over-written?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah never mind, I thought the WindowDelegate was only storing the pointer, but it takes ownership of the Box itself.

@mitchmindtree mitchmindtree changed the base branch from impl-20 to master February 11, 2017 23:31
@mitchmindtree
Copy link
Copy Markdown
Contributor Author

Now that #126 has been merged I've rebased this against the master branch.

@MichaelShaw
Copy link
Copy Markdown

Testing the PR locally on macOS 10.12.3 with an Iris Pro.

#39, #104 , #56 & #31 that were all broken on master are fixed on this PR.

Haven't checked the Event::Awakened related fix as I don't know a way to produce unknown NSEvents on master.

varphone added a commit to varphone/winit that referenced this pull request May 13, 2025
…ons < 17763

In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll` also returns a non-null pointer.
However, the function does not match the expected `extern "system" fn() -> bool` prototype,
which causes a crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such crashes on older Windows versions.
varphone added a commit to varphone/winit that referenced this pull request May 13, 2025
…ons < 17763

In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll` also returns a non-null pointer.
However, the function does not match the expected `extern "system" fn() -> bool` prototype,
which causes a crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such crashes on older Windows versions.
varphone added a commit to varphone/winit that referenced this pull request May 13, 2025
…ons < 17763

In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll` also returns a non-null pointer.
However, the function does not match the expected `extern "system" fn() -> bool` prototype,
which causes a crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such crashes on older Windows versions.
kchibisov pushed a commit that referenced this pull request May 14, 2025
In Windows versions < 17763, `GetProcAddress("#132")` from `uxtheme.dll`
also returns a non-null pointer. However, the function does not match
the expected `extern "system" fn() -> bool` prototype, which causes a
crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such
crashes on older Windows versions.
kchibisov pushed a commit to kchibisov/winit that referenced this pull request May 17, 2025
In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll`
also returns a non-null pointer. However, the function does not match
the expected `extern "system" fn() -> bool` prototype, which causes a
crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such
crashes on older Windows versions.
kchibisov pushed a commit to kchibisov/winit that referenced this pull request May 17, 2025
In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll`
also returns a non-null pointer. However, the function does not match
the expected `extern "system" fn() -> bool` prototype, which causes a
crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such
crashes on older Windows versions.
kchibisov pushed a commit to kchibisov/winit that referenced this pull request May 17, 2025
In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll`
also returns a non-null pointer. However, the function does not match
the expected `extern "system" fn() -> bool` prototype, which causes a
crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such
crashes on older Windows versions.
kchibisov pushed a commit to kchibisov/winit that referenced this pull request May 17, 2025
In Windows versions < 17763, `GetProcAddress("rust-windowing#132")` from `uxtheme.dll`
also returns a non-null pointer. However, the function does not match
the expected `extern "system" fn() -> bool` prototype, which causes a
crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such
crashes on older Windows versions.
kchibisov pushed a commit that referenced this pull request May 21, 2025
In Windows versions < 17763, `GetProcAddress("#132")` from `uxtheme.dll`
also returns a non-null pointer. However, the function does not match
the expected `extern "system" fn() -> bool` prototype, which causes a
crash when it is called.

This fix ensures compatibility by adding proper checks to prevent such
crashes on older Windows versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants