From 0b1bcfcc205077d31493ec851f87ca4b54b4eab5 Mon Sep 17 00:00:00 2001 From: Rokas Kupstys Date: Wed, 8 Jun 2022 12:34:35 +0300 Subject: [PATCH] Menus: Separate menu sets by nav layer. (#3496, #4797) + Demo: Remove incorrect and useless suggestion to use PushID(). Fixes a common case where opening menu in one nav layer and hovering a menu in another nav layer would open that menu without a click. --- docs/CHANGELOG.txt | 2 ++ imgui.cpp | 1 + imgui_demo.cpp | 7 ------- imgui_internal.h | 3 ++- imgui_widgets.cpp | 11 ++++++++--- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 23a55026d..e1e8737f3 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -101,6 +101,8 @@ Other Changes: - Menus: Adjusted BeginMenu() closing logic so hovering void or non-MenuItem() in parent window always lead to menu closure. Fixes using items that are not MenuItem() or BeginItem() at the root level of a popup with a child menu opened. +- Menus: Menus emitted from the main/scrolling layer are not part of the same menuset as menus emitted + from the menu-bar, avoiding accidental hovering from one to the other. (#3496, #4797) [@rokups] - Stack Tool: Added option to copy item path to clipboard. (#4631) - Settings: Fixed out-of-bounds read when .ini file on disk is empty. (#5351) [@quantum5] - DrawList: Fixed PathArcTo() emitting terminating vertices too close to arc vertices. (#4993) [@thedmd] diff --git a/imgui.cpp b/imgui.cpp index 892043872..19fac158b 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -6071,6 +6071,7 @@ bool ImGui::Begin(const char* name, bool* p_open, ImGuiWindowFlags flags) { ImGuiPopupData& popup_ref = g.OpenPopupStack[g.BeginPopupStack.Size]; popup_ref.Window = window; + popup_ref.ParentNavLayer = parent_window_in_stack->DC.NavLayerCurrent; g.BeginPopupStack.push_back(popup_ref); window->PopupId = popup_ref.PopupId; } diff --git a/imgui_demo.cpp b/imgui_demo.cpp index 46fe4f294..6684936c2 100644 --- a/imgui_demo.cpp +++ b/imgui_demo.cpp @@ -3546,19 +3546,12 @@ static void ShowDemoWindowPopups() ImGui::TextWrapped("Below we are testing adding menu items to a regular window. It's rather unusual but should work!"); ImGui::Separator(); - // Note: As a quirk in this very specific example, we want to differentiate the parent of this menu from the - // parent of the various popup menus above. To do so we are encloding the items in a PushID()/PopID() block - // to make them two different menusets. If we don't, opening any popup above and hovering our menu here would - // open it. This is because once a menu is active, we allow to switch to a sibling menu by just hovering on it, - // which is the desired behavior for regular menus. - ImGui::PushID("foo"); ImGui::MenuItem("Menu item", "CTRL+M"); if (ImGui::BeginMenu("Menu inside a regular window")) { ShowExampleMenuFile(); ImGui::EndMenu(); } - ImGui::PopID(); ImGui::Separator(); ImGui::TreePop(); } diff --git a/imgui_internal.h b/imgui_internal.h index 3f7cc5f82..309b4bd71 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -1038,12 +1038,13 @@ struct ImGuiPopupData ImGuiID PopupId; // Set on OpenPopup() ImGuiWindow* Window; // Resolved on BeginPopup() - may stay unresolved if user never calls OpenPopup() ImGuiWindow* SourceWindow; // Set on OpenPopup() copy of NavWindow at the time of opening the popup + int ParentNavLayer; // Resolved on BeginPopup(). Actually a ImGuiNavLayer type (declared down below), initialized to -1 which is not part of an enum, but serves well-enough as "not any of layers" value int OpenFrameCount; // Set on OpenPopup() ImGuiID OpenParentId; // Set on OpenPopup(), we need this to differentiate multiple menu sets from each others (e.g. inside menu bar vs loose menu items) ImVec2 OpenPopupPos; // Set on OpenPopup(), preferred popup position (typically == OpenMousePos when using mouse) ImVec2 OpenMousePos; // Set on OpenPopup(), copy of mouse position at the time of opening popup - ImGuiPopupData() { memset(this, 0, sizeof(*this)); OpenFrameCount = -1; } + ImGuiPopupData() { memset(this, 0, sizeof(*this)); ParentNavLayer = OpenFrameCount = -1; } }; enum ImGuiNextWindowDataFlags_ diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index aaa42dcf8..980172e8e 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -6939,14 +6939,19 @@ static bool IsRootOfOpenMenuSet() if ((g.OpenPopupStack.Size <= g.BeginPopupStack.Size) || (window->Flags & ImGuiWindowFlags_ChildMenu)) return false; - // Initially we used 'OpenParentId' to differentiate multiple menu sets from each others (e.g. inside menu bar vs loose menu items) based on parent ID. + // Initially we used 'upper_popup->OpenParentId == window->IDStack.back()' to differentiate multiple menu sets from each others + // (e.g. inside menu bar vs loose menu items) based on parent ID. // This would however prevent the use of e.g. PuhsID() user code submitting menus. // Previously this worked between popup and a first child menu because the first child menu always had the _ChildWindow flag, // making hovering on parent popup possible while first child menu was focused - but this was generally a bug with other side effects. // Instead we don't treat Popup specifically (in order to consistently support menu features in them), maybe the first child menu of a Popup - // doesn't have the _ChildWindow flag, and we rely on this IsRootOfOpenMenuSet() check to allow hovering between root window/popup and first chilld menu. + // doesn't have the _ChildWindow flag, and we rely on this IsRootOfOpenMenuSet() check to allow hovering between root window/popup and first child menu. + // In the end, lack of ID check made it so we could no longer differentiate between separate menu sets. To compensate for that, we at least check parent window nav layer. + // This fixes the most common case of menu opening on hover when moving between window content and menu bar. Multiple different menu sets in same nav layer would still + // open on hover, but that should be a lesser problem, because if such menus are close in proximity in window content then it won't feel weird and if they are far apart + // it likely won't be a problem anyone runs into. const ImGuiPopupData* upper_popup = &g.OpenPopupStack[g.BeginPopupStack.Size]; - return (/*upper_popup->OpenParentId == window->IDStack.back() &&*/ upper_popup->Window && (upper_popup->Window->Flags & ImGuiWindowFlags_ChildMenu)); + return (window->DC.NavLayerCurrent == upper_popup->ParentNavLayer && upper_popup->Window && (upper_popup->Window->Flags & ImGuiWindowFlags_ChildMenu)); } bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled)