-
Notifications
You must be signed in to change notification settings - Fork 172
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
Don't care about focus when processing events / updating #227
Conversation
Thanks. But sadly, this does not work well 😞 Consider the situation when another window (ideally a translucent one) is active and is partially covering the imgui window.
|
How does SDL backend manage it? Does it keep track of which window is active? And can’t it be left to the user? E.g. making sure to only call ProcessEvent of the currently focused window, but not others? |
SDL backend uses a combination of SDL_GetMouseFocus() + SDL_HINT_MOUSE_FOCUS_CLICKTHROUGH to check whether a window is hovered without focus, but this isn't something SFML supports so I don't think there's anything you can do about it To me this change seems sensible - windows don't get events anyway when they aren't focussed so adding extra checks in your code is redundant and just adds risk of issues (as reported) |
I don't think it's OK that clicking on an unrelated application will send clicks to an imgui-sfml application that happens to be hidden behind it. |
How does this happen? SFML only receives events if the window has focus |
Just run it and see for yourself if you don't believe |
for (unsigned int i = 0; i < 3; i++) { | ||
io.MouseDown[i] = s_currWindowCtx->touchDown[i] || sf::Touch::isDown(i) || | ||
s_currWindowCtx->mousePressed[i] || | ||
sf::Mouse::isButtonPressed((sf::Mouse::Button)i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oprypin I see what you're saying now - it's because this directly queries the mouse state here which bypasses the event queue.
Ideally think this should use events instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it looks like this is what causes problems, thanks.
I guess I should really rework how ProcessEvent/Update/Render etc. work and not use any direct queries since they don't respect window focus.
Of course, maybe if I check s_currWindowCtx.window->hasFocus()
, it might solve the problem, but it feels somewhat hacky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, this doesn't solve the problem for hover events... Sad.
I guess hover events happen because we're changing |
Yeah I don’t think you can do anything about hovering on partially visible windows unless you write the platform implementations for it yourself. If you just change to use events then users would still be able to implement this themselves if desired |
Fixes #212, #206, #88
Now the user has to not call Update/ProcessEvent on lost focus manually.
Might break some things (e.g. trigger some asserts if the user didn't call NewFrame/Render on focus lost).
@oprypin @ChrisThrasher, please review