From 3532ed1621171647ce4a969be40fd69c58e47b75 Mon Sep 17 00:00:00 2001 From: ocornut Date: Thu, 29 Sep 2022 18:07:35 +0200 Subject: [PATCH] Menus, Nav: Fixed keyboard/gamepad navigation occasionally erroneously landing on menu-item in parent when the parent is not a popup. (#5730) Replace BeginMenu/MenuItem swapping g.NavWindow with a more adequate ImGuiItemFlags_NoWindowHoverableCheck. Expecting more subtle issues to stem from this. Note that NoWindowHoverableCheck is not supported by IsItemHovered() but then IsItemHovered() on BeginMenu() never worked: fix should be easy in BeginMenu() + add test is IsItemHovered(), will do later --- docs/CHANGELOG.txt | 2 ++ imgui.cpp | 4 ++-- imgui.h | 2 +- imgui_internal.h | 1 + imgui_widgets.cpp | 10 ++++------ 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 9bb793476b55..b1a28251a64d 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -135,6 +135,8 @@ Other Changes: towards a sub-menu. (#2517, #5614). [@rokups] - Menus: Fixed gaps in closing logic which would make child-menu erroneously close when crossing the gap between a menu item inside a window and a child-menu in a secondary viewport. (#5614) +- Menus, Nav: Fixed keyboard/gamepad navigation occasionally erroneously landing on menu-item in + parent when the parent is not a popup. (#5730) - Nav: Fixed moving/resizing window with gamepad or keyboard when running at very high framerate. - Nav: Pressing Space/GamepadFaceDown on a repeating button uses the same repeating rate as a mouse hold. - Nav: Fixed an issue opening a menu with Right key from a non-menu window. diff --git a/imgui.cpp b/imgui.cpp index 8867192abab5..69569a91484b 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -3651,7 +3651,8 @@ bool ImGui::ItemHoverable(const ImRect& bb, ImGuiID id) return false; // Done with rectangle culling so we can perform heavier checks now. - if (!IsWindowContentHoverable(window, ImGuiHoveredFlags_None)) + ImGuiItemFlags item_flags = (g.LastItemData.ID == id ? g.LastItemData.InFlags : g.CurrentItemFlags); + if (!(item_flags & ImGuiItemFlags_NoWindowHoverableCheck) && !IsWindowContentHoverable(window, ImGuiHoveredFlags_None)) { g.HoveredIdDisabled = true; return false; @@ -3663,7 +3664,6 @@ bool ImGui::ItemHoverable(const ImRect& bb, ImGuiID id) SetHoveredID(id); // When disabled we'll return false but still set HoveredId - ImGuiItemFlags item_flags = (g.LastItemData.ID == id ? g.LastItemData.InFlags : g.CurrentItemFlags); if (item_flags & ImGuiItemFlags_Disabled) { // Release active id if turning disabled diff --git a/imgui.h b/imgui.h index 7a3fab7ef0de..27e97e64846b 100644 --- a/imgui.h +++ b/imgui.h @@ -23,7 +23,7 @@ // Library Version // (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM > 12345') #define IMGUI_VERSION "1.89 WIP" -#define IMGUI_VERSION_NUM 18825 +#define IMGUI_VERSION_NUM 18826 #define IMGUI_HAS_TABLE /* diff --git a/imgui_internal.h b/imgui_internal.h index a866fb8f6010..bae32248bd84 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -775,6 +775,7 @@ enum ImGuiItemFlags_ ImGuiItemFlags_SelectableDontClosePopup = 1 << 5, // false // Disable MenuItem/Selectable() automatically closing their popup window ImGuiItemFlags_MixedValue = 1 << 6, // false // [BETA] Represent a mixed/indeterminate value, generally multi-selection where values differ. Currently only supported by Checkbox() (later should support all sorts of widgets) ImGuiItemFlags_ReadOnly = 1 << 7, // false // [ALPHA] Allow hovering interactions but underlying value is not changed. + ImGuiItemFlags_NoWindowHoverableCheck = 1 << 8, // false // Disable hoverable check in ItemHoverable() // Controlled by widget code ImGuiItemFlags_Inputable = 1 << 10, // false // [WIP] Auto-activate input mode when tab focused. Currently only used and supported by a few items before it becomes a generic feature. diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 049df88fa960..1c19eca1b58a 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -7020,9 +7020,8 @@ bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled) // Odd hack to allow hovering across menus of a same menu-set (otherwise we wouldn't be able to hover parent without always being a Child window) // This is only done for items for the menu set and not the full parent window. const bool menuset_is_open = IsRootOfOpenMenuSet(); - ImGuiWindow* backed_nav_window = g.NavWindow; if (menuset_is_open) - g.NavWindow = window; + PushItemFlag(ImGuiItemFlags_NoWindowHoverableCheck, true); // The reference position stored in popup_pos will be used by Begin() to find a suitable position for the child menu, // However the final position is going to be different! It is chosen by FindBestWindowPosForPopup(). @@ -7071,7 +7070,7 @@ bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled) const bool hovered = (g.HoveredId == id) && enabled && !g.NavDisableMouseHover; if (menuset_is_open) - g.NavWindow = backed_nav_window; + PopItemFlag(); bool want_open = false; bool want_close = false; @@ -7207,9 +7206,8 @@ bool ImGui::MenuItemEx(const char* label, const char* icon, const char* shortcut // See BeginMenuEx() for comments about this. const bool menuset_is_open = IsRootOfOpenMenuSet(); - ImGuiWindow* backed_nav_window = g.NavWindow; if (menuset_is_open) - g.NavWindow = window; + PushItemFlag(ImGuiItemFlags_NoWindowHoverableCheck, true); // We've been using the equivalent of ImGuiSelectableFlags_SetNavIdOnHover on all Selectable() since early Nav system days (commit 43ee5d73), // but I am unsure whether this should be kept at all. For now moved it to be an opt-in feature used by menus only. @@ -7261,7 +7259,7 @@ bool ImGui::MenuItemEx(const char* label, const char* icon, const char* shortcut EndDisabled(); PopID(); if (menuset_is_open) - g.NavWindow = backed_nav_window; + PopItemFlag(); return pressed; }