Key Events have changed in ImGui

[jvannugteren]

In the latest version of ImGui the key events enums have shifted by 512 preventing proper capture of keys such as delete and backspace. Now the function "AddKeyEvent" has to be used to do the translation.

I think the issue can be fixed by replacing the switch in src/Magnum/ImGuiIntegration/Context.hpp file to:

switch(event.key()) {
        /* LCOV_EXCL_START */
        case KeyEvent::Key::LeftShift:
        case KeyEvent::Key::RightShift:
            io.KeyShift = value;
            break;
        case KeyEvent::Key::LeftCtrl:
        case KeyEvent::Key::RightCtrl:
            io.KeyCtrl = value;
            break;
        case KeyEvent::Key::LeftAlt:
        case KeyEvent::Key::RightAlt:
            io.KeyAlt = value;
            break;
        case KeyEvent::Key::LeftSuper:
        case KeyEvent::Key::RightSuper:
            io.KeySuper = value;
            break;
        case KeyEvent::Key::Tab:
            io.AddKeyEvent(ImGuiKey_Tab, value);
            break;
        case KeyEvent::Key::Up:
            io.AddKeyEvent(ImGuiKey_UpArrow, value);
            break;
        case KeyEvent::Key::Down:
            io.AddKeyEvent(ImGuiKey_DownArrow, value);
            break;
        case KeyEvent::Key::Left:
            io.AddKeyEvent(ImGuiKey_LeftArrow, value);
            break;
        case KeyEvent::Key::Right:
            io.AddKeyEvent(ImGuiKey_RightArrow, value);
            break;
        case KeyEvent::Key::Home:
            io.AddKeyEvent(ImGuiKey_Home, value);
            break;
        case KeyEvent::Key::End:
            io.AddKeyEvent(ImGuiKey_End, value);
            break;
        case KeyEvent::Key::PageUp:
            io.AddKeyEvent(ImGuiKey_PageUp, value);
            break;
        case KeyEvent::Key::PageDown:
            io.AddKeyEvent(ImGuiKey_PageDown, value);
            break;
        case KeyEvent::Key::Enter:
        case KeyEvent::Key::NumEnter:
            io.AddKeyEvent(ImGuiKey_Enter, value);
            break;
        case KeyEvent::Key::Esc:
            io.AddKeyEvent(ImGuiKey_Escape, value);
            break;
        case KeyEvent::Key::Space:
            io.AddKeyEvent(ImGuiKey_Space, value);
            break;
        case KeyEvent::Key::Backspace:
            io.AddKeyEvent(ImGuiKey_Backspace, value);
            break;
        case KeyEvent::Key::Delete:
            io.AddKeyEvent(ImGuiKey_Delete, value);
            break;
        case KeyEvent::Key::A:
            io.AddKeyEvent(ImGuiKey_A, value);
            break;
        case KeyEvent::Key::C:
            io.AddKeyEvent(ImGuiKey_C, value);
            break;
        case KeyEvent::Key::V:
            io.AddKeyEvent(ImGuiKey_V, value);
            break;
        case KeyEvent::Key::X:
            io.AddKeyEvent(ImGuiKey_X, value);
            break;
        case KeyEvent::Key::Y:
            io.AddKeyEvent(ImGuiKey_Y, value);
            break;
        case KeyEvent::Key::Z:
            io.AddKeyEvent(ImGuiKey_Z, value);
            break;
        /* LCOV_EXCL_STOP */

        /* Unknown key, do nothing */
        default: return false;
    }

[pezcode]

I'm gonna leave this here, some useful info on the why's and how's: ocornut/imgui#4858. Also note this little comment from the same link:

I think we will try to make some more use of it before declaring the change valid.

Might be worth waiting until this hits a tagged release to actually add support for this, in case they pull or rework this.

[jvannugteren]

Fair enough. The "backwards compatibility" doesn't currently work for us though.

[pezcode]

Okay, I understand the immediate issue now. Regardless of the AddKeyEvent changes, we should have been doing something like:

// at startup
io.KeyMap[ImGuiKey_Tab] = KeyEvent::Key::Tab;

// in the switch
io.KeysDown[KeyEvent::Key::Tab];

This fixes the issue of out-of-bounds indexing io.KeysDown with the new remapped ImGuiKey enum, and as a nice side-effect we can save ourselves the switch altogether + users get a full key map in io.KeysDown with platform-specific keys.

The AddKeyEvent changes need a bit more thought, since we need backwards-compatibility with older imgui versions.

[pezcode]

Actually it's not as simple: ocornut/imgui#2269 (comment)

Maybe for now we have to hack around this by ifdefing around ImGuiKey_Tab being >= 512 and then subtracting ImGuiKey_NamedKey_BEGIN. Sigh 😵‍

[pezcode]

I opened a PR that should at least fix the backward compatibility. Let me know if that works for you or doesn't (ideally on the PR page).

[ocornut]

Indeed this is one (unusual but technically valid) use case we didn't think about when designing the backward compatibility....
Unfortunately I'm not sure how we can fix it.
We will be adding new asserts to detect it + mark this specific use case as a breaking change.

[ocornut]

Might be worth waiting until this hits a tagged release to actually add support for this, in case they pull or rework this.

FYI the Key aspect of this change IHMO at this point is a settled API.
Bug/tweaks may arise before we tag 1.87 but your feedback here would be super helpful.

What will not yet committed and will come before we tag 1.87 is transitioning other inputs into event calls (e.g io.AddMouseButtonEvent() standardizing the new IO API and allowing trickling input queue (to handle very slow framerate better). That may be pushed next week.

[jvannugteren]

Sure I'll keep an eye out for issues that may emerge due to these changes. My "Rat" GUI code requires docking branch, so normally I find out when you merge the master into that.

[mosra]

This is now fixed with #93, merged as c60149c.