Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Help users if they are reusing the same ID (Fix #7669) #7961

Closed
wants to merge 1 commit into from
Closed

Help users if they are reusing the same ID (Fix #7669) #7961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions imgui.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4904,6 +4904,10 @@ void ImGui::NewFrame()
g.HoverItemDelayTimer = g.HoverItemDelayClearTimer = 0.0f; // May want a decaying timer, in which case need to clamp at max first, based on max of caller last requested timer.
}

// TRKID: inside ImGui::NewFrame, empty the list of items, but keep its allocated data. Also reset WasDuplicateTipDisplayedAlready
g.UidsThisFrame.resize(0); // Empty the list of items, but keep its allocated data
g.UidWasTipDisplayed = false;

// Drag and drop
g.DragDropAcceptIdPrev = g.DragDropAcceptIdCurr;
g.DragDropAcceptIdCurr = 0;
Expand Down Expand Up @@ -5235,6 +5239,14 @@ void ImGui::EndFrame()
return;
IM_ASSERT(g.WithinFrameScope && "Forgot to call ImGui::NewFrame()?");

// TRKID: inside ImGui::EndFrame(), ensure matched calls to Push/PopAllowDuplicateID, reset UidHighlightedDuplicate
IM_ASSERT(g.UidAllowDuplicatesStack == 0 && "Mismatched Push/PopAllowDuplicateID");
if (g.UidHighlightedTimestamp != ImGui::GetFrameCount())
{
g.UidHighlightedTimestamp = 0;
g.UidHighlightedDuplicate = 0;
}

CallContextHooks(&g, ImGuiContextHookType_EndFramePre);

ErrorCheckEndFrameSanityChecks();
Expand Down Expand Up @@ -8485,8 +8497,106 @@ ImGuiID ImGui::GetID(int int_id)
ImGuiWindow* window = GImGui->CurrentWindow;
return window->GetID(int_id);
}

IM_MSVC_RUNTIME_CHECKS_RESTORE

