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

Update link to Magnum bindings #2269

Merged
merged 1 commit into from
Jan 10, 2019
Merged

Conversation

mosra
Copy link
Contributor

@mosra mosra commented Jan 9, 2019

I noticed links to Magnum bindings in the README (thanks for adding them, btw!), these community projects now became an official part of the library, so I updated the links to point to an up-to-date place.

Thank you! :)

The various community projects that integrated Dear ImGui into Magnum
were merged together and are now an official part of the engine.
@ocornut ocornut merged commit 289569e into ocornut:master Jan 10, 2019
@ocornut
Copy link
Owner

ocornut commented Jan 10, 2019

Thank you, merged this.
I tested your web demo and right-click seemed to trigger the browser menu :( not sure how easy it is to fix within web world.

@mosra
Copy link
Contributor Author

mosra commented Jan 10, 2019

Oh, I missed that, thanks for the heads-up! Should be fixed now (it may need a force-refresh, tho) :)

@mosra mosra deleted the magnum-integration branch January 10, 2019 14:41
@ocornut
Copy link
Owner

ocornut commented Jan 11, 2019

I also noticed you are not setting up the key mapping properly, in context:
https://github.com/mosra/magnum-integration/tree/master/src/Magnum/ImGuiIntegration

io.KeyMap[ImGuiKey_Tab]        = ImGuiKey_Tab;
io.KeyMap[ImGuiKey_LeftArrow] = ImGuiKey_LeftArrow;

etc.

The intent of the io.KeyDown[] and io.KeyMap[] array is to store data indexable by your native type (e.g. KeyEvent::Key::XXX) in io.KeyDown[], so user can benefit from various inputs function in ImGui using your native enums.

However in Magnum those values seem to differ per platform, in particular, SDL's SDLK_xxx values (unlike SDL_SCANCODE) are not small indices making it problematic to use those index directly for SDL.
I don't have a better solution other than making your KeyEvent::Key::XXX values a fixed enum with valid/small indexes or waiting until I can safely refactor the input system on my side. Maybe worth adding comments in the integration code.

@mosra
Copy link
Contributor Author

mosra commented Jan 11, 2019

Hmm. Yes, I'm aware of this. I guess it's a tradeoff between either having to map from platform-specific (SDL, GLFW, ...) enums to generic zero-based enums or having to map from Magnum enums to ImGui. Doing a mapping from SDL/... to zero-based enums would only move this problem elsewhere, and the users won't be able to use Magnum enums when peeking into SDL_Event etc. directly. In case of mapping from Magnum to ImGui the switch-case is much smaller because you have a small and clearly-defined set of keys. And since the integration code is providing this mapping for the user, it shouldn't lead to many inconveniences, I think.

This intention needs to be documented a bit better in the code, yeah. And I'll revisit this once you have updates to the event handling. Thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants