fix: OSX backend improvements #4759

stuartcarnie · 2021-11-24T19:34:27Z

This PR makes changes to the macOS backend and the Apple example that uses the macOS backend.

Firstly, the PR addresses some issues and shortcomings of in imgui_impl_osx.mm as follows

Fixed keyboard support using kVK_ codes and keyboard modifier handling.
- Keyboard navigation now works correctly for features such as navigating windows
Added complete gamepad support via GameController framework

Secondly, the example_apple_metal.mm example has addressed the unhandled key event chimes, which adds a keyDown: handler.

This PR does not touch the core of IMGUI.

NOTE:

There is a single breaking change to the ImGui_ImplOSX_Init API, which now takes the same NSView being used by the ImGui_ImplOSX_NewFrame. This NSView is used to attach an NSTextInputClient to the responder chain to process key presses using the same approach as GLFW and SDL. This approach ensures the macOS ImGui backend us macOS APIs for determining characters that should be accepted as text input.

ocornut · 2021-11-24T19:45:47Z

backends/imgui_impl_osx.mm

-            if (key != -1)
-                io.KeysDown[key] = true;
+            NSString* str = [event characters];
+            io.AddInputCharactersUTF8(str.UTF8String);


Some characters were previously intentionally filtered, why removing this?

Ahh, sorry. I will provide a fix for this.

I will also resolve the OpenGL build

It's worth double checking if those checks are still valid, or if they are specific to old versions of the OS. like the 127 code test I believe is checking for the delete key, but I don't have any keyboards that generate the 127 code. And I think the 0xF700 0xFF00 range might be an old way of filtering F keys? I might be wrong on this, but I think that's what this line of code was intended for.

I've reworked it to use macOS's NSTextInputClient protocol, which is the same as GLFW:

https://github.com/glfw/glfw/blob/b55a517ae0c7b5127dffa79a64f5406021bf9076/src/cocoa_window.m#L722-L723

and SDL2:

https://github.com/spurious/SDL-mirror/blob/e17aacbd09e65a4fd1e166621e011e581fb017a8/src/video/cocoa/SDL_cocoakeyboard.m#L53

This means we only send characters to ImGui when macOS tells us so:

https://github.com/stuartcarnie/imgui/blob/28c32a6c105441eb7557a3c58f9d3d3b69fedc38/backends/imgui_impl_osx.mm#L92-L107

ocornut · 2021-11-24T19:49:18Z

Thank you for this PR!

I didn't develop the original backend but I was always surprised by the fact keycodes were not accessible, it seems like they are? Why do you think this was previously overlooked? Its certainly much nicer in your PR.
As commented above, why was the character filtering removed? You can use git blame on that file to find why/when they were added, if necessary.
CI fails: https://github.com/ocornut/imgui/runs/4316537421?check_suite_focus=true

stuartcarnie · 2021-11-29T23:34:10Z

👋🏻 hey @ocornut, I've updated this PR and expanded the description to reflect the recent changes. The OSX backend should now be ready for another review.

ocornut · 2021-12-03T14:11:22Z

It's worth double checking if those checks are still valid, or if they are specific to old versions of the OS. like the 127 code test I believe is checking for the delete key, but I don't have any keyboards that generate the 127 code. And I think the 0xF700 0xFF00 range might be an old way of filtering F keys? I might be wrong on this, but I think that's what this line of code was intended for.

git blame is your friend and this is all pretty recent.

We can probably merge the gamepad stuff asap (if this is resquashed and split into two distinct commits, as well as brace styles fixed). The other inputs stuff would need more careful review and maybe more comments in the code.

stuartcarnie · 2021-12-10T22:07:37Z

@ocornut I have separated the commits, so that the game controller is isolated. There is also a fix for macOS 12.1 to address incorrect scrolling behaviour (which incidentally breaks a number of other macOS apps I have used)

thedmd · 2021-12-10T22:34:32Z

@ocornut FYI, io_inputqueue does have osx backend redone to handle all keys and modifiers. Maybe we could pull that into backend and cherry-pick gamepad support from this PR?

stuartcarnie · 2021-12-10T23:03:55Z

👋🏻 hi @ocornut!

I have pushed up a separate commit with the macOS keyboard fixes. It includes a number of comments and fixes the brace formatting.

