-
Notifications
You must be signed in to change notification settings - Fork 174
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.
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
Race condition with Window.hasFocus() #206
Comments
I didn't fully understand the symptoms. This is the important part:
Not recorded for how long, or until what happens? How haven't others noticed this? |
Apologies, I should have been more clear.
When this bug is triggered, most sf::Events will not be forwarded to imgui until the user does something to cause an sf::Event::LostFocus, and then an sf::Event::GainedFocus. In other words, when this bug is triggered the user needs to do something like click outside of the window and then click back onto the window, or alt tab in order to get events to propagate from sfml to imgui. The following events are not forwarded to imgui when the bug is triggered:
This is because these events are all wrapped an an if statement checking whether WindowContext.windowHasFocus is true. Check the file imgui-SFML.cpp in the function processEvent on line 556 to see.
They are not recorded until the user does something to reset the variable windowHasFocus, such as click outside of the window or alt tab.
I think it's hard to trigger this on non-M1 macs. The initial problem was reported by someone with an M1 mac. Another M1 mac user was able to replicate the hasFocus() returning false behavior saying the following
It seems the way you start the app on an M1 mac also plays a role. Quoting discord again
A third user on windows also reported that hasFocus can initially report false, though this one is more involved. Apparently if you start a fullscreen window but click in the time between the app launch and the window creation it will initially report false before returning true if I understand correctly. I may have been mistaken when I originally wrote the issue, this may be the only way to trigger this on windows. I myself cannot replicate Window.hasFocus() -> false on initial creation, but I'm on Linux. |
So
|
I feel like it would be easier to just ignore Gain/Lost focus events and let the user handle them manually I feel like this will make ImGui-SFML's code much easier and predictable. What do you think? |
I think that makes a lot of sense. Like you said, it'd fix the bug and simplify the code. For me personally (and I imagine others) this code is redundant as I still need to handle suspending the code for the entire app when LostFocus, having imgui suspend itself is like is stopping a problem in a river-fork downstream rather than at the source. I could be wrong here but it looks like with the way imgui currently suspends itself (with the check for lost focus / gain focus at the end of the processEvent function) the first click back which gains focus will never be processed by imgui. If so then you currently can't click a button when gaining focus with a mouseclick, which may be undesirable behavior for some. |
Workaround for those (like me) who consume it via a package manager and don't want to touch the sources. |
imgui-sfml relies on the false assumption that Window.hasFocus() will report true immediately after creation. Unfortunately, there is a short period of time after window creation where Window.hasFocus() is not yet true. This is the case for both macOS, Windows, and likely Linux though not confirmed there.
This results in an intermittent bug where input such as mouse clicks will be skipped / not recorded.
This was an issue reported by l04 on the SFML Discord. You can find a (spread out making it somewhat hard to follow) conversation about the issue starting here,
https://discord.com/channels/175298431294636032/175298431294636032/972715469310148658
Quoting l04 here
A simple fix would be to set the windowHasFocus variable to true immediately on init rather than checking with the function call, though this is admittedly an ugly fix and I'm not sure that this is applicable in all cases. For example in the case of multiple windows.
The text was updated successfully, but these errors were encountered: