Skip to content

Commit

Permalink
Menus, Nav: Fixed keyboard/gamepad navigation occasionally erroneousl…
Browse files Browse the repository at this point in the history
…y landing on menu-item in parent when the parent is not a popup. (ocornut#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
  • Loading branch information
ocornut authored and kjblanchard committed May 5, 2023
1 parent 2cc4722 commit 788154a
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 9 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions imgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion imgui.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

/*
Expand Down
1 change: 1 addition & 0 deletions imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 4 additions & 6 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 788154a

Please sign in to comment.