Popups: Fixed an issue when reopening a same popup multiple times wou…

…ld 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

@@ -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).

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
  {

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).

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)

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));