Browse Source

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
ocornut 2 years ago
parent
commit
3532ed1621
5 changed files with 10 additions and 9 deletions
  1. 2 0
      docs/CHANGELOG.txt
  2. 2 2
      imgui.cpp
  3. 1 1
      imgui.h
  4. 1 0
      imgui_internal.h
  5. 4 6
      imgui_widgets.cpp

+ 2 - 0
docs/CHANGELOG.txt

@@ -135,6 +135,8 @@ Other Changes:
   towards a sub-menu. (#2517, #5614). [@rokups]
   towards a sub-menu. (#2517, #5614). [@rokups]
 - Menus: Fixed gaps in closing logic which would make child-menu erroneously close when crossing
 - 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)
   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: 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: 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.
 - Nav: Fixed an issue opening a menu with Right key from a non-menu window.

+ 2 - 2
imgui.cpp

@@ -3651,7 +3651,8 @@ bool ImGui::ItemHoverable(const ImRect& bb, ImGuiID id)
         return false;
         return false;
 
 
     // Done with rectangle culling so we can perform heavier checks now.
     // 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;
         g.HoveredIdDisabled = true;
         return false;
         return false;
@@ -3663,7 +3664,6 @@ bool ImGui::ItemHoverable(const ImRect& bb, ImGuiID id)
         SetHoveredID(id);
         SetHoveredID(id);
 
 
     // When disabled we'll return false but still set HoveredId
     // 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)
     if (item_flags & ImGuiItemFlags_Disabled)
     {
     {
         // Release active id if turning disabled
         // Release active id if turning disabled

+ 1 - 1
imgui.h

@@ -23,7 +23,7 @@
 // Library Version
 // Library Version
 // (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM > 12345')
 // (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               "1.89 WIP"
-#define IMGUI_VERSION_NUM           18825
+#define IMGUI_VERSION_NUM           18826
 #define IMGUI_HAS_TABLE
 #define IMGUI_HAS_TABLE
 
 
 /*
 /*

+ 1 - 0
imgui_internal.h

@@ -775,6 +775,7 @@ enum ImGuiItemFlags_
     ImGuiItemFlags_SelectableDontClosePopup = 1 << 5,  // false     // Disable MenuItem/Selectable() automatically closing their popup window
     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_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_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
     // 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.
     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.

+ 4 - 6
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)
     // 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.
     // This is only done for items for the menu set and not the full parent window.
     const bool menuset_is_open = IsRootOfOpenMenuSet();
     const bool menuset_is_open = IsRootOfOpenMenuSet();
-    ImGuiWindow* backed_nav_window = g.NavWindow;
     if (menuset_is_open)
     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,
     // 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().
     // 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;
     const bool hovered = (g.HoveredId == id) && enabled && !g.NavDisableMouseHover;
     if (menuset_is_open)
     if (menuset_is_open)
-        g.NavWindow = backed_nav_window;
+        PopItemFlag();
 
 
     bool want_open = false;
     bool want_open = false;
     bool want_close = 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.
     // See BeginMenuEx() for comments about this.
     const bool menuset_is_open = IsRootOfOpenMenuSet();
     const bool menuset_is_open = IsRootOfOpenMenuSet();
-    ImGuiWindow* backed_nav_window = g.NavWindow;
     if (menuset_is_open)
     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),
     // 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.
     // 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();
         EndDisabled();
     PopID();
     PopID();
     if (menuset_is_open)
     if (menuset_is_open)
-        g.NavWindow = backed_nav_window;
+        PopItemFlag();
 
 
     return pressed;
     return pressed;
 }
 }