Skip to content

macOS: Unbundled window activation hack#1318

Merged
august64 merged 19 commits intorust-windowing:masterfrom
august64:macos-unbundled
Jan 11, 2020
Merged

macOS: Unbundled window activation hack#1318
august64 merged 19 commits intorust-windowing:masterfrom
august64:macos-unbundled

Conversation

@august64
Copy link
Copy Markdown
Member

  • Tested on all platforms changed
  • Compilation warnings were addressed
  • cargo fmt has been run on this branch
  • cargo doc builds successfully
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

The explanation for this PR is actually in a wall of comments, so check the diff for context. I have a tendency to view winit as a Rosetta stone of windowing, hence the thorough commenting...

I scanned through the issue tracker, and it doesn't look like there's an open issue for this. You'd think someone from Alacritty would've reported this already, but since Alacritty comes so nicely bundled, only someone developing Alacritty would be able to hit this. I also suspect that there aren't many people developing with winit on macOS in general.

As an exciting bonus, I removed the commented-out cruft I left in app_delegate.rs last year. It was just copy-pasted from the iOS backend, which I was using as a reference. It was all intended to be removed, but I forgot to ever clean it up.

Solution derived from this Godot PR: godotengine/godot#17187
Equivalent GLFW issue: glfw/glfw#1515

@august64 august64 added DS - appkit Affects the AppKit/macOS backend C - waiting on maintainer A maintainer must review this code labels Dec 14, 2019
Copy link
Copy Markdown

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This all looks really complicated. I believe the conventional way to do this is to simply call [notification.object activateIgnoringOtherApps:YES]; in the applicationWillFinishLaunching: notification for the NSApplicationDelegate. Is there a reason this doesn't work?

@august64
Copy link
Copy Markdown
Member Author

@aleksijuvani this PR exists specifically because that doesn't work, though you're free to try yourself.

@august64
Copy link
Copy Markdown
Member Author

@vbogaevsky would you be able to take a look at this?

@vbogaevsky
Copy link
Copy Markdown
Contributor

@francesca64, I'll review and test it this night.

Copy link
Copy Markdown
Contributor

@vbogaevsky vbogaevsky left a comment

Choose a reason for hiding this comment

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

First, I tried to recreate uninteractable state on master with window and window_run_return examples and was unsuccessful.
When the app started, if it was not in focus, it got into focus automatically. Was I doing something wrong?
Then I've tried same procedure on merge branch, and both times macOs dropdown menu was inaccessible until I switched to Finder and back to the application. Is it intended behaviour?

@august64
Copy link
Copy Markdown
Member Author

@vbogaevsky that sounds like the opposite of what I'd expect; what you observed on macos-unbundled is what's expected on master, and what you observed on master is exactly what's expected on macos-unbundled. Can you verify that the branches weren't swapped?

(Enabling info logging can help verify what's happening as well, though that would require plugging in env_logger...)

Copy link
Copy Markdown
Contributor

@vbogaevsky vbogaevsky left a comment

Choose a reason for hiding this comment

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

Currently on MacOS Catalina when an app starts it gets into focus, but drop down menu does not respond until you switch to another app and back again.

@august64
Copy link
Copy Markdown
Member Author

august64 commented Jan 2, 2020

@vbogaevsky in which case? I'm having trouble following you, so being more explicit about context would help me a lot.

@vbogaevsky
Copy link
Copy Markdown
Contributor

@francesca64 in both cases (master and PR branch).
On PR branch the app is active i.e. msg_send![NSApp(), isActive] and State::get_activated(this) return YES and true respectively.
But in OS dropdown menu is inaccessible. To make it accessible you have to switch to another app and then back.

@august64
Copy link
Copy Markdown
Member Author

@vbogaevsky ah, thanks. I'm able to repro that, but it doesn't seem to be a regression?

How acceptable is this PR?

@vbogaevsky
Copy link
Copy Markdown
Contributor

@francesca64 It's not a regression indeed, and I believe we can merge after resolving conflicts in src/platform_impl/macos/app.rs

@august64
Copy link
Copy Markdown
Member Author

@vbogaevsky thanks!

(I didn't notice you'd already solved the conflicts, so double thanks! Now I feel silly for having made those merge commits.)

@august64 august64 merged commit 1ddceeb into rust-windowing:master Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C - waiting on maintainer A maintainer must review this code DS - appkit Affects the AppKit/macOS backend

Development

Successfully merging this pull request may close these issues.

3 participants