NOTE the resolved FIXME:

// FIXME: All the key handling is wrong and broken. Refer to GLFW's cocoa_init.mm and cocoa_window.mm.

This final commit implements these recommendations and is consistent with GLFW's and SDL2's approach of using a NSTextInputClient to interface with the standard macOS input management system, removing the guesswork from the OSX backed.

stuartcarnie · 2021-12-10T23:04:37Z

@thedmd I've pushed up another commit – can you take a look at that?

stuartcarnie · 2021-12-10T23:05:35Z

@thedmd where is io_inputqueue? I can't find that as a branch here.

thedmd · 2021-12-10T23:13:16Z

io_inputqueue is on private repository. I will try to extract keyboard support to separate commit. Bonus thing is, it does not require tapping to NSView.

I will let you know, when it is ready. :)

Note the original FIXME: refered to GLFWs Cocoa implementation, which is largely what this commit provides.

thedmd · 2021-12-11T23:14:09Z

@stuartcarnie I managed to pull keyboard handling out of the private branch.

https://github.com/thedmd/imgui/tree/thedmd/osx-backend-keyboard

It is based on "extra keys" PR. I made support for it optional, so it is easy to pull and test changes as they are.

After some fiddling I tweaked examples too, to handle key event using NSView, just to get rid of this annoying 'ding' sound when typing.

Feedback is welcome.

thedmd · 2021-12-12T12:26:55Z

I did some cleaning up. In order to get new improved keyboard handling, there is what need to be done:

cherry-pick Backends: OSX: Add support for extra keys (7cf90e5) - this add an optional support for extra keys, does not change a think if you do not use extra keys PR, but will help to prevent conflicts
cherry-pick Backends: OSX: Map virtual keys to ImGui keys(aa6b037) - this add support for all keys ImGui didn't had earlier, including better (but not great) character input
cherry-pick Examples: Use NSView/UIVIew to capture input (0d03a90) - patch to example to use *View for input, which allow backend to consume events and prevent 'dinging' when keystrokes are hit; note, you will hear dinging, unless something that consume test is active (focus any text input to see the results)

stuartcarnie · 2021-12-12T20:22:20Z

@thedmd those look ok, however, I prefer to use macOS's own input manager system via NSTextInputClient, which is also how GLFW and SDL2 are implemented. That does require an NSView to be passed, but I don't think that is a large burden.

I really like the idea of shortcuts and the extra keys!

thedmd · 2021-12-12T21:27:20Z

You're right, NSTextInputClient is the way. Text handling in my code is still crippled.

When your PR I will merged, I will rebase my commits. :)

ocornut · 2021-12-13T10:26:38Z

I've merged the game controller commit b720f1f already, thank you!

…mouse cursor shape unconditionally. (#4759, #4253, #1873) Note the original FIXME: refered to GLFWs Cocoa implementation, which is largely what this commit provides.

ocornut · 2021-12-13T11:03:15Z

Merged everything else now (squashed and tweaked as 1b6b860)

Slightly tangential, but CI says:

example_apple_opengl2/main.mm:149:108: warning: method possibly missing a [super reshape] call [-Wobjc-missing-super-calls]
-(void)reshape                              { [[self openGLContext] update]; [self updateAndDrawDemoView]; }

Any suggestion?

thedmd · 2021-12-13T11:19:07Z

I updated it this way:

-(void)reshape                              { [super reshape]; [[self openGLContext] update]; [self updateAndDrawDemoView]; }

)

