From 900b29055c5da1546b7d68ad52e0e4e61facb701 Mon Sep 17 00:00:00 2001 From: ocornut Date: Wed, 22 May 2024 13:59:07 +0200 Subject: [PATCH] Internals, Inputs: *Breaking* Swapped parameter order of Shortcut(). (#456) Amend 4448d975 (#456, #2637, #2620, #2891, #3370, #4828, #5108, #5242, #5641) --- docs/CHANGELOG.txt | 11 ++++++++--- imgui.cpp | 31 ++++++++++++++++++------------- imgui_internal.h | 4 ++-- imgui_widgets.cpp | 20 ++++++++++---------- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 2aa1bdc1f..4b69c9259 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -53,11 +53,16 @@ Breaking changes: - Backends: SDL_Renderer2/SDL_Renderer3: and ImGui_ImplSDLRenderer2_RenderDrawData() and ImGui_ImplSDLRenderer3_RenderDrawData() now takes a SDL_Renderer* parameter. This was previously overlooked from the API but it will facilitate eventual support for multi-viewports. -- Internals, Inputs: (not public nor documented yet, but disclosing breaking changes +- Inputs: (not public nor documented yet, but disclosing breaking changes because I expect a few advanced users caught on owner-aware inputs system): - - Renamed ImGuiKeyOwner_None to ImGuiKeyOwner_NoOwner, to make use more explicit and + - (Internals) Renamed ImGuiKeyOwner_None to ImGuiKeyOwner_NoOwner, to make use more explicit and reduce confusion with the default it is a non-zero value and cannot be the default value. - + - (Internals) Shortcut(), SetShortcutRouting(): swapped last two parameters order in function signatures: + Before: Shortcut(ImGuiKeyChord key_chord, ImGuiID owner_id = 0, ImGuiInputFlags flags = 0); + After: Shortcut(ImGuiKeyChord key_chord, ImGuiInputFlags flags = 0, ImGuiID owner_id = 0); + - 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. Other changes: diff --git a/imgui.cpp b/imgui.cpp index 985e00c3e..09faa73ab 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -143,7 +143,7 @@ CODE - CTRL+X, CTRL+C, CTRL+V: Use OS clipboard. - CTRL+Z, CTRL+Y: Undo, Redo. - ESCAPE: Revert text to its original value. - - On OSX, controls are automatically adjusted to match standard OSX text editing shortcuts and behaviors. + - On OSX, controls are automatically adjusted to match standard OSX text editing 2ts and behaviors. - KEYBOARD CONTROLS - Basic: @@ -430,7 +430,12 @@ CODE When you are not sure about an old symbol or function name, try using the Search/Find function of your IDE to look for comments or references in all imgui files. You can read releases logs https://github.com/ocornut/imgui/releases for more details. - - 2024/05/22 (1.90.7) - inputs internals: renamed ImGuiKeyOwner_None to ImGuiKeyOwner_NoOwner, to make use more explicit and reduce confusion with the default it is a non-zero value and cannot be the default value (never made public, but disclosing as I expect a few users caught on owner-aware inputs). + - 2024/05/22 (1.90.7) - inputs (internals): renamed ImGuiKeyOwner_None to ImGuiKeyOwner_NoOwner, to make use more explicit and reduce confusion with the default it is a non-zero value and cannot be the default value (never made public, but disclosing as I expect a few users caught on owner-aware inputs). + - inputs (internals): Shortcut(), SetShortcutRouting(): swapped last two parameters order in function signatures: + - old: Shortcut(ImGuiKeyChord key_chord, ImGuiID owner_id = 0, ImGuiInputFlags flags = 0); + - new: Shortcut(ImGuiKeyChord key_chord, ImGuiInputFlags flags = 0, ImGuiID owner_id = 0); + for various 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. - 2024/05/16 (1.90.7) - inputs: on macOS X, Cmd and Ctrl keys are now automatically swapped by io.AddKeyEvent() as this naturally align with how macOS X uses those keys. - it shouldn't really affect you unless you had custom shortcut swapping in place for macOS X apps. - removed ImGuiMod_Shortcut which was previously dynamically remapping to Ctrl or Cmd/Super. It is now unnecessary to specific cross-platform idiomatic shortcuts. (#2343, #4084, #5923, #456) @@ -8641,7 +8646,7 @@ static bool IsKeyChordPotentiallyCharInput(ImGuiKeyChord key_chord) // - Routes and key ownership are attributed at the beginning of next frame based on best score and mod state. // (Conceptually this does a "Submit for next frame" + "Test for current frame". // As such, it could be called TrySetXXX or SubmitXXX, or the Submit and Test operations should be separate.) -bool ImGui::SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiInputFlags flags) +bool ImGui::SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiInputFlags flags, ImGuiID owner_id) { ImGuiContext& g = *GImGui; if ((flags & ImGuiInputFlags_RouteTypeMask_) == 0) @@ -8664,7 +8669,7 @@ bool ImGui::SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiI // Note how ImGuiInputFlags_RouteAlways won't set routing and thus won't set owner. May want to rework this? if (flags & ImGuiInputFlags_RouteAlways) { - IMGUI_DEBUG_LOG_INPUTROUTING("SetShortcutRouting(%s, owner_id=0x%08X, flags=%04X) -> always, no register\n", GetKeyChordName(key_chord), owner_id, flags); + IMGUI_DEBUG_LOG_INPUTROUTING("SetShortcutRouting(%s, flags=%04X, owner_id=0x%08X) -> always, no register\n", GetKeyChordName(key_chord), flags, owner_id); return true; } @@ -8678,7 +8683,7 @@ bool ImGui::SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiI // (We cannot filter based on io.InputQueueCharacters[] contents because of trickling and key<>chars submission order are undefined) if ((flags & ImGuiInputFlags_RouteFocused) && g.IO.WantTextInput && IsKeyChordPotentiallyCharInput(key_chord)) { - IMGUI_DEBUG_LOG_INPUTROUTING("SetShortcutRouting(%s, owner_id=0x%08X, flags=%04X) -> filtered as potential char input\n", GetKeyChordName(key_chord), owner_id, flags); + IMGUI_DEBUG_LOG_INPUTROUTING("SetShortcutRouting(%s, flags=%04X, owner_id=0x%08X) -> filtered as potential char input\n", GetKeyChordName(key_chord), flags, owner_id); return false; } @@ -8695,7 +8700,7 @@ bool ImGui::SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiI // FIXME-SHORTCUT: A way to configure the location/focus-scope to test would render this more flexible. const int score = CalcRoutingScore(g.CurrentFocusScopeId, owner_id, flags); - IMGUI_DEBUG_LOG_INPUTROUTING("SetShortcutRouting(%s, owner_id=0x%08X, flags=%04X) -> score %d\n", GetKeyChordName(key_chord), owner_id, flags, score); + IMGUI_DEBUG_LOG_INPUTROUTING("SetShortcutRouting(%s, flags=%04X, owner_id=0x%08X) -> score %d\n", GetKeyChordName(key_chord), flags, owner_id, score); if (score == 255) return false; @@ -9671,10 +9676,10 @@ void ImGui::SetNextItemShortcut(ImGuiKeyChord key_chord) g.NextItemData.Shortcut = key_chord; } -bool ImGui::Shortcut(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiInputFlags flags) +bool ImGui::Shortcut(ImGuiKeyChord key_chord, ImGuiInputFlags flags, ImGuiID owner_id) { //ImGuiContext& g = *GImGui; - //IMGUI_DEBUG_LOG("Shortcut(%s, owner_id=0x%08X, flags=%X)\n", GetKeyChordName(key_chord, g.TempBuffer.Data, g.TempBuffer.Size), owner_id, flags); + //IMGUI_DEBUG_LOG("Shortcut(%s, flags=%X, owner_id=0x%08X)\n", GetKeyChordName(key_chord, g.TempBuffer.Data, g.TempBuffer.Size), flags, owner_id); // When using (owner_id == 0/Any): SetShortcutRouting() will use CurrentFocusScopeId and filter with this, so IsKeyPressed() is fine with he 0/Any. if ((flags & ImGuiInputFlags_RouteTypeMask_) == 0) @@ -9686,7 +9691,7 @@ bool ImGui::Shortcut(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiInputFlags owner_id = GetRoutingIdFromOwnerId(owner_id); // Submit route - if (!SetShortcutRouting(key_chord, owner_id, flags)) + if (!SetShortcutRouting(key_chord, flags, owner_id)) return false; // Default repeat behavior for Shortcut() @@ -10001,7 +10006,7 @@ static void ItemHandleShortcut(ImGuiID id) { // FIXME: Generalize Activation queue? ImGuiContext& g = *GImGui; - if (ImGui::Shortcut(g.NextItemData.Shortcut, id, ImGuiInputFlags_None) && g.NavActivateId == 0) + if (ImGui::Shortcut(g.NextItemData.Shortcut, ImGuiInputFlags_None, id) && g.NavActivateId == 0) { g.NavActivateId = id; // Will effectively disable clipping. g.NavActivateFlags = ImGuiActivateFlags_PreferInput | ImGuiActivateFlags_FromShortcut; @@ -12795,8 +12800,8 @@ static void ImGui::NavUpdateWindowing() const ImGuiID owner_id = ImHashStr("###NavUpdateWindowing"); const bool nav_gamepad_active = (io.ConfigFlags & ImGuiConfigFlags_NavEnableGamepad) != 0 && (io.BackendFlags & ImGuiBackendFlags_HasGamepad) != 0; const bool nav_keyboard_active = (io.ConfigFlags & ImGuiConfigFlags_NavEnableKeyboard) != 0; - const bool keyboard_next_window = allow_windowing && g.ConfigNavWindowingKeyNext && Shortcut(g.ConfigNavWindowingKeyNext, owner_id, ImGuiInputFlags_Repeat | ImGuiInputFlags_RouteAlways); - const bool keyboard_prev_window = allow_windowing && g.ConfigNavWindowingKeyPrev && Shortcut(g.ConfigNavWindowingKeyPrev, owner_id, ImGuiInputFlags_Repeat | ImGuiInputFlags_RouteAlways); + const bool keyboard_next_window = allow_windowing && g.ConfigNavWindowingKeyNext && Shortcut(g.ConfigNavWindowingKeyNext, ImGuiInputFlags_Repeat | ImGuiInputFlags_RouteAlways, owner_id); + const bool keyboard_prev_window = allow_windowing && g.ConfigNavWindowingKeyPrev && Shortcut(g.ConfigNavWindowingKeyPrev, ImGuiInputFlags_Repeat | ImGuiInputFlags_RouteAlways, owner_id); const bool start_windowing_with_gamepad = allow_windowing && nav_gamepad_active && !g.NavWindowingTarget && IsKeyPressed(ImGuiKey_NavGamepadMenu, 0, ImGuiInputFlags_None); const bool start_windowing_with_keyboard = allow_windowing && !g.NavWindowingTarget && (keyboard_next_window || keyboard_prev_window); // Note: enabled even without NavEnableKeyboard! if (start_windowing_with_gamepad || start_windowing_with_keyboard) @@ -15844,7 +15849,7 @@ void ImGui::ShowIDStackToolWindow(bool* p_open) Checkbox("Ctrl+C: copy path to clipboard", &tool->CopyToClipboardOnCtrlC); SameLine(); TextColored((time_since_copy >= 0.0f && time_since_copy < 0.75f && ImFmod(time_since_copy, 0.25f) < 0.25f * 0.5f) ? ImVec4(1.f, 1.f, 0.3f, 1.f) : ImVec4(), "*COPIED*"); - if (tool->CopyToClipboardOnCtrlC && Shortcut(ImGuiMod_Ctrl | ImGuiKey_C, 0, ImGuiInputFlags_RouteGlobalOverFocused)) + if (tool->CopyToClipboardOnCtrlC && Shortcut(ImGuiMod_Ctrl | ImGuiKey_C, ImGuiInputFlags_RouteGlobalOverFocused)) { tool->CopyToClipboardLastTime = (float)g.Time; char* p = g.TempBuffer.Data; diff --git a/imgui_internal.h b/imgui_internal.h index 6913e0873..6246867dc 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -3291,8 +3291,8 @@ namespace ImGui // - Shortcut() submits a route then if currently can be routed calls IsKeyChordPressed() -> function has (desirable) side-effects. // - Use Tools->Metrics/Debugger->Inputs to view input routes. IMGUI_API void SetNextItemShortcut(ImGuiKeyChord key_chord); - IMGUI_API bool Shortcut(ImGuiKeyChord key_chord, ImGuiID owner_id = 0, ImGuiInputFlags flags = 0); - IMGUI_API bool SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiID owner_id, ImGuiInputFlags flags = 0); // owner_id needs to be explicit and cannot be 0 + IMGUI_API bool Shortcut(ImGuiKeyChord key_chord, ImGuiInputFlags flags = 0, ImGuiID owner_id = 0); + IMGUI_API bool SetShortcutRouting(ImGuiKeyChord key_chord, ImGuiInputFlags flags, ImGuiID owner_id); // owner_id needs to be explicit and cannot be 0 IMGUI_API bool TestShortcutRouting(ImGuiKeyChord key_chord, ImGuiID owner_id); IMGUI_API ImGuiKeyRoutingData* GetShortcutRoutingData(ImGuiKeyChord key_chord); diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index bf47709c6..d25dd372f 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -4442,7 +4442,7 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ // (For Tab and Enter: Win32/SFML/Allegro are sending both keys and chars, GLFW and SDL are only sending keys. For Space they all send all threes) if ((flags & ImGuiInputTextFlags_AllowTabInput) && !is_readonly) { - if (Shortcut(ImGuiKey_Tab, id, ImGuiInputFlags_Repeat)) + if (Shortcut(ImGuiKey_Tab, ImGuiInputFlags_Repeat, id)) { unsigned int c = '\t'; // Insert TAB if (InputTextFilterCharacter(&g, &c, flags, callback, callback_user_data)) @@ -4450,7 +4450,7 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ } // FIXME: Implement Shift+Tab /* - if (Shortcut(ImGuiKey_Tab | ImGuiMod_Shift, id, ImGuiInputFlags_Repeat)) + if (Shortcut(ImGuiKey_Tab | ImGuiMod_Shift, ImGuiInputFlags_Repeat, id)) { } */ @@ -4493,18 +4493,18 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ // Using Shortcut() with ImGuiInputFlags_RouteFocused (default policy) to allow routing operations for other code (e.g. calling window trying to use CTRL+A and CTRL+B: formet would be handled by InputText) // Otherwise we could simply assume that we own the keys as we are active. const ImGuiInputFlags f_repeat = ImGuiInputFlags_Repeat; - const bool is_cut = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_X, id, f_repeat) || Shortcut(ImGuiMod_Shift | ImGuiKey_Delete, id, f_repeat)) && !is_readonly && !is_password && (!is_multiline || state->HasSelection()); - const bool is_copy = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_C, id) || Shortcut(ImGuiMod_Ctrl | ImGuiKey_Insert, id)) && !is_password && (!is_multiline || state->HasSelection()); - const bool is_paste = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_V, id, f_repeat) || Shortcut(ImGuiMod_Shift | ImGuiKey_Insert, id, f_repeat)) && !is_readonly; - const bool is_undo = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z, id, f_repeat)) && !is_readonly && is_undoable; - const bool is_redo = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y, id, f_repeat) || (is_osx && Shortcut(ImGuiMod_Ctrl | ImGuiMod_Shift | ImGuiKey_Z, id, f_repeat))) && !is_readonly && is_undoable; - const bool is_select_all = Shortcut(ImGuiMod_Ctrl | ImGuiKey_A, id); + const bool is_cut = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_X, f_repeat, id) || Shortcut(ImGuiMod_Shift | ImGuiKey_Delete, f_repeat, id)) && !is_readonly && !is_password && (!is_multiline || state->HasSelection()); + const bool is_copy = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_C, 0, id) || Shortcut(ImGuiMod_Ctrl | ImGuiKey_Insert, 0, id)) && !is_password && (!is_multiline || state->HasSelection()); + const bool is_paste = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_V, f_repeat, id) || Shortcut(ImGuiMod_Shift | ImGuiKey_Insert, f_repeat, id)) && !is_readonly; + const bool is_undo = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_Z, f_repeat, id)) && !is_readonly && is_undoable; + const bool is_redo = (Shortcut(ImGuiMod_Ctrl | ImGuiKey_Y, f_repeat, id) || (is_osx && Shortcut(ImGuiMod_Ctrl | ImGuiMod_Shift | ImGuiKey_Z, f_repeat, id))) && !is_readonly && is_undoable; + const bool is_select_all = Shortcut(ImGuiMod_Ctrl | ImGuiKey_A, 0, id); // We allow validate/cancel with Nav source (gamepad) to makes it easier to undo an accidental NavInput press with no keyboard wired, but otherwise it isn't very useful. const bool nav_gamepad_active = (io.ConfigFlags & ImGuiConfigFlags_NavEnableGamepad) != 0 && (io.BackendFlags & ImGuiBackendFlags_HasGamepad) != 0; const bool is_enter_pressed = IsKeyPressed(ImGuiKey_Enter, true) || IsKeyPressed(ImGuiKey_KeypadEnter, true); const bool is_gamepad_validate = nav_gamepad_active && (IsKeyPressed(ImGuiKey_NavGamepadActivate, false) || IsKeyPressed(ImGuiKey_NavGamepadInput, false)); - const bool is_cancel = Shortcut(ImGuiKey_Escape, id, f_repeat) || (nav_gamepad_active && Shortcut(ImGuiKey_NavGamepadCancel, id, f_repeat)); + const bool is_cancel = Shortcut(ImGuiKey_Escape, f_repeat, id) || (nav_gamepad_active && Shortcut(ImGuiKey_NavGamepadCancel, f_repeat, id)); // FIXME: Should use more Shortcut() and reduce IsKeyPressed()+SetKeyOwner(), but requires modifiers combination to be taken account of. // FIXME-OSX: Missing support for Alt(option)+Right/Left = go to end of line, or next line if already in end of line. @@ -4702,7 +4702,7 @@ bool ImGui::InputTextEx(const char* label, const char* hint, char* buf, int buf_ // The reason we specify the usage semantic (Completion/History) is that Completion needs to disable keyboard TABBING at the moment. ImGuiInputTextFlags event_flag = 0; ImGuiKey event_key = ImGuiKey_None; - if ((flags & ImGuiInputTextFlags_CallbackCompletion) != 0 && Shortcut(ImGuiKey_Tab, id)) + if ((flags & ImGuiInputTextFlags_CallbackCompletion) != 0 && Shortcut(ImGuiKey_Tab, 0, id)) { event_flag = ImGuiInputTextFlags_CallbackCompletion; event_key = ImGuiKey_Tab;