Skip to content

Commit

Permalink
Internals, Inputs: *Breaking* Swapped parameter order of Shortcut(). (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ocornut committed May 23, 2024
1 parent 55748cd commit 900b290
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 28 deletions.
11 changes: 8 additions & 3 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
31 changes: 18 additions & 13 deletions imgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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;

Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
20 changes: 10 additions & 10 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4442,15 +4442,15 @@ 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))
state->OnKeyPressed((int)c);
}
// FIXME: Implement Shift+Tab
/*
if (Shortcut(ImGuiKey_Tab | ImGuiMod_Shift, id, ImGuiInputFlags_Repeat))
if (Shortcut(ImGuiKey_Tab | ImGuiMod_Shift, ImGuiInputFlags_Repeat, id))
{
}
*/
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 900b290

Please sign in to comment.