// TRKID: ReserveUniqueID impl: returns GetID(), but may display a warning. Should be called only once per frame with a given ID
// Developer notes (to be removed before merging)
// - The prefix "Reserve" is an attempt to make it clear that this method should be called only once per frame for each str id.
// It is already the case in the existing codebase of Dear ImGui. However, I opted to make it clear in the name of the function.
// - Some calls to GetID() were changed to ReserveUniqueID() inside imgui_widgets.cpp.
IMGUI_API ImGuiID ImGuiWindow::ReserveUniqueID(const char* str_id)
{
ImGuiContext& g = *GImGui;
ImGuiID id = GetID(str_id);

// If PushAllowDuplicateID was called, do not check for uniqueness
if (g.UidAllowDuplicatesStack != 0)
return id;

// Visual aid: a red circle in the top-left corner of all widgets that use a duplicate of the id
if (id == g.UidHighlightedDuplicate)
{
ImGui::GetWindowDrawList()->AddCircleFilled(ImGui::GetCursorScreenPos(), 3.f, 0xFF0000FF);
}

// TRKID: Debug duplicates during development
// Only enable this temporarily when looking for duplicate ID throughout the code. Never enable in production, as it result in a O(n^2) search!
#ifdef IMGUI_DEBUG_PARANOID // can be defined inside imconfig.h
if (g.UidsThisFrame.contains(id))
{
IMGUI_DEBUG_LOG("Detected Duplicated ID : 0x%08X (\"%s\")\n", id, str_id);
IM_ASSERT_PARANOID(false && "Duplicated ID detected");
}
#endif

// Only check for uniqueness if hovered (this avoid a O(n^2) search at each frame)
if (id == ImGui::GetHoveredID())
{
if (g.UidsThisFrame.contains(id) && !g.UidWasTipDisplayed)
{
// Prepare highlight on the next frame for widgets that use this ID
g.UidHighlightedDuplicate = id;
g.UidHighlightedTimestamp = ImGui::GetFrameCount();
if (ImGui::BeginTooltip())
{
if (! g.DebugItemPickerActive)
{
ImGui::Text("Duplicate ID detected: \"%s\"!", str_id);
ImGui::Text("Widgets with a red dot (");
ImGui::SameLine();
ImVec2 dot_position = ImGui::GetCursorScreenPos();
dot_position.y += ImGui::GetFontSize() / 2.f;
dot_position.x -= ImGui::GetFontSize() / 15.f;
ImGui::GetWindowDrawList()->AddCircleFilled(dot_position, 3.f, 0xFF0000FF);
ImGui::Text(" )in their top-left corner use this same ID");
ImGui::Separator();
ImGui::BulletText("Either use \"##\" to pass a complement to the ID that \nwon't be visible to the end-user, for example:");
ImGui::Text(" if (ImGui::Button(\"Save##foo1\")");
ImGui::Text(" // Handle click action");
ImGui::BulletText("Or use ImGui::PushID() / PopID() to change the ID stack in a given \ncontext, for example:");
ImGui::Text(" ImGui::PushID(\"MyContext\")");
ImGui::Text(" if (ImGui::Button(\"Save\")");
ImGui::Text(" // Handle click action");
ImGui::Text(" ImGui::PopID();");
ImGui::Separator();

// Launch ItemPicker if Ctrl-P is pressed
ImGui::Text(" Press Ctrl-P to break in any of those items ");
ImGui::GetWindowDrawList()->AddRectFilled(ImGui::GetItemRectMin(), ImGui::GetItemRectMax(), IM_COL32(255, 255, 0, 32));
if (ImGui::IsKeyPressed(ImGuiKey_P) && (ImGui::IsKeyDown(ImGuiKey_LeftCtrl) || ImGui::IsKeyDown(ImGuiKey_RightCtrl)))
g.DebugItemPickerActive = true;
}
else
{
// Add additional info at the bottom of the ItemPicker tooltip
ImGui::Separator();
ImGui::Text("Click on one of the widgets which uses a duplicated item");
}
ImGui::EndTooltip();
}
g.UidWasTipDisplayed = true;
}
}
g.UidsThisFrame.push_back(id);
return id;
}

// TRKID: Implement PushAllowDuplicateID / PopAllowDuplicateID
IMGUI_API void ImGui::PushAllowDuplicateID()
{
ImGuiContext& g = *GImGui;
g.UidAllowDuplicatesStack++;
}

IMGUI_API void ImGui::PopAllowDuplicateID()
{
ImGuiContext& g = *GImGui;
IM_ASSERT(g.UidAllowDuplicatesStack > 0 && "Mismatched PopAllowDuplicateID()");
g.UidAllowDuplicatesStack--;
}


//-----------------------------------------------------------------------------
// [SECTION] INPUTS
//-----------------------------------------------------------------------------
Expand Down
15 changes: 12 additions & 3 deletions imgui_demo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2000,8 +2000,9 @@ static void ShowDemoWindowWidgets(ImGuiDemoWindowData* demo_data)
ImGui::SameLine();
ImGui::SliderInt("Sample count", &display_count, 1, 400);
float (*func)(void*, int) = (func_type == 0) ? Funcs::Sin : Funcs::Saw;
ImGui::PlotLines("Lines", func, NULL, display_count, 0, NULL, -1.0f, 1.0f, ImVec2(0, 80));
ImGui::PlotHistogram("Histogram", func, NULL, display_count, 0, NULL, -1.0f, 1.0f, ImVec2(0, 80));
// TRKID: imgui_demo.cpp / Widgets/Plotting: simple id fix
ImGui::PlotLines("Lines##2", func, NULL, display_count, 0, NULL, -1.0f, 1.0f, ImVec2(0, 80));
ImGui::PlotHistogram("Histogram##2", func, NULL, display_count, 0, NULL, -1.0f, 1.0f, ImVec2(0, 80));
ImGui::Separator();

