From b17b2fb732f4dda7641461a30c446837776d6667 Mon Sep 17 00:00:00 2001 From: ocornut Date: Mon, 24 Jan 2022 13:13:38 +0100 Subject: [PATCH] Popups: Fixed an issue when reopening a same popup multiple times would offset them by 1 pixel on the right. (#4936) Passing explicit ImGuiPopupFlags_MouseButtonRight to OpenPopupOnItemClick() calls somehow document the unusual (due to legacy) default value. --- docs/CHANGELOG.txt | 1 + imgui.cpp | 16 +++++----------- imgui.h | 3 ++- imgui_demo.cpp | 4 ++-- imgui_widgets.cpp | 12 ++++++------ 5 files changed, 16 insertions(+), 20 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 577624d6f..cbd324b51 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -110,6 +110,7 @@ Other Changes: - Fixed a situation where CTRL+Tab or Modal can occasionally lead to the creation of ImDrawCmd with zero triangles, which would makes the draw operation of some backends assert (e.g. Metal with debugging). (#4857) - Popups: Fixed a regression crash when a new window is created after a modal on the same frame. (#4920) [@rokups] +- Popups: Fixed an issue when reopening a same popup multiple times would offset them by 1 pixel on the right. (#4936) - Tables, ImDrawListSplitter: Fixed erroneously stripping trailing ImDrawList::AddCallback() when submitted in last column or last channel and when there are no other drawing operation. (#4843, #4844) [@hoffstadt] - Tables: Fixed positioning of Sort icon on right-most column with some settings (not resizable + no borders). (#4918). diff --git a/imgui.cpp b/imgui.cpp index 3df186d0b..bf2452d87 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -6235,8 +6235,6 @@ bool ImGui::Begin(const char* name, bool* p_open, ImGuiWindowFlags flags) SetWindowPos(window, window->SetWindowPosVal - window->Size * window->SetWindowPosPivot, 0); // Position given a pivot (e.g. for centering) else if ((flags & ImGuiWindowFlags_ChildMenu) != 0) window->Pos = FindBestWindowPosForPopup(window); - else if ((flags & ImGuiWindowFlags_Popup) != 0 && !window_pos_set_by_api && window_just_appearing_after_hidden_for_resize) - window->Pos = FindBestWindowPosForPopup(window); else if ((flags & ImGuiWindowFlags_Tooltip) != 0 && !window_pos_set_by_api && !window_is_child_tooltip) window->Pos = FindBestWindowPosForPopup(window); @@ -9261,7 +9259,7 @@ void ImGui::OpenPopupOnItemClick(const char* str_id, ImGuiPopupFlags popup_flags // - You may want to handle the whole on user side if you have specific needs (e.g. tweaking IsItemHovered() parameters). // This is essentially the same as: // id = str_id ? GetID(str_id) : GetItemID(); -// OpenPopupOnItemClick(str_id); +// OpenPopupOnItemClick(str_id, ImGuiPopupFlags_MouseButtonRight); // return BeginPopup(id); // Which is essentially the same as: // id = str_id ? GetID(str_id) : GetItemID(); @@ -9420,11 +9418,6 @@ ImVec2 ImGui::FindBestWindowPosForPopup(ImGuiWindow* window) r_avoid = ImRect(parent_window->Pos.x + horizontal_overlap, -FLT_MAX, parent_window->Pos.x + parent_window->Size.x - horizontal_overlap - parent_window->ScrollbarSizes.x, FLT_MAX); return FindBestWindowPosForPopupEx(window->Pos, window->Size, &window->AutoPosLastDirection, r_outer, r_avoid, ImGuiPopupPositionPolicy_Default); } - if (window->Flags & ImGuiWindowFlags_Popup) - { - ImRect r_avoid = ImRect(window->Pos.x - 1, window->Pos.y - 1, window->Pos.x + 1, window->Pos.y + 1); - return FindBestWindowPosForPopupEx(window->Pos, window->Size, &window->AutoPosLastDirection, r_outer, r_avoid, ImGuiPopupPositionPolicy_Default); - } if (window->Flags & ImGuiWindowFlags_Tooltip) { // Position tooltip (always follows mouse) @@ -9932,9 +9925,10 @@ static ImVec2 ImGui::NavCalcPreferredRefPos() if (g.NavDisableHighlight || !g.NavDisableMouseHover || !window) { // Mouse (we need a fallback in case the mouse becomes invalid after being used) - if (IsMousePosValid(&g.IO.MousePos)) - return g.IO.MousePos; - return g.MouseLastValidPos; + // The +1.0f offset when stored by OpenPopupEx() allows reopening this or another popup (same or another mouse button) while not moving the mouse, it is pretty standard. + // In theory we could move that +1.0f offset in OpenPopupEx() + ImVec2 p = IsMousePosValid(&g.IO.MousePos) ? g.IO.MousePos : g.MouseLastValidPos; + return ImVec2(p.x + 1.0f, p.y); } else { diff --git a/imgui.h b/imgui.h index a09566a08..20edf8394 100644 --- a/imgui.h +++ b/imgui.h @@ -681,6 +681,7 @@ namespace ImGui // - CloseCurrentPopup() is called by default by Selectable()/MenuItem() when activated (FIXME: need some options). // - Use ImGuiPopupFlags_NoOpenOverExistingPopup to avoid opening a popup if there's already one at the same level. This is equivalent to e.g. testing for !IsAnyPopupOpen() prior to OpenPopup(). // - Use IsWindowAppearing() after BeginPopup() to tell if a window just opened. + // - IMPORTANT: Notice that for OpenPopupOnItemClick() we exceptionally default flags to 1 (== ImGuiPopupFlags_MouseButtonRight) for backward compatibility with older API taking 'int mouse_button = 1' parameter IMGUI_API void OpenPopup(const char* str_id, ImGuiPopupFlags popup_flags = 0); // call to mark popup as open (don't call every frame!). IMGUI_API void OpenPopup(ImGuiID id, ImGuiPopupFlags popup_flags = 0); // id overload to facilitate calling from nested stacks IMGUI_API void OpenPopupOnItemClick(const char* str_id = NULL, ImGuiPopupFlags popup_flags = 1); // helper to open popup when clicked on last item. Default to ImGuiPopupFlags_MouseButtonRight == 1. (note: actually triggers on the mouse _released_ event to be consistent with popup behaviors) @@ -690,7 +691,7 @@ namespace ImGui // - Helpers to do OpenPopup+BeginPopup where the Open action is triggered by e.g. hovering an item and right-clicking. // - They are convenient to easily create context menus, hence the name. // - IMPORTANT: Notice that BeginPopupContextXXX takes ImGuiPopupFlags just like OpenPopup() and unlike BeginPopup(). For full consistency, we may add ImGuiWindowFlags to the BeginPopupContextXXX functions in the future. - // - IMPORTANT: we exceptionally default their flags to 1 (== ImGuiPopupFlags_MouseButtonRight) for backward compatibility with older API taking 'int mouse_button = 1' parameter, so if you add other flags remember to re-add the ImGuiPopupFlags_MouseButtonRight. + // - IMPORTANT: Notice that we exceptionally default their flags to 1 (== ImGuiPopupFlags_MouseButtonRight) for backward compatibility with older API taking 'int mouse_button = 1' parameter, so if you add other flags remember to re-add the ImGuiPopupFlags_MouseButtonRight. IMGUI_API bool BeginPopupContextItem(const char* str_id = NULL, ImGuiPopupFlags popup_flags = 1); // open+begin popup when clicked on last item. Use str_id==NULL to associate the popup to previous item. If you want to use that on a non-interactive item such as Text() you need to pass in an explicit ID here. read comments in .cpp! IMGUI_API bool BeginPopupContextWindow(const char* str_id = NULL, ImGuiPopupFlags popup_flags = 1);// open+begin popup when clicked on current window. IMGUI_API bool BeginPopupContextVoid(const char* str_id = NULL, ImGuiPopupFlags popup_flags = 1); // open+begin popup when clicked in void (where there are no windows). diff --git a/imgui_demo.cpp b/imgui_demo.cpp index d73389fe2..7952615b6 100644 --- a/imgui_demo.cpp +++ b/imgui_demo.cpp @@ -7570,8 +7570,8 @@ static void ShowExampleAppCustomRendering(bool* p_open) // Context menu (under default mouse threshold) ImVec2 drag_delta = ImGui::GetMouseDragDelta(ImGuiMouseButton_Right); - if (opt_enable_context_menu && ImGui::IsMouseReleased(ImGuiMouseButton_Right) && drag_delta.x == 0.0f && drag_delta.y == 0.0f) - ImGui::OpenPopupOnItemClick("context"); + if (opt_enable_context_menu && drag_delta.x == 0.0f && drag_delta.y == 0.0f) + ImGui::OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); if (ImGui::BeginPopup("context")) { if (adding_line) diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 8f52f1fb9..77d86e54a 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -4924,7 +4924,7 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag value_changed |= DragInt(ids[n], &i[n], 1.0f, 0, hdr ? 0 : 255, fmt_table_int[fmt_idx][n]); } if (!(flags & ImGuiColorEditFlags_NoOptions)) - OpenPopupOnItemClick("context"); + OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); } } else if ((flags & ImGuiColorEditFlags_DisplayHex) != 0 && (flags & ImGuiColorEditFlags_NoInputs) == 0) @@ -4952,7 +4952,7 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag IM_UNUSED(r); // Fixes C6031: Return value ignored: 'sscanf'. } if (!(flags & ImGuiColorEditFlags_NoOptions)) - OpenPopupOnItemClick("context"); + OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); } ImGuiWindow* picker_active_window = NULL; @@ -4969,11 +4969,11 @@ bool ImGui::ColorEdit4(const char* label, float col[4], ImGuiColorEditFlags flag // Store current color and open a picker g.ColorPickerRef = col_v4; OpenPopup("picker"); - SetNextWindowPos(g.LastItemData.Rect.GetBL() + ImVec2(-1, style.ItemSpacing.y)); + SetNextWindowPos(g.LastItemData.Rect.GetBL() + ImVec2(0.0f, style.ItemSpacing.y)); } } if (!(flags & ImGuiColorEditFlags_NoOptions)) - OpenPopupOnItemClick("context"); + OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); if (BeginPopup("picker")) { @@ -5187,7 +5187,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl } } if (!(flags & ImGuiColorEditFlags_NoOptions)) - OpenPopupOnItemClick("context"); + OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); } else if (flags & ImGuiColorEditFlags_PickerHueBar) { @@ -5204,7 +5204,7 @@ bool ImGui::ColorPicker4(const char* label, float col[4], ImGuiColorEditFlags fl value_changed = value_changed_sv = true; } if (!(flags & ImGuiColorEditFlags_NoOptions)) - OpenPopupOnItemClick("context"); + OpenPopupOnItemClick("context", ImGuiPopupFlags_MouseButtonRight); // Hue bar logic SetCursorScreenPos(ImVec2(bar0_pos_x, picker_pos.y));