Fix NSApplication initialization to prevent some issues on macOS #131
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
martincapello
force-pushed
the
fix-macos-fullscreen-issue
branch
from
92397f0 to
08bda1c
Compare
| // need the application's initialization done inside run to prevent issues | ||
| // such as: https://github.com/aseprite/aseprite/issues/4795 | ||
| [m_app run]; | ||
|
|
||
| [m_appDelegate resetCliFiles]; |
There was a problem hiding this comment.
Should we put this line under the scope of the if clause as well?
There was a problem hiding this comment.
Just in case, we should test if dropping files to the app icon in the dock works. Probably it should be tested with the final Aseprite.app bundle, not only the bin/aseprite binary. I could give a try next week.
There was a problem hiding this comment.
I've tested with the binary with and without this fix, and it doesn't work either way. So it seems that we can only test this with the final bundle?
There was a problem hiding this comment.
I've just kind of tested this by vandalizing a v1.3.12 bundle (replaced the release binary with a compiled one 😬) and it worked...not sure if it will work by creating the bundle, but seems like it.
|
clang-tidy review says "All clean, LGTM! 👍" |
By calling [NSApplication run] and then [NSApplication stop] we are able to initialize the application instance and prevent issues in fullscreen mode (fix aseprite/aseprite#4795)
martincapello
force-pushed
the
fix-macos-fullscreen-issue
branch
from
08bda1c to
5237850
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
os/osx/app.mm
Outdated
| if (![[NSRunningApplication currentApplication] isFinishedLaunching]) | ||
| // Note that the [m_app run] call doesn't block because we are calling | ||
| // [NSApp stop] from [AppDelegateOSX applicationDidFinishLaunching]. We only | ||
| // need the application's initialization done inside run to prevent issues | ||
| // such as: https://github.com/aseprite/aseprite/issues/4795 | ||
| [m_app run]; |
There was a problem hiding this comment.
Just add {} to contain the whole comment+statement:
| if (![[NSRunningApplication currentApplication] isFinishedLaunching]) | |
| // Note that the [m_app run] call doesn't block because we are calling | |
| // [NSApp stop] from [AppDelegateOSX applicationDidFinishLaunching]. We only | |
| // need the application's initialization done inside run to prevent issues | |
| // such as: https://github.com/aseprite/aseprite/issues/4795 | |
| [m_app run]; | |
| if (![[NSRunningApplication currentApplication] isFinishedLaunching]) { | |
| // Note that the [m_app run] call doesn't block because we are calling | |
| // [NSApp stop] from [AppDelegateOSX applicationDidFinishLaunching]. We only | |
| // need the application's initialization done inside run to prevent issues | |
| // such as: https://github.com/aseprite/aseprite/issues/4795 | |
| [m_app run]; | |
| } |
| // need the application's initialization done inside run to prevent issues | ||
| // such as: https://github.com/aseprite/aseprite/issues/4795 | ||
| [m_app run]; | ||
|
|
||
| [m_appDelegate resetCliFiles]; |
There was a problem hiding this comment.
Just in case, we should test if dropping files to the app icon in the dock works. Probably it should be tested with the final Aseprite.app bundle, not only the bin/aseprite binary. I could give a try next week.
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Please @martincapello follow this rule https://github.com/aseprite/aseprite/blob/bbe656c856453b6a3d5cd617f8312417bcf11035/docs/CODING_STYLE.md?plain=1#L54-L59 even if those multiple lines are just comments. |
martincapello
force-pushed
the
fix-macos-fullscreen-issue
branch
from
a567b01 to
8099a17
Compare
I saw your comment just after I pushed the last commit... |
|
clang-tidy review says "All clean, LGTM! 👍" |
By calling [NSApplication run] and then [NSApplication stop] we are able to initialize the application instance and prevent issues in fullscreen mode (fix aseprite/aseprite#4795)