ImGui::Text("Need better plotting and graphing? Consider using ImPlot:");
Expand Down Expand Up @@ -2596,6 +2597,10 @@ static void ShowDemoWindowWidgets(ImGuiDemoWindowData* demo_data)
IMGUI_DEMO_MARKER("Widgets/Drag and Drop/Drag to reorder items (simple)");
if (ImGui::TreeNode("Drag to reorder items (simple)"))
{
// TRKID: Widgets/Drag and Drop/Drag to reorder items (simple) this demo will trigger a duplicated id during *one* frame (and *one* frame only)
// This is because it reorders items mid-flight, and may redisplay the same item in the frame where reordering happens.
// This is not a big issue : a red dot may flash for *one* frame (an assert may trigger if IMGUI_DEBUG_PARANOID is defined)

// Simple reordering
HelpMarker(
"We don't use the drag and drop api at all here! "
Expand Down Expand Up @@ -4225,7 +4230,8 @@ static void ShowDemoWindowLayout()
// down by FramePadding.y ahead of time)
ImGui::AlignTextToFramePadding();
ImGui::Text("OK Blahblah"); ImGui::SameLine();
ImGui::Button("Some framed item"); ImGui::SameLine();
// TRKID: imgui_demo.cpp, "Layout/Text Baseline Alignment": simple ID fix
ImGui::Button("Some framed item##2"); ImGui::SameLine();
HelpMarker("We call AlignTextToFramePadding() to vertically align the text baseline by +FramePadding.y");

// SmallButton() uses the same vertical padding as Text
Expand Down Expand Up @@ -7146,6 +7152,8 @@ static void ShowDemoWindowColumns()
ImGui::Columns(columns_count, NULL, v_borders);
for (int i = 0; i < columns_count * lines_count; i++)
{
// TRKID: imgui_demo.cpp / Columns (legacy API)/Borders: added ImGui::PushID for each cell
ImGui::PushID(i);
if (h_borders && ImGui::GetColumnIndex() == 0)
ImGui::Separator();
ImGui::Text("%c%c%c", 'a' + i, 'a' + i, 'a' + i);
Expand All @@ -7155,6 +7163,7 @@ static void ShowDemoWindowColumns()
ImGui::Text("Long text that is likely to clip");
ImGui::Button("Button", ImVec2(-FLT_MIN, 0.0f));
ImGui::NextColumn();
ImGui::PopID();
}
ImGui::Columns(1);
if (h_borders)
Expand Down
26 changes: 26 additions & 0 deletions imgui_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2079,6 +2079,18 @@ struct ImGuiContext
ImGuiID LastActiveId; // Store the last non-zero ActiveId, useful for animation.
float LastActiveIdTimer; // Store the last non-zero ActiveId timer since the beginning of activation, useful for animation.

// TRKID: struct ImGuiContext: added fields UidIXXX (UidsThisFrame & co)
// Technical note (to remove upon merging):
// I considered adding these fields to ImGuiWindowTempData, but had to revert:
// since it is legal to call several times ImGui::Begin("MyWindow") within the same frame,
// clearing the list of ItemID inside ImGui::End() would not be sufficient, as identical
// IDs may appear within different calls to ImGui::Begin("MyWindow")/ImGui::End() and not be detected.
ImVector<ImGuiID> UidsThisFrame; // A list of item IDs submitted this frame (used to detect and warn about duplicate ID usage).
int UidAllowDuplicatesStack; // Stack depth for allowing duplicate IDs (if > 0, duplicate IDs are allowed). See PushAllowDuplicateID / PopAllowDuplicateID
ImGuiID UidHighlightedDuplicate; // Will be set if a duplicated item is hovered (all duplicated items will appear with a red dot in the top left corner on the next frame)
int UidHighlightedTimestamp; // Timestamp (FrameCount) of the highlight (which will be shown on the next frame)
bool UidWasTipDisplayed; // Will be set to true to avoid displaying multiple times the same tooltip

// Key/Input Ownership + Shortcut Routing system
// - The idea is that instead of "eating" a given key, we can link to an owner.
// - Input query can then read input by specifying ImGuiKeyOwner_Any (== 0), ImGuiKeyOwner_NoOwner (== -1) or a custom ID.
Expand Down Expand Up @@ -2400,6 +2412,13 @@ struct ImGuiContext
LastActiveId = 0;
LastActiveIdTimer = 0.0f;

// TRKID: initialize ImGuiContext UidXXX fields
UidsThisFrame.clear();
UidAllowDuplicatesStack = 0;
UidHighlightedDuplicate = 0;
UidHighlightedTimestamp = 0;
UidWasTipDisplayed = false;

LastKeyboardKeyPressTime = LastKeyModsChangeTime = LastKeyModsChangeFromNoneTime = -1.0;

ActiveIdUsingNavDirMask = 0x00;
Expand Down Expand Up @@ -2718,6 +2737,8 @@ struct IMGUI_API ImGuiWindow
ImGuiID GetID(int n);
ImGuiID GetIDFromPos(const ImVec2& p_abs);
ImGuiID GetIDFromRectangle(const ImRect& r_abs);
// TRKID: ReserveUniqueID declaration: returns GetID(), but may display a warning. Should be called only once per frame with a given ID
ImGuiID ReserveUniqueID(const char* str_id); // returns GetID(), but may display a warning tooltip if the ID is not unique. Should be called only once per frame with a given ID stack.

// We don't use g.FontSize because the window may be != g.CurrentWindow.
ImRect Rect() const { return ImRect(Pos.x, Pos.y, Pos.x + Size.x, Pos.y + Size.y); }
Expand Down Expand Up @@ -3219,6 +3240,10 @@ namespace ImGui
IMGUI_API ImGuiID GetIDWithSeed(const char* str_id_begin, const char* str_id_end, ImGuiID seed);
IMGUI_API ImGuiID GetIDWithSeed(int n, ImGuiID seed);

// TRKID: PushAllowDuplicateID() / PopAllowDuplicateID() declared inside imgui_internal
IMGUI_API void PushAllowDuplicateID(); // Disables the tooltip warning when duplicate IDs are detected.
IMGUI_API void PopAllowDuplicateID(); // Re-enables the tooltip warning when duplicate IDs are detected.

// Basic Helpers for widget code
IMGUI_API void ItemSize(const ImVec2& size, float text_baseline_y = -1.0f);
inline void ItemSize(const ImRect& bb, float text_baseline_y = -1.0f) { ItemSize(bb.GetSize(), text_baseline_y); } // FIXME: This is a misleading API since we expect CursorPos to be bb.Min.
Expand Down Expand Up @@ -3548,6 +3573,7 @@ namespace ImGui
IMGUI_API void Scrollbar(ImGuiAxis axis);
IMGUI_API bool ScrollbarEx(const ImRect& bb, ImGuiID id, ImGuiAxis axis, ImS64* p_scroll_v, ImS64 avail_v, ImS64 contents_v, ImDrawFlags flags);
IMGUI_API ImRect GetWindowScrollbarRect(ImGuiWindow* window, ImGuiAxis axis);
IMGUI_API ImGuiID ReserveWindowScrollbarID(ImGuiWindow* window, ImGuiAxis axis);
IMGUI_API ImGuiID GetWindowScrollbarID(ImGuiWindow* window, ImGuiAxis axis);
IMGUI_API ImGuiID GetWindowResizeCornerID(ImGuiWindow* window, int n); // 0..3: corners
IMGUI_API ImGuiID GetWindowResizeBorderID(ImGuiWindow* window, ImGuiDir dir);
Expand Down
Loading
Loading