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.

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

Fix glitches when using ImGui in an app with multiple windows #2242

Merged
merged 1 commit into from May 10, 2021

Conversation

totalgee
Copy link
Contributor

@totalgee totalgee commented May 3, 2021

  • two calls to toPixels() were using the default App version (which
    uses the "current" Window to do the conversion), but the current
    Window is indeterministic when in an update handler, so with
    multiple windows, it is likely to reference the wrong one.
  • fixes Cinder issue ImGui glitches when using multiple Windows #2241.

- two calls to toPixels() were using the default `App` version (which
  uses the "current" Window to do the conversion), but the current
  Window is indeterministic when in an `update` handler, so with
  multiple windows, it is likely to reference the wrong one.
- fixes Cinder issue cinder#2241.
@totalgee
Copy link
Contributor Author

totalgee commented May 3, 2021

Note that there are some other calls to toPixels() in the Cinder ImGui code, e.g. in the various mouse and keyboard Event handlers. These seem to work fine (so I didn't change them, though they could easily be fixed by using event->getWindow()->toPixels()). Presumably the current window is correct in these callbacks, since you're directly interacting with them... (let me know if you want me to change these, too, I initially opted for the minimum fix that corrected my issue ;-) This also seems to be the case with the resize handler, although there we have no direct way to get the window, but it could easily be "fixed" by binding the window in the signal handler, similar to what is done for the update signal handler. Again, let me know if you want that change to be made.

@richardeakin
Copy link
Collaborator

richardeakin commented May 4, 2021

Nice find, this fix makes sense to me.

@richardeakin
Copy link
Collaborator

I think if it makes sense to you for these other winder->toPixel changes to happen it would be more robust and can review it, though for now happy to merge this.

@richardeakin richardeakin merged commit 51adfb1 into cinder:master May 10, 2021
totalgee added a commit to totalgee/Cinder that referenced this pull request May 10, 2021
@totalgee
Copy link
Contributor Author

I think if it makes sense to you for these other winder->toPixel changes to happen it would be more robust and can review it, though for now happy to merge this.

Okay, I decided to make these changes, since I was in the code, and if I don't do it now I'll forget about it. See #2243 (this one can stay closed). Thanks!

richardeakin added a commit that referenced this pull request Aug 29, 2021
Make ImGui multi-window more robust; fix issues noted in #2242
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants