search

vsg-dev / vsgImGui

Integration of VulkanSceneGraph with ImGui
MIT License
43 stars 28 forks source link

Interrogate WantCaptureKeyboard() to ensure that non-ImGui keyboard events get handled by VSG #37

Closed rolandhill closed 1 year ago

rolandhill commented 1 year ago

Reinstating io.WantCaptureKeyboard. Without this, all events are handled by ImGui and the regular VSG event handling system won't receive any keyboard events.

robertosfield commented 1 year ago

I'm confused by the title of this PR. This looks to picked up with the first commit in this PR. More looks to be going on rather just a test.

Could you explain what this PR is addressing? I presume it's not just a test, but something functional.

rolandhill commented 1 year ago

The title was ambiguous now that I reread it. I've changed it now.

First commit: SendEventsToImgui originally checked if ImGui needs keyboard input using io.WantCaptureKeyboard(). If false, then the 'handled' flag is not set and the keyboard event can then be handled by the VSG handler. This check was removed in @kannode's PR, so now ImGui processes all keyboard events whether you are focussed on an ImGui control or not and it sets the 'handled' flag.

Second commit: Checking for io.WantCaptureKeyboard() introduces a problem. When the user presses Enter in an ImGui control, ImGui's desire to capture the keyboard is removed on the key press. Therefore, when we check io.WantCaptureKeyboard() in the key release event it returns negative and screws up subsequent attempts to enter text in controls. We therefore need to also process Enter keys in the ImGui KeyRelease handler.

Third commit : The new event handling in ImGui includes functions to add mouse events. This commit isn't strictly necessary, but I thought that it's good practice to use the functions provided by ImGui rather than setting values directly.

The first two commits are really just replacing code that was already there prior to the update to ImGui 1.89.

kannode commented 1 year ago

Roland I am going to try out your PR.

robertosfield commented 1 year ago

@kannode how did you get on with testing this PR?

kannode commented 1 year ago

@robertosfield @rolandhill The PR worked well for me on Mac.

robertosfield commented 1 year ago

Thanks for the testing. I'll go ahead and merge :-)

kannode commented 1 year ago

@rolandhill Thanks for upgrading to use the newer AddXX