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

OpenPopup after BeginPopup fails when no windows are focused #4308

Closed
cfillion opened this issue Jul 10, 2021 · 2 comments
Closed

OpenPopup after BeginPopup fails when no windows are focused #4308

cfillion opened this issue Jul 10, 2021 · 2 comments
Labels
bug popups

Comments

@cfillion
Copy link
Contributor

cfillion commented Jul 10, 2021 •
edited
Loading

Version/Branch of Dear ImGui:

Version: 1.83
Branch: master and docking

My Issue:

Popups fail to open when OpenPopup is called after BeginPopup and there are no focused windows. (EDIT: To be precise, it works as expected either if OpenPopup is used before the BeginPopup, or if any window has focus.)

This appears to be because NewFrame calls ClosePopupsOverWindow even when g.NavWindow is NULL.

imgui/imgui.cpp

Line 4105 in 5dd1e38

ClosePopupsOverWindow(g.NavWindow, false);

Screenshots:

Demo > Popups & Modal windows > Context menus

Demo > Widgets > Basic

Standalone, minimal, complete and verifiable example:

ImGui::Begin("Example Bug");
if(ImGui::BeginPopup("popup")) {
  ImGui::Text("Contents");
  ImGui::EndPopup();
}
ImGui::Text("right-click me");
ImGui::OpenPopupOnItemClick("popup", ImGuiPopupFlags_MouseButtonRight);
ImGui::End();
@ocornut
Copy link
Owner

ocornut commented Jul 12, 2021

Thank you, I could confirm the bug.
With the calls done in this order, the window doesn't have time to get focus and popup gets culled on next frame.

@ocornut ocornut mentioned this issue Jul 14, 2021
Closed
rokups added a commit to rokups/imgui that referenced this issue Jul 15, 2021
@rokups
Popups: Fix OpenPopup() being called after BeginPopup() resulting in …
c408d56 
…a failure to open a popup. (ocornut#4308)

Story of removed line begins in commit b80cf0a. It's purpose was to close popups when they lost focus.
Later on few other changes were introduced:
* bcc49ff - closing popups with RMB without changing window focus
* af679a1 - closing popups in FocusWindow()
These two changes covered all the cases of popup closing which made deleted line obsolete. Conveniently, now all popup closing is handled from within UpdateMouseMovingWindowEndFrame() either by calling FocusWindow() or ClosePopupsOverWindow().
rokups added a commit to rokups/imgui that referenced this issue Jul 22, 2021
@rokups
Popups: Fix OpenPopup() being called after BeginPopup() resulting in …
5955dfb 
…a failure to open a popup. (ocornut#4308)

Story of removed line begins in commit b80cf0a. It's purpose was to close popups when they lost focus.
Later on few other changes were introduced:
* bcc49ff - closing popups with RMB without changing window focus
* af679a1 - closing popups in FocusWindow()
These two changes covered all the cases of popup closing which made deleted line obsolete. Conveniently, now all popup closing is handled from within UpdateMouseMovingWindowEndFrame() either by calling FocusWindow() or ClosePopupsOverWindow().
ocornut pushed a commit that referenced this issue Jul 26, 2021
@rokups @ocornut
Popups: Fix OpenPopup() being called after BeginPopup() resulting in …
3e4c890 
…a failure to open a popup when focus is NULL. (#4308)

Story of removed line begins in commit b80cf0a. It's purpose was to close popups when they lost focus.
Later on few other changes were introduced:
* bcc49ff - closing popups with RMB without changing window focus
* af679a1 - closing popups in FocusWindow()
These two changes covered all the cases of popup closing which made deleted line obsolete. Conveniently, now all popup closing is handled from within UpdateMouseMovingWindowEndFrame() either by calling FocusWindow() or ClosePopupsOverWindow().
@ocornut
Copy link
Owner

ocornut commented Jul 26, 2021

Should be fixed by 3e4c890
(We're not currently 100% sure this won't have another side effect, I guess we'll find out if it does)

@ocornut ocornut closed this as completed Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug popups
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cfillion @ocornut