Skip to content

Commit

Permalink
IsItemHovered(): fixed return value false positive when used after En…
Browse files Browse the repository at this point in the history
…dChild(), EndGroup() or widgets using either... (ocornut#3851, ocornut#1370)

...when the hovered location is located within a child window, e.g. InputTextMultiline().
This is intended to have no side effects, but brace yourself for the possible comeback..
This essentially makes IsItemHovered() not accept hover from child windows, but EndChild/EndGroup are forwarded.
More or less should fix/revert c76f014 which was a revert of 344d48b
  • Loading branch information
ocornut committed Mar 4, 2021
1 parent b53b8f5 commit ee643b2
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 11 deletions.
3 changes: 3 additions & 0 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ Other Changes:
area (e.g. when window is collapsed + moved in a corner) to facilitate moving the window away. (#3825)
- Window, Nav: Fixed crash when calling SetWindowFocus(NULL) as the time a new window appears. (#3865) [@nem0]
- Tables: Fixed unaligned accesses when using TableSetBgColor(ImGuiTableBgTarget_CellBg). (#3872)
- IsItemHovered(): fixed return value false positive when used after EndChild(), EndGroup() or widgets using
either of them, when the hovered location is located within a child window, e.g. InputTextMultiline().
This is intended to have no side effects, but brace yourself for the possible comeback.. (#3851, #1370)
- Added GetAllocatorFunctions() to facilitate sharing allocators accross DLL boundaries. (#3836)
- ImFontAtlas: Added 'bool TexPixelsUseColors' output to help backend decide of underlying texture format. (#3369)
This can currently only ever be set by the Freetype renderer.
Expand Down
26 changes: 18 additions & 8 deletions imgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3122,20 +3122,22 @@ bool ImGui::IsItemHovered(ImGuiHoveredFlags flags)
return IsItemFocused();

// Test for bounding box overlap, as updated as ItemAdd()
if (!(window->DC.LastItemStatusFlags & ImGuiItemStatusFlags_HoveredRect))
ImGuiItemStatusFlags status_flags = window->DC.LastItemStatusFlags;
if (!(status_flags & ImGuiItemStatusFlags_HoveredRect))
return false;
IM_ASSERT((flags & (ImGuiHoveredFlags_RootWindow | ImGuiHoveredFlags_ChildWindows)) == 0); // Flags not supported by this function

// Test if we are hovering the right window (our window could be behind another window)
// [2017/10/16] Reverted commit 344d48be3 and testing RootWindow instead. I believe it is correct to NOT test for RootWindow but this leaves us unable to use IsItemHovered() after EndChild() itself.
// Until a solution is found I believe reverting to the test from 2017/09/27 is safe since this was the test that has been running for a long while.
//if (g.HoveredWindow != window)
// return false;
if (g.HoveredRootWindow != window->RootWindow && !(flags & ImGuiHoveredFlags_AllowWhenOverlapped))
return false;
// [2021/03/02] Reworked / reverted the revert, finally. Note we want e.g. BeginGroup/ItemAdd/EndGroup to work as well. (#3851)
// [2017/10/16] Reverted commit 344d48be3 and testing RootWindow instead. I believe it is correct to NOT test for RootWindow but this leaves us unable
// to use IsItemHovered() after EndChild() itself. Until a solution is found I believe reverting to the test from 2017/09/27 is safe since this was
// the test that has been running for a long while.
if (g.HoveredWindow != window && (status_flags & ImGuiItemStatusFlags_HoveredWindow) == 0)
if ((flags & ImGuiHoveredFlags_AllowWhenOverlapped) == 0)
return false;

// Test if another item is active (e.g. being dragged)
if (!(flags & ImGuiHoveredFlags_AllowWhenBlockedByActiveItem))
if ((flags & ImGuiHoveredFlags_AllowWhenBlockedByActiveItem) == 0)
if (g.ActiveId != 0 && g.ActiveId != window->DC.LastItemId && !g.ActiveIdAllowOverlap && g.ActiveId != window->MoveId)
return false;

Expand Down Expand Up @@ -5014,6 +5016,8 @@ void ImGui::EndChild()
// Not navigable into
ItemAdd(bb, 0);
}
if (g.HoveredWindow == window)
parent_window->DC.LastItemStatusFlags |= ImGuiItemStatusFlags_HoveredWindow;
}
g.WithinEndChild = false;
g.LogLinePosY = -FLT_MAX; // To enforce a carriage return
Expand Down Expand Up @@ -7647,6 +7651,7 @@ void ImGui::BeginGroup()
group_data.BackupCurrLineSize = window->DC.CurrLineSize;
group_data.BackupCurrLineTextBaseOffset = window->DC.CurrLineTextBaseOffset;
group_data.BackupActiveIdIsAlive = g.ActiveIdIsAlive;
group_data.BackupHoveredIdIsAlive = g.HoveredId != 0;
group_data.BackupActiveIdPreviousFrameIsAlive = g.ActiveIdPreviousFrameIsAlive;
group_data.EmitItem = true;

Expand Down Expand Up @@ -7700,6 +7705,11 @@ void ImGui::EndGroup()
window->DC.LastItemId = g.ActiveIdPreviousFrame;
window->DC.LastItemRect = group_bb;

// Forward Hovered flag
const bool group_contains_curr_hovered_id = (group_data.BackupHoveredIdIsAlive == false) && g.HoveredId != 0;
if (group_contains_curr_hovered_id)
window->DC.LastItemStatusFlags |= ImGuiItemStatusFlags_HoveredWindow;

// Forward Edited flag
if (group_contains_curr_active_id && g.ActiveIdHasBeenEditedThisFrame)
window->DC.LastItemStatusFlags |= ImGuiItemStatusFlags_Edited;
Expand Down
8 changes: 5 additions & 3 deletions imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -697,12 +697,13 @@ enum ImGuiItemStatusFlags_
{
ImGuiItemStatusFlags_None = 0,
ImGuiItemStatusFlags_HoveredRect = 1 << 0,
ImGuiItemStatusFlags_HasDisplayRect = 1 << 1,
ImGuiItemStatusFlags_HasDisplayRect = 1 << 1, // LastItemDisplayRect is valid
ImGuiItemStatusFlags_Edited = 1 << 2, // Value exposed by item was edited in the current frame (should match the bool return value of most widgets)
ImGuiItemStatusFlags_ToggledSelection = 1 << 3, // Set when Selectable(), TreeNode() reports toggling a selection. We can't report "Selected" because reporting the change allows us to handle clipping with less issues.
ImGuiItemStatusFlags_ToggledOpen = 1 << 4, // Set when TreeNode() reports toggling their open state.
ImGuiItemStatusFlags_HasDeactivated = 1 << 5, // Set if the widget/group is able to provide data for the ImGuiItemStatusFlags_Deactivated flag.
ImGuiItemStatusFlags_Deactivated = 1 << 6 // Only valid if ImGuiItemStatusFlags_HasDeactivated is set.
ImGuiItemStatusFlags_Deactivated = 1 << 6, // Only valid if ImGuiItemStatusFlags_HasDeactivated is set.
ImGuiItemStatusFlags_HoveredWindow = 1 << 7 // Override the HoveredWindow test to allow cross-window hover testing.

#ifdef IMGUI_ENABLE_TEST_ENGINE
, // [imgui_tests only]
Expand Down Expand Up @@ -924,7 +925,7 @@ struct ImGuiStyleMod
};

// Stacked storage data for BeginGroup()/EndGroup()
struct ImGuiGroupData
struct IMGUI_API ImGuiGroupData
{
ImGuiID WindowID;
ImVec2 BackupCursorPos;
Expand All @@ -935,6 +936,7 @@ struct ImGuiGroupData
float BackupCurrLineTextBaseOffset;
ImGuiID BackupActiveIdIsAlive;
bool BackupActiveIdPreviousFrameIsAlive;
bool BackupHoveredIdIsAlive;
bool EmitItem;
};

Expand Down

0 comments on commit ee643b2

Please sign in to comment.