…th focusing and tweak speed. (#4759)

ocornut · 2022-01-17T15:23:10Z

There was an inconsistency with gamepad mapping for navigation, fixed b6582a4

ocornut added backends osx/ios labels Nov 24, 2021

ocornut reviewed Nov 24, 2021

View reviewed changes

stuartcarnie force-pushed the sgc/fix_osx branch from d63dca2 to 28c32a6 Compare November 24, 2021 23:18

stuartcarnie requested a review from ocornut November 24, 2021 23:22

stuartcarnie force-pushed the sgc/fix_osx branch 2 times, most recently from 9a680eb to 4fdbf46 Compare November 29, 2021 23:26

ocornut force-pushed the master branch from bdf6cf3 to 657073a Compare December 6, 2021 18:16

stuartcarnie force-pushed the sgc/fix_osx branch from 4fdbf46 to b754802 Compare December 10, 2021 21:42

stuartcarnie added 2 commits December 11, 2021 09:19

feat: macOS add Game Controller support

72e52c5

fix: macOS 12.1 scroll event behaviour has changed.

3a3b9ae

stuartcarnie force-pushed the sgc/fix_osx branch from 6c26d97 to 3a3b9ae Compare December 10, 2021 22:20

fix: OSX Don't set cursor unconditionally.

957b5cb

fix: macOS keyboard support

0e0dfb8

Note the original FIXME: refered to GLFWs Cocoa implementation, which is largely what this commit provides.

stuartcarnie force-pushed the sgc/fix_osx branch from b73bacd to 0e0dfb8 Compare December 11, 2021 08:36

ocornut pushed a commit that referenced this pull request Dec 13, 2021

Backends: OSX: Add Game Controller support. (#4759)

b720f1f

ocornut closed this Dec 13, 2021

ocornut added a commit that referenced this pull request Dec 13, 2021

Fix OSX warnings + Examples/Apple+GL2 calls super reshape. (#4759, #1873

8b83e0a

)

ocornut added a commit that referenced this pull request Dec 13, 2021

Fix OSX warnings + Examples/Apple+GL2 calls super reshape. (#4759, #1873

d735066

)

stuartcarnie deleted the sgc/fix_osx branch December 13, 2021 19:45

ocornut added a commit that referenced this pull request Jan 17, 2022

Backends: OSX: Fix Game Controller nav mapping to use shoulder for bo…

b6582a4

…th focusing and tweak speed. (#4759)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: OSX backend improvements #4759

fix: OSX backend improvements #4759

stuartcarnie commented Nov 24, 2021 •

edited

Loading

ocornut Nov 24, 2021

stuartcarnie Nov 24, 2021

meshula Nov 24, 2021

stuartcarnie Nov 24, 2021 •

edited

Loading

ocornut commented Nov 24, 2021

stuartcarnie commented Nov 29, 2021

ocornut commented Dec 3, 2021

stuartcarnie commented Dec 10, 2021

thedmd commented Dec 10, 2021

stuartcarnie commented Dec 10, 2021

stuartcarnie commented Dec 10, 2021

stuartcarnie commented Dec 10, 2021

thedmd commented Dec 10, 2021

thedmd commented Dec 11, 2021

thedmd commented Dec 12, 2021 •

edited

Loading

stuartcarnie commented Dec 12, 2021 •

edited

Loading

thedmd commented Dec 12, 2021

ocornut commented Dec 13, 2021

ocornut commented Dec 13, 2021

thedmd commented Dec 13, 2021

ocornut commented Jan 17, 2022

fix: OSX backend improvements #4759

fix: OSX backend improvements #4759

Conversation

stuartcarnie commented Nov 24, 2021 • edited Loading

ocornut Nov 24, 2021

Choose a reason for hiding this comment

stuartcarnie Nov 24, 2021

Choose a reason for hiding this comment

meshula Nov 24, 2021

Choose a reason for hiding this comment

stuartcarnie Nov 24, 2021 • edited Loading

Choose a reason for hiding this comment

ocornut commented Nov 24, 2021

stuartcarnie commented Nov 29, 2021

ocornut commented Dec 3, 2021

stuartcarnie commented Dec 10, 2021

thedmd commented Dec 10, 2021

stuartcarnie commented Dec 10, 2021

stuartcarnie commented Dec 10, 2021

stuartcarnie commented Dec 10, 2021

thedmd commented Dec 10, 2021

thedmd commented Dec 11, 2021

thedmd commented Dec 12, 2021 • edited Loading

stuartcarnie commented Dec 12, 2021 • edited Loading

thedmd commented Dec 12, 2021

ocornut commented Dec 13, 2021

ocornut commented Dec 13, 2021

thedmd commented Dec 13, 2021

ocornut commented Jan 17, 2022

stuartcarnie commented Nov 24, 2021 •

edited

Loading

stuartcarnie Nov 24, 2021 •

edited

Loading

thedmd commented Dec 12, 2021 •

edited

Loading

stuartcarnie commented Dec 12, 2021 •

edited

Loading