Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImguiIntegration: support for imgui IO event API #93

Closed
wants to merge 5 commits into from

Conversation

pezcode
Copy link
Contributor

@pezcode pezcode commented Jan 19, 2022

This adds support for the new IO handling about to land in imgui 1.87. The main advantages:

  • Full range of keys that roughly matches the Application classes' KeyEvent::Key, meaning you can do key handling with imgui, independent of the Application class
  • Input events are trickled, e.g. mouse button down followed by mouse button up in the same frame is now spaced out over two frames. This makes the manual workaround for mouse buttons obsolete and fixes the same issue for keys.

A few notes:

Todo:

  • Test with 1.87 final and update the API version used to check for the new IO API

Beginning with imgui 1.87 ImGuiKey_* values start at 512, which means we can't use them directly to index io.KeysDown (which has size 512, not coincidentally). This is a quick fix to make the legacy code work. Long-term we need to add support for the new key system introduced in 1.87 which lets us queue a full range of backend-agnostic ImGuiKey with AddKeyEvent().
@codecov
Copy link

codecov bot commented Jan 19, 2022

Codecov Report

Merging #93 (a29dea2) into master (da3c1ab) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
- Coverage   77.25%   77.06%   -0.20%     
==========================================
  Files          21       21              
  Lines         941      933       -8     
==========================================
- Hits          727      719       -8     
  Misses        214      214              
Impacted Files Coverage Δ
src/Magnum/ImGuiIntegration/Context.cpp 89.13% <100.00%> (-1.13%) ⬇️
src/Magnum/ImGuiIntegration/Context.hpp 86.48% <100.00%> (+1.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da3c1ab...a29dea2. Read the comment docs.

Copy link
Owner

@mosra mosra left a comment

Choose a reason for hiding this comment

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

Thanks! I'll need a bit more time to wrap my head around all the changes, so this is just a partial review.

src/Magnum/ImGuiIntegration/Context.hpp Outdated Show resolved Hide resolved
This was linked to issues Feb 3, 2022
@pezcode pezcode marked this pull request as ready for review February 7, 2022 20:18
@mosra
Copy link
Owner

mosra commented Feb 9, 2022

Merged as c60149c (I squashed all changes together since it didn't seem to make sense to preserve the later-removed temporary hack), thanks a lot!

@mosra mosra closed this Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ImGUI IO assertion failure Key Events have changed in ImGui
2 participants