Skip to content

Fix adding events to an inactive context#6256

Closed
cfillion wants to merge 3 commits intoocornut:dockingfrom
cfillion:fix-add-event-multictx
Closed

Fix adding events to an inactive context#6256
cfillion wants to merge 3 commits intoocornut:dockingfrom
cfillion:fix-add-event-multictx

Conversation

@cfillion
Copy link
Copy Markdown
Contributor

Version 1.89.4's release notes mention that events can now be added to any context:

IO: Lifted constraint to call io.AddEventXXX functions from current context. (#4921, #5856, #6199)

However this is broken:

  • FindLatestInputEvent called by most AddXXXEvent functions uses the current global context
  • AddKeyEvent calls ImGui::GetKeyData which also uses the current context
  • AddMouseViewportEvent from the docking branch still has the old "Can only add events to current context" assertion

There's probably a better solution for GetKeyData than temporarily changing the current context. This PR is based on the docking branch because of the last AddMouseViewportEvent fix. The first two commits cleanly apply to the current master branch.

@ocornut
Copy link
Copy Markdown
Owner

ocornut commented Mar 19, 2023

Thanks!
And ouch, i’ll work on basic tests for this too.

@ocornut
Copy link
Copy Markdown
Owner

ocornut commented Mar 21, 2023

There's probably a better solution for GetKeyData than temporarily changing the current context.

This is indeed problematic as is, as depending on GImGui storage type it may break the expectation that ImGuiIO AddEventXXX calls may be thread-safe.

The "right" solution would be adding a ctx pointer to GetKeyData() which is the crux of #5856, but this tends to drag a lots more stuff and I am still unsure of the preferable way to pass that param. On it.

@ocornut
Copy link
Copy Markdown
Owner

ocornut commented Mar 21, 2023

Merged 7269498 + e55a0ef
Docking branch includes the change for AddMouseViewportEvent() too.

EDIT Slight quirk merging but Docking is fixed with ad44f58

@ocornut ocornut closed this Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants