Skip to content

Commit

Permalink
Added io.ConfigDebugHighlightIdConflicts debug feature! (#7961, #7669)
Browse files Browse the repository at this point in the history
  • Loading branch information
ocornut committed Sep 10, 2024
1 parent a8eec24 commit 67cd4ea
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 7 deletions.
19 changes: 19 additions & 0 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,25 @@ Breaking changes:

Other changes:

- Added io.ConfigDebugHighlightIdConflicts debug feature! (#7961, #7669)
THIS DETECTS THE MOST COMMON USER ERROR BY FIRST-TIME DEAR IMGUI PROGRAMMERS!
- The tool detects when multiple items are sharing the same identifier, due to not
using PushID/PopID in loops, or not using ID stack facilities such as "##" suffixes.
Very frequently it happens when using empty "" labels.
- When hovering an item with a conflicting ID, all visible items with the same ID will
be highlighted and an explanatory tooltip is made visible.
- The feature may be disabled and is exposed in Demo->Tools menu.
- I've been wanting to add this tool for a long time, but was stalled by finding a way to
not make it spammy + make it practically zero cost. After @pthom made various proposals to
solve the same problem (thanks for pushing me!), I decided it was time to finish it.
- Added ImGuiItemFlags_AllowDuplicateId to use with PushItemFlag/PopItemFlag() if for some
reason you intend to have duplicate identifiers.
- (#74, #96, #480, #501, #647, #654, #719, #843, #894, #1057, #1173, #1390, #1414, #1556, #1768,
#2041, #2116, #2330, #2475, #2562, #2667, #2807, #2885, #3102, #3375, #3526, #3964, #4008,
#4070, #4158, #4172, #4199, #4375, #4395, #4471, #4548, #4612, #4631, #4657, #4796, #5210,
#5303, #5360, #5393, #5533, #5692, #5707, #5729, #5773, #5787, #5884, #6046, #6093, #6186,
#6223, #6364, #6387, #6567, #6692, #6724, #6939, #6984, #7246, #7270, #7375, #7421, #7434,
#7472, #7581, #7724, #7926, #7937 and probably more..)
- Nav: pressing any keyboard key while holding Alt disable toggling nav layer on Alt release. (#4439)
- InputText: added CJK double-width punctuation to list of separators considered for CTRL+Arrow.
- TextLinkOpenURL(): modified tooltip to display a verb "Open 'xxxx'". (#7885, #7660)
Expand Down
5 changes: 3 additions & 2 deletions docs/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,11 @@ ctx->RSSetScissorRects(1, &r);
### Q: How can I have multiple widgets with the same label?
### Q: How can I have multiple windows with the same label?

**USING THE SAME LABEL+ID IS THE MOST COMMON USER MISTAKE:**
**USING THE SAME LABEL+ID IS THE MOST COMMON USER MISTAKE!**
<br>**USING AN EMPTY LABEL IS THE SAME AS USING THE SAME LABEL AS YOUR PARENT WIDGET!**
<table>
<tr>
<td><img src="https://github.com/ocornut/imgui/assets/8225057/76eb9467-74d1-4e95-9f56-be81c6dd029d"></td>
<td><img src="https://github.com/user-attachments/assets/776a8315-1164-4178-9a8c-df52e0ff28aa"></td>
<td>
<pre lang="cpp">
ImGui::Begin("Incorrect!");
Expand Down
42 changes: 42 additions & 0 deletions imgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1395,6 +1395,8 @@ ImGuiIO::ImGuiIO()
ConfigWindowsResizeFromEdges = true;
ConfigWindowsMoveFromTitleBarOnly = false;
ConfigMemoryCompactTimer = 60.0f;
ConfigDebugIsDebuggerPresent = false;
ConfigDebugHighlightIdConflicts = true;
ConfigDebugBeginReturnValueOnce = false;
ConfigDebugBeginReturnValueLoop = false;

Expand Down Expand Up @@ -4290,6 +4292,17 @@ bool ImGui::ItemHoverable(const ImRect& bb, ImGuiID id, ImGuiItemFlags item_flag
{
ImGuiContext& g = *GImGui;
ImGuiWindow* window = g.CurrentWindow;

// Detect ID conflicts
#ifndef IMGUI_DISABLE_DEBUG_TOOLS
if (id != 0 && g.HoveredIdPreviousFrame == id && (item_flags & ImGuiItemFlags_AllowDuplicateId) == 0)
{
g.HoveredIdPreviousFrameItemCount++;
if (g.DebugDrawIdConflicts == id)
window->DrawList->AddRect(bb.Min - ImVec2(1,1), bb.Max + ImVec2(1,1), IM_COL32(255, 0, 0, 255), 0.0f, ImDrawFlags_None, 2.0f);
}
#endif

if (g.HoveredWindow != window)
return false;
if (!IsMouseHoveringRect(bb.Min, bb.Max))
Expand Down Expand Up @@ -4833,6 +4846,11 @@ void ImGui::NewFrame()
if (g.DragDropActive && g.DragDropPayload.SourceId == g.ActiveId)
KeepAliveID(g.DragDropPayload.SourceId);

// [DEBUG]
g.DebugDrawIdConflicts = 0;
if (g.IO.ConfigDebugHighlightIdConflicts && g.HoveredIdPreviousFrameItemCount > 1)
g.DebugDrawIdConflicts = g.HoveredIdPreviousFrame;

// Update HoveredId data
if (!g.HoveredIdPreviousFrame)
g.HoveredIdTimer = 0.0f;
Expand All @@ -4843,6 +4861,7 @@ void ImGui::NewFrame()
if (g.HoveredId && g.ActiveId != g.HoveredId)
g.HoveredIdNotActiveTimer += g.IO.DeltaTime;
g.HoveredIdPreviousFrame = g.HoveredId;
g.HoveredIdPreviousFrameItemCount = 0;
g.HoveredId = 0;
g.HoveredIdAllowOverlap = false;
g.HoveredIdIsDisabled = false;
Expand Down Expand Up @@ -5235,6 +5254,29 @@ void ImGui::EndFrame()
return;
IM_ASSERT(g.WithinFrameScope && "Forgot to call ImGui::NewFrame()?");

#ifndef IMGUI_DISABLE_DEBUG_TOOLS
if (g.DebugDrawIdConflicts != 0)
{
PushStyleColor(ImGuiCol_PopupBg, ImLerp(g.Style.Colors[ImGuiCol_PopupBg], ImVec4(1.0f, 0.0f, 0.0f, 1.0f), 0.10f));
if (g.DebugItemPickerActive == false && BeginTooltipEx(ImGuiTooltipFlags_OverridePrevious, ImGuiWindowFlags_None))
{
SeparatorText("MESSAGE FROM DEAR IMGUI");
Text("Programmer error: %d visible items with conflicting ID!", g.HoveredIdPreviousFrameItemCount);
BulletText("Code should use PushID()/PopID() in loops, or append \"##xx\" to same-label identifiers!");
BulletText("Empty label e.g. Button(\"\") == same ID as parent widget/node. Use Button(\"##xx\") instead!");
BulletText("Press F1 to open \"FAQ -> About the ID Stack System\" and read details.");
BulletText("Press CTRL+P to activate Item Picker and debug-break in item call-stack.");
BulletText("Set io.ConfigDebugDetectIdConflicts=false to disable this warning in non-programmers builds.");
EndTooltip();
}
PopStyleColor();
if (Shortcut(ImGuiMod_Ctrl | ImGuiKey_P, ImGuiInputFlags_RouteGlobal))
DebugStartItemPicker();
if (Shortcut(ImGuiKey_F1, ImGuiInputFlags_RouteGlobal) && g.PlatformIO.Platform_OpenInShellFn != NULL)
g.PlatformIO.Platform_OpenInShellFn(&g, "https://github.com/ocornut/imgui/blob/master/docs/FAQ.md#qa-usage");
}
#endif

CallContextHooks(&g, ImGuiContextHookType_EndFramePre);

ErrorCheckEndFrameSanityChecks();
Expand Down
11 changes: 10 additions & 1 deletion imgui.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
// Library Version
// (Integer encoded as XYYZZ for use in #if preprocessor conditionals, e.g. '#if IMGUI_VERSION_NUM >= 12345')
#define IMGUI_VERSION "1.91.2 WIP"
#define IMGUI_VERSION_NUM 19112
#define IMGUI_VERSION_NUM 19113
#define IMGUI_HAS_TABLE

/*
Expand Down Expand Up @@ -1135,6 +1135,7 @@ enum ImGuiItemFlags_
ImGuiItemFlags_NoNavDefaultFocus = 1 << 2, // false // Disable item being a candidate for default focus (e.g. used by title bar items).
ImGuiItemFlags_ButtonRepeat = 1 << 3, // false // Any button-like behavior will have repeat mode enabled (based on io.KeyRepeatDelay and io.KeyRepeatRate values). Note that you can also call IsItemActive() after any button to tell if it is being held.
ImGuiItemFlags_AutoClosePopups = 1 << 4, // true // MenuItem()/Selectable() automatically close their parent popup window.
ImGuiItemFlags_AllowDuplicateId = 1 << 5, // false // Allow submitting an item with the same identifier as an item already submitted this frame without triggering a warning tooltip if io.ConfigDebugHighlightIdConflicts is set.
};

// Flags for ImGui::InputText()
Expand Down Expand Up @@ -2231,13 +2232,15 @@ struct ImGuiIO
const char* LogFilename; // = "imgui_log.txt"// Path to .log file (default parameter to ImGui::LogToFile when no file is specified).
void* UserData; // = NULL // Store your own data.

// Font system
ImFontAtlas*Fonts; // <auto> // Font atlas: load, rasterize and pack one or more fonts into a single texture.
float FontGlobalScale; // = 1.0f // Global scale all fonts
bool FontAllowUserScaling; // = false // Allow user scaling text of individual window with CTRL+Wheel.
ImFont* FontDefault; // = NULL // Font to use on NewFrame(). Use NULL to uses Fonts->Fonts[0].
ImVec2 DisplayFramebufferScale; // = (1, 1) // For retina display or other situations where window coordinates are different from framebuffer coordinates. This generally ends up in ImDrawData::FramebufferScale.

// Miscellaneous options
// (you can visualize and interact with all options in 'Demo->Configuration')
bool MouseDrawCursor; // = false // Request ImGui to draw a mouse cursor for you (if you are on a platform without a mouse cursor). Cannot be easily renamed to 'io.ConfigXXX' because this is frequently used by backend implementations.
bool ConfigMacOSXBehaviors; // = defined(__APPLE__) // Swap Cmd<>Ctrl keys + OS X style text editing cursor movement using Alt instead of Ctrl, Shortcuts using Cmd/Super instead of Ctrl, Line/Text Start and End using Cmd+Arrows instead of Home/End, Double click selects by word instead of selecting whole text, Multi-selection in lists uses Cmd/Super instead of Ctrl.
bool ConfigNavSwapGamepadButtons; // = false // Swap Activate<>Cancel (A<>B) buttons, matching typical "Nintendo/Japanese style" gamepad layout.
Expand Down Expand Up @@ -2267,6 +2270,12 @@ struct ImGuiIO
// e.g. io.ConfigDebugIsDebuggerPresent = ::IsDebuggerPresent() on Win32, or refer to ImOsIsDebuggerPresent() imgui_test_engine/imgui_te_utils.cpp for a Unix compatible version).
bool ConfigDebugIsDebuggerPresent; // = false // Enable various tools calling IM_DEBUG_BREAK().

// Tools to detect code submitting items with conflicting/duplicate IDs
// - Code should use PushID()/PopID() in loops, or append "##xx" to same-label identifiers.
// - Empty label e.g. Button("") == same ID as parent widget/node. Use Button("##xx") instead!
// - See FAQ https://github.com/ocornut/imgui/blob/master/docs/FAQ.md#q-about-the-id-stack-system
bool ConfigDebugHighlightIdConflicts;// = true // Highlight and show an error message when multiple items have conflicting identifiers.

// Tools to test correct Begin/End and BeginChild/EndChild behaviors.
// - Presently Begin()/End() and BeginChild()/EndChild() needs to ALWAYS be called in tandem, regardless of return value of BeginXXX()
// - This is inconsistent with other BeginXXX functions and create confusion for many users.
Expand Down
18 changes: 14 additions & 4 deletions imgui_demo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,8 @@ void ImGui::ShowDemoWindow(bool* p_open)
ImGui::SeparatorText("Debug");
ImGui::Checkbox("io.ConfigDebugIsDebuggerPresent", &io.ConfigDebugIsDebuggerPresent);
ImGui::SameLine(); HelpMarker("Enable various tools calling IM_DEBUG_BREAK().\n\nRequires a debugger being attached, otherwise IM_DEBUG_BREAK() options will appear to crash your application.");
ImGui::Checkbox("io.ConfigDebugHighlightIdConflicts", &io.ConfigDebugHighlightIdConflicts);
ImGui::SameLine(); HelpMarker("Highlight and show an error message when multiple items have conflicting identifiers.");
ImGui::BeginDisabled();
ImGui::Checkbox("io.ConfigDebugBeginReturnValueOnce", &io.ConfigDebugBeginReturnValueOnce);
ImGui::EndDisabled();
Expand Down Expand Up @@ -684,6 +686,7 @@ static void ShowDemoWindowMenuBar(ImGuiDemoWindowData* demo_data)
if (ImGui::BeginMenu("Tools"))
{
IMGUI_DEMO_MARKER("Menu/Tools");
ImGuiIO& io = ImGui::GetIO();
#ifndef IMGUI_DISABLE_DEBUG_TOOLS
const bool has_debug_tools = true;
#else
Expand All @@ -692,14 +695,16 @@ static void ShowDemoWindowMenuBar(ImGuiDemoWindowData* demo_data)
ImGui::MenuItem("Metrics/Debugger", NULL, &demo_data->ShowMetrics, has_debug_tools);
ImGui::MenuItem("Debug Log", NULL, &demo_data->ShowDebugLog, has_debug_tools);
ImGui::MenuItem("ID Stack Tool", NULL, &demo_data->ShowIDStackTool, has_debug_tools);
ImGui::MenuItem("Style Editor", NULL, &demo_data->ShowStyleEditor);
bool is_debugger_present = ImGui::GetIO().ConfigDebugIsDebuggerPresent;
bool is_debugger_present = io.ConfigDebugIsDebuggerPresent;
if (ImGui::MenuItem("Item Picker", NULL, false, has_debug_tools && is_debugger_present))
ImGui::DebugStartItemPicker();
if (!is_debugger_present)
ImGui::SetItemTooltip("Requires io.ConfigDebugIsDebuggerPresent=true to be set.\n\nWe otherwise disable the menu option to avoid casual users crashing the application.\n\nYou can however always access the Item Picker in Metrics->Tools.");
ImGui::Separator();
ImGui::MenuItem("Style Editor", NULL, &demo_data->ShowStyleEditor);
ImGui::MenuItem("About Dear ImGui", NULL, &demo_data->ShowAbout);

ImGui::SeparatorText("Debug Options");
ImGui::MenuItem("Highlight ID Conflicts", NULL, &io.ConfigDebugHighlightIdConflicts, has_debug_tools);
ImGui::EndMenu();
}
ImGui::EndMenuBar();
Expand Down Expand Up @@ -2596,7 +2601,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)"))
{
// FIXME: temporary ID Conflict during reordering as a same item may be submitting twice.
// FIXME: there is temporary (usually single-frame) ID Conflict during reordering as a same item may be submitting twice.
// This code was always slightly faulty but in a way which was not easily noticeable.
// Until we fix this, enable ImGuiItemFlags_AllowDuplicateId to disable detecting the issue.
ImGui::PushItemFlag(ImGuiItemFlags_AllowDuplicateId, true);

// Simple reordering
HelpMarker(
Expand All @@ -2619,6 +2627,8 @@ static void ShowDemoWindowWidgets(ImGuiDemoWindowData* demo_data)
}
}
}

ImGui::PopItemFlag();
ImGui::TreePop();
}

Expand Down
4 changes: 4 additions & 0 deletions imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -2042,9 +2042,11 @@ struct ImGuiContext
ImVec2 WheelingAxisAvg;

// Item/widgets state and tracking information
ImGuiID DebugDrawIdConflicts; // Set when we detect multiple items with the same identifier
ImGuiID DebugHookIdInfo; // Will call core hooks: DebugHookIdInfo() from GetID functions, used by ID Stack Tool [next HoveredId/ActiveId to not pull in an extra cache-line]
ImGuiID HoveredId; // Hovered widget, filled during the frame
ImGuiID HoveredIdPreviousFrame;
int HoveredIdPreviousFrameItemCount; // Count numbers of items using the same ID as last frame's hovered id
float HoveredIdTimer; // Measure contiguous hovering time
float HoveredIdNotActiveTimer; // Measure contiguous hovering time where the item has not been active
bool HoveredIdAllowOverlap;
Expand Down Expand Up @@ -2365,8 +2367,10 @@ struct ImGuiContext
WheelingWindowStartFrame = WheelingWindowScrolledFrame = -1;
WheelingWindowReleaseTimer = 0.0f;

DebugDrawIdConflicts = 0;
DebugHookIdInfo = 0;
HoveredId = HoveredIdPreviousFrame = 0;
HoveredIdPreviousFrameItemCount = 0;
HoveredIdAllowOverlap = false;
HoveredIdIsDisabled = false;
HoveredIdTimer = HoveredIdNotActiveTimer = 0.0f;
Expand Down
3 changes: 3 additions & 0 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3551,6 +3551,8 @@ int ImParseFormatPrecision(const char* fmt, int default_precision)

// Create text input in place of another active widget (e.g. used when doing a CTRL+Click on drag/slider widgets)
// FIXME: Facilitate using this in variety of other situations.
// FIXME: Among other things, setting ImGuiItemFlags_AllowDuplicateId in LastItemData is currently correct but
// the expected relationship between TempInputXXX functions and LastItemData is a little fishy.
bool ImGui::TempInputText(const ImRect& bb, ImGuiID id, const char* label, char* buf, int buf_size, ImGuiInputTextFlags flags)
{
// On the first frame, g.TempInputTextId == 0, then on subsequent frames it becomes == id.
Expand All @@ -3561,6 +3563,7 @@ bool ImGui::TempInputText(const ImRect& bb, ImGuiID id, const char* label, char*
ClearActiveID();

g.CurrentWindow->DC.CursorPos = bb.Min;
g.LastItemData.InFlags |= ImGuiItemFlags_AllowDuplicateId;
bool value_changed = InputTextEx(label, NULL, buf, buf_size, bb.GetSize(), flags | ImGuiInputTextFlags_MergedItem);
if (init)
{
Expand Down

0 comments on commit 67cd4ea

Please sign in to comment.