-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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
Shortcuts API #456
Comments
I tested this (in the opengl_example) on Mac and added a printf() in KeyCallback function and I got there everytime when pressing some key and holding ctrl, alt or command. |
Sorry my request wasn't clear, the problem is with the CharCallback not the KeyCallback. To enable CharCallback with modifiers in GLFW 3.1 you need change |
Did the changes and Alt seems to work just fine. Ctrl seems to work in some cases but not in others. |
Any specific pattern with CTRL working vs not working? Thanks for looking, this is really helpful. This stupid app-side input problem has been the number one barrier for adding shortcuts so I am eager to solve it. I submitted a GLFW tentative patch for Windows but have no way to look into Mac right now. For this specific use of shortcuts frankly I don't mind too much if some weird keys don't work seeing it is an application-side problem that can be fully solved later, I just would like basic support to work for most users. |
So it seems Ctrl+A-Z doesn't work but 0-9 and other non A-Z keys (like /= etc) works fine. Alt seems to work on all keys. Ctrl+Alt-.... behaves the same as Ctrl- only |
Thank you, very useful. Taking it to glfw. |
np :) |
The first point in the list appears to be the most problematic. It looks like I may just need to redesign the IO API, which was desirable anyway, to switch to an event-registration system so e.g. have a May or not need to break compatibility with the user-glue code, very obviously if I can avoid it I'll try very hard to avoid it. I always liked how ImGui took this nice shortcut of taking a sample of input state because it makes initial integration with all sort of libraries easier, but we also need a way out of that. |
Yeah that sounds. Good also something I have been thinking about for a while (sorry for being a bit of topic) is the versioning of the lib. Something I have started to come to like is "Semantic Versioning" http://semver.org I kinda like this approach as it's clear when breaking changes comes that may cause issue. This would of course break the current versioning and it's of course up to you to use or not but may be something to consider. |
Sorry to jump on like this, but would it be possible to allow for more flexibility in the shortcut modifier? On OS X it's quite annoying whenever an application doesn't implement Cmd+A/S/V/N/X shortcuts, and chooses to go with Ctrl+whatever. I already maintain a local patchjob of Just a suggestion, of course. |
How do you think it would work? I don't know enough about Mac experience to design that. My idea is that the user would pass in strings to the api, e.g. "CTRL+S". Then it could be the responsibility of user code to pass in "CMD+S" there? Or an optional/default behavior would be to allow to remap CTRL to CMD by default? (both for key testing and display). Is the Command key a wholly different key? Does typical third API feed it as an modifier? GLFW has GLFW_KEY_LEFT_SUPER/GLFW_KEY_RIGHT_SUPER keys for the Command key and a keyboard modifier GLFW_MOD_SUPER so that's all ok. Is Super a good terminology that can adapt to both Command and Windows key? EDIT SDL2 has KMOD_GUI/KMOD_LGUI/KMOD_RGUI so I'll assume other API are supporting this as a modifiers keys as well. Also - you could post your InputTextEx() mod for reference? |
So the way I have done that in the past is like this: https://github.com/emoon/rocket/blob/master/ogl_editor/src/Menu.c?ts=4#L26 EMGUI_KEY_COMMAND, EMGUI_KEY_CTRL First part is the modifier on Mac and the other is on Windows/Linux. The thing is that while the Windows key isn't used that much on Windows actually for shortcuts inside programs that is certainly the case on Mac. |
Ah, thanks for responding. I've changed a number of things actually:
(3) is done a bit hackily, since I'm not too familiar with the code, I did a move left word, select word right. I know that these changes work on OS X, but since I don't have a windows machine to test on (and I'm not sure the rest of my code would work), I can only guarantee my changes to work... "in theory". It's all compile time checks too, so no performance hit either. I also added a Here's the diff: https://gist.github.com/zhiayang/5764aacd621ebbed6f0b Here's my edited version: https://gist.github.com/zhiayang/27b3f032a054e503484a |
On InputTextEx( ) for Mac: regardless of the shortcut API above we should probably merge those three into master, add the KeyCmd member. They could be a runtime settings (for slightly better build coverage) and set the default differently based on For shortcuts, we could require the user to pass in different strings per os for apps caring about portability in this manner (not all imgui apps will do), but that would make user code more bulky. Or support a syntax like "CTRL|CMD+S", simple but a little weird. Or have a system where by default "CTRL+S" gets translated to "CMD+S" on Mac ("Mac" defined via a runtime flag again) BUT with a mechanism to enforce actual CTRL when desired, aka a different identifier for CTRL on Mac, e.g. "XCTRL". Or Control key stays "CTRL" and we have a different short identifier meaning "CTRL|CMD"., I don't know how to name that.
With R/L variants "LCTRL", "RCTRL". |
I'll have a go at a proper pull request for this, it shouldn't take long. A few things to clarify though:
EDIT: Thanks for sticking around I suppose, hope to hear your opinions on this. Should make for a more fluid and comfortable experience for OS X users, at least. |
Late answer and happy new year :)
I'll have to think a little further about that but for now you can put it in ImGuiIO. Down the line I expect user controllable behaviors for variety of things (eg: slider/drag behavior). Probably just keeping things in IO would just be simpler, but for some behavior it might make sense to Push/Pop the value and then perhaps the ImGuiStyle or another structure may feel more suited. For now let's not worry about that.
I suggest to make a different bool. Eg: InputTextShortcutsUsesSuperKey, InputTextDoubleClickSelectsWord.
Yes, easier to always have in the structure regardless of OS. Let's call it KeySuper because glfw is the cool active thing in town for (with a comment mentioning it maps to Cmd/Windows key). People don't really use the Windows key ar all because it is awkward to use.
Yes, yes.
I don't know either. I wouldn't bother setting them unless it looks trivial. If you don't have access to a Windows machine I'll do some basic tests but not worry if there's a hurdle.
Ideally merged in STB itself with an additional flag. Sean probably won't merge soon but we can apply the patch locally.
Haven't looked yet, sorry. Thanks for the help! |
Instead of using the "&Save" syntax you could use It does require that the user will also need to pass the correct index of which letter to highlight. Because you can't rely on any mapping between them. Then there is no overhead with text processing. |
That would add an extra parameter to every function which would be practically a no-no. |
I'm tempted to say to change the string param to a special struct that includes all the data for the & alt menu (plus the label/ID stuff ofcourse). Which also has a constructor taking a char* so it won't break existing code. Which can then do the bit of parsing needed for the "###ID" stuff and you may as well add the & parsing as well. Also it may be an option to use Though the viability of that struct may depend on how the ImStr experiment #494 turns out. |
On a slightly related note something that would be nice is keyboard navigation of menus (excluding the actual shortcuts) but perhaps that belongs in a separate issue. |
Agree, it would make sense to aim for that support early on (there's a keyboard navigation task and menus could be worked on earlier). Tho up/down without alt+letter may feel incomplete?
|
I was thinking about popups in this case (at least on Mac if you have a popup and press arrow down you get keyboard focus and can navigate it with arrows and then enter to select an item) But for regular menus that would be nice yes. |
So the simple solution to my problem with obtaining translated characters when ALT/CTRL is pressed to use with shortcuts, is instead to include printables like A-Z 0-9 in the ImGuiKey_ enum and work with key events. Pros:
Cons:
|
Is this the right thread to watch for things like cmd-a doing a select-all in a text box instead of ctrl-a on os x? |
…versions of IsKeyPressed(), IsKeyChordPressed(), IsMouseClicked(). (#456) For several reasons those changes makes sense. They are being made because making some of those API public. Only past users of imgui_internal.h with the extra parameters will be affected. Added asserts for valid flags in various functions to detect _some_ misuses, BUT NOT ALL. Amend 4448d97 (#456, #2637, #2620, #2891, #3370, #4828, #5108, #5242, #5641)
…active. Made NavCalcPreferredRefPos() take account for remote activation. (#456) Unsure why filter in ItemHandleShortcut(), will probably find out soon enough.
…ow evaluation to SetShortcutRouting() for now. (#456)
I have pushed a dozen more commits related to this topic, henceforth officially referred to as the Duke-Nukem-Forever™ of GitHub issues. Now added public API: // Inputs Utilities: Shortcut Testing & Routing
// - ImGuiKeyChord = a ImGuiKey + optional ImGuiMod_Alt/ImGuiMod_Ctrl/ImGuiMod_Shift/ImGuiMod_Super.
// ImGuiKey_C // Accepted by functions taking ImGuiKey or ImGuiKeyChord arguments)
// ImGuiMod_Ctrl | ImGuiKey_C // Accepted by functions taking ImGuiKeyChord arguments)
// only ImGuiMod_XXX values are legal to combine with an ImGuiKey. You CANNOT combine two ImGuiKey values.
// - The general idea is that several callers may register interest in a shortcut, and only one owner gets it.
// Parent -> call Shortcut(Ctrl+S) // When Parent is focused, Parent gets the shortcut.
// Child1 -> call Shortcut(Ctrl+S) // When Child1 is focused, Child1 gets the shortcut (Child1 overrides Parent shortcuts)
// Child2 -> no call // When Child2 is focused, Parent gets the shortcut.
// The whole system is order independent, so if Child1 makes its calls before Parent, results will be identical.
// This is an important property as it facilitate working with foreign code or larger codebase.
// - To understand the difference:
// - IsKeyChordPressed() compares mods and call IsKeyPressed() -> function has no side-effect.
// - Shortcut() submits a route, routes are resolved, if it currently can be routed it calls IsKeyChordPressed() -> function has (desirable) side-effects as it can prevents another call from getting the route.
// - Visualize registered routes in 'Metrics/Debugger->Inputs'.
bool Shortcut(ImGuiKeyChord key_chord, ImGuiInputFlags flags = 0);
void SetNextItemShortcut(ImGuiKeyChord key_chord, ImGuiInputFlags flags = 0); // Flags for Shortcut(), SetNextItemShortcut(),
// (and for upcoming extended versions of IsKeyPressed(), IsMouseClicked(), Shortcut(), SetKeyOwner(), SetItemKeyOwner() that are still in imgui_internal.h)
// Don't mistake with ImGuiInputTextFlags! (which is for ImGui::InputText() function)
enum ImGuiInputFlags_
{
ImGuiInputFlags_None = 0,
ImGuiInputFlags_Repeat = 1 << 0, // Enable repeat. Return true on successive repeats. Default for legacy IsKeyPressed(). NOT Default for legacy IsMouseClicked(). MUST BE == 1.
// Flags for Shortcut(), SetNextItemShortcut()
// - Default policy is RouteFocused. Can select only 1 policy among all available.
// - Priorities: GlobalHighest > Focused (if owner is active item) > GlobalOverFocused > Focused (if in focused window) > Global.
ImGuiInputFlags_RouteFocused = 1 << 12, // Focus stack route (default): Accept inputs if window is in focus stack. Deep-most focused window takes inputs. ActiveId takes inputs over deep-most focused window.
ImGuiInputFlags_RouteGlobal = 1 << 13, // Global route (normal priority): unless a focused window or active item registered the route) -> recommended Global priority.
ImGuiInputFlags_RouteGlobalOverFocused = 1 << 14, // Global route (higher priority): unless an active item registered the route, e.g. CTRL+A registered by InputText will take priority over this).
ImGuiInputFlags_RouteGlobalHighest = 1 << 15, // Global route (highest priority): unlikely you need to use that: will interfere with every active items, e.g. CTRL+A registered by InputText will be overridden by this)
ImGuiInputFlags_RouteAlways = 1 << 16, // Do not register route, poll keys directly.
ImGuiInputFlags_RouteUnlessBgFocused = 1 << 17, // Option: global routes will not be applied if underlying background/void is focused (== no Dear ImGui windows are focused). Useful for overlay applications.
ImGuiInputFlags_RouteFromRootWindow = 1 << 18, // Option: route evaluated from the point of view of root window rather than current window.
// Flags for SetNextItemShortcut()
ImGuiInputFlags_Tooltip = 1 << 19, // Automatically display a tooltip when hovering item.
}; Those seemingly innocuous functions have required months/years of yak-shaking and iterations. Much of the inputs system has transitioning to key-owner-awareness since 1.89 (november 2022)... with further iterations and long-time work on input routing (which I swear I must have rewritten 10 times).. untangled key handling (1.87 #4921)... related backend works for translated keys... decent macOS support (#2343 (comment))... remove nav activation/highlight without stealing focus/active id.... mod and keychord design, etc. Not everything is in public API because I'm not sure of all the stuff, but those two are rather useful already: Some typical use would be: ImGui::SetNextItemShortcut(ImGuiMod_Ctrl | ImGuiKey_S);
ImGui::Button("Save"); By default it use the focus route, so here Ctrl+S will trigger button if parent window is focused, or one of its child. But also note, e.g. ImGui::SetNextItemShortcut(ImGuiMod_Ctrl | ImGuiKey_S, ImGuiInputFlags_RouteGlobal);
ImGui::Button("Save"); May be triggered when other windows are focused. I currently offer an opt-in mouse-only (intentional) auto tooltip: ImGui::SetNextItemShortcut(ImGuiMod_Ctrl | ImGuiKey_S, ImGuiInputFlags_Tooltip);
ImGui::Button("Save"); But I would consider making it opt-out once I am done with working on tooltip priority system (#7570) Amusingly, because this is the Duke-Nukem-Forever™ of GitHub issues, in spite of much progress, many elements discussed at the very top of this thread are still undone:
IMPORTANT! IF and only IF you have been using key-owner-aware API or Shortcut() prior to today, please be mindful of the following badly breaking changes: |
I have not used the shortcut interface much so far and did not look at its implementation yet. The newly added public interface looks promising, but could use a few more policies. So just as a suggestion...
If they were to be added, Moving the mouse to a window where you want to send the next keypress without an additional click is pretty efficient. Some applications like f.e. Maya use this shortcut policy and (some of) our artists like it a lot. When I get around to overhauling the shortcut system in our tools someday, I will either implement this policy in a custom system or maybe use the new public one if it supports hover routes by then. There's no need to implement it just for me, but I think this might be useful for other users too and it should integrate nicely with the other policies. Another shortcut feature that I find to be very useful would be keychord sequences. Visual Studio f.e. has sequences like I don't know if keychord sequences whould already be doable with some clever use of this interface or if they are feasible for an immediate mode interface at all, I will probably need to experiment a bit. So no concrete suggestion yet, just maybe something to keep in mind for the future. |
It seems possible to implement by mimicking some of the logic.
I would like to implement that indeed, the idea would be to pack in upper bits of ImGuiKeyChord. I think we already manipulate enough state that it should be feasible, but I presume it won't be trivial. |
…gs_RouteGlobalOverActive, made ImGuiInputFlags_RouteGlobalOverFocused and ImGuiInputFlags_RouteGlobalOverActive flags. (#456)
I am concerned about priorities between Hovered and Focused. Regardless of being lower or higher, mixing both types is going to be very misleading. It may requires app hygiene to not mix both together too much. I have found a solution to slightly decrease the perceived complexity of this. Reducing the number of route types (only 1 of those can be choosen) ImGuiInputFlags_RouteActive = 1 << 10, // Route to active item only.
ImGuiInputFlags_RouteFocused = 1 << 11, // Route to windows in the focus stack (DEFAULT). Deep-most focused window takes inputs. Active item takes inputs over deep-most focused window.
ImGuiInputFlags_RouteGlobal = 1 << 12, // Global route (unless a focused window or active item registered the route).
ImGuiInputFlags_RouteAlways = 1 << 13, // Do not register route, poll keys directly. And making others flags which may be combined: ImGuiInputFlags_RouteGlobalOverFocused = 1 << 14, // Option: global route, higher priority than focused route (unless active item in focused route). automatically sets ImGuiInputFlags_RouteGlobal.
ImGuiInputFlags_RouteGlobalOverActive = 1 << 15, // Option: global route, higher priority than active item. Unlikely you need to use that: will interfere with every active items, e.g. CTRL+A registered by InputText will be overridden by this. May not be fully honored as user/internal code is likely to always assume they can access keys when active. Automatically sets ImGuiInputFlags_RouteGlobal.
ImGuiInputFlags_RouteUnlessBgFocused = 1 << 16, // Option: global route, will not be applied if underlying background/void is focused (== no Dear ImGui windows are focused). Useful for overlay applications. Notice how ImGuiInputFlags_RouteGlobalHighest got renamed to ImGuiInputFlags_RouteGlobalOverActive: with this subtlety of making them combinable flags, it doesn't require the reader to understand by heart the global/respective priority order. It also means if we want to add an Hovered route we will only need to add |
I renamed those: // - Routing options
ImGuiInputFlags_RouteOverFocused = 1 << 14, // Option: global route: higher priority than focused route (unless active item in focused route).
ImGuiInputFlags_RouteOverActive = 1 << 15, // Option: global route: higher priority than active item. Unlikely you need to use that: will interfere with every active items, e.g. CTRL+A registered by InputText will be overridden by this. May not be fully honored as user/internal code is likely to always assume they can access keys when active.
ImGuiInputFlags_RouteUnlessBgFocused = 1 << 16, // Option: global route: will not be applied if underlying background/void is focused (== no Dear ImGui windows are focused). Useful for overlay applications. Because I think we would to use them with hypothetical Hovered route. |
…tFlags_RouteOverFocused, ImGuiInputFlags_RouteGlobalOverActive -> ImGuiInputFlags_RouteOverActive in previsiion of using them with a Hovered route. (#456)
I'm not quite sure I can follow the current progress of this issue, so I'm just going to simply ask: does ImGUI support keyboard shortcuts for MenuItems which run the same code as if they were clicked? (regardless of whether the menu is open or not) |
No, it doesn't do that now. Currently the shortcut field of MenuItem() is displayed for not used for anything else. However, I do have the working proof of concept for it (~30 lines patch), and it was fully in my mind with the recent work done. But it's not fully in as it requires quite some menu API design (opt-in, fallback for recursion, transitioning to use ImGuiKeyChord to reduce amount of parsing which involve transition to multiple signatures). |
Specifically, my big remaining design issue is the following: (1) I would like to change BeginMenu(const char* label, bool enabled = true); to BeginMenu(const char* label, ImGuiMenuFlags flags = 0);`, with flags, e.g. // Flags for ImGui::BeginMenu()
enum ImGuiMenuFlags_
{
ImGuiMenuFlags_None = 0,
ImGuiMenuFlags_Disabled = 1 << 1, // We skip using (1 << 0) to detect ABI/bindings possible issue with old BeginMenu(const char*, bool)
ImGuiMenuFlags_ProcessShortcuts = 1 << 2,
}; That part is not so problematic and has been a common transition pattern, the signatures can co-exist, and also the (2) However, the equivalent change for bool MenuItem(const char* label, const char* shortcut = NULL, bool selected = false, bool enabled = true); // return true when activated.
bool MenuItem(const char* label, const char* shortcut, bool* p_selected, bool enabled = true); // return true when activated + toggle (*p_selected) if p_selected != NULL (2.A) It would make sense to change (2.B) Out of experience So I need to factor those things in and come up with a design that senseful, forward facing, while not breaking backward compatibility in an horrible manner. |
Writing down notes about implementing a shortcut system. I have started using menus more extensively in my own applications, and the lack of support for shortcuts has become a little annoying.
Without shortcuts the user is required to duplicate code to handle menu items and shortcuts for a same action. Consider that MenuItem() can takes both a "checked" and "enabled" parameter which may need to be fetch/computed from your application state, duplicating that code can be pretty annoying and ImGui strive to reduce redundancy so it's quite a flaw to have to do that.
In the old Menus API #126 thread I said we'd need;
And
So here is a rough list of what I think we need. Unfortunately some of those will need the user to update their ImGui integration to provide the necessary inputs, but there won't be any breakage.
PART A, for regular shortcuts (typically global shortcuts, but they can be local to a window)
EDIT Not needed!
- [ ] We are going to need translated characters, aka the "A" in "CTRL+A" or "ALT+A" is a translated character. So the end-user application needs to feed those inputs probably via io.AddInputCharacter() and this isn't really a problem for ImGui to solve. However, and that's very surprising, retrieve this information under Windows is NOT straightforward. The WM_CHAR message isn't sent when ALT or CTRL are pressed. WM_SYSCHAR is only sent when ALT is pressed. No CHAR messages are sent when CTRL is pressed. By adding this in a Windows message handler I seem to be able to retrieve those characters.It is a little scary but appears to work. End-user would need a similar mechanism which is rather annoying. For Windows messages user can copy demo code if they are pointed to it. GLFW has been patched in the 3.1 branch to support the ALT key but not the CTRL key yet (would be nice if it does so), what that means is that support for those inputs won't be widespread in most applications soon and it is only likely to be widely available in GLFW when 3.2 ships (also means that support for CTRL key would better be pushed in GLFW before they release 3.2!). Or user can freely do their own cheap conversions if they don't care about funky localisation things.
I'd be curious to know whether GLFW 3.1 gives you character inputs in ImGui_ImplGlfw_CharCallback when ALT or CTRL are pressed on MacOS, Linux, etc. See following rely for instructions on how to perform the test. If you can do a test let me know which version of GLFW you are using. Thanks!
PART B (for & ALT-style local shortcuts, lesser priority)
if (ImGui::Button("Refresh") || ImGui::IsShortcutPressed("F5") { .. }
.That's it for now. Those are merely notes for myself. If there are
The text was updated successfully, but these errors were encountered: