Skip to content

Prevent UI events from propagating beyond imgui#569

Merged
Korijn merged 4 commits intopygfx:mainfrom
panxinmiao:event_propagation
Sep 9, 2024
Merged

Prevent UI events from propagating beyond imgui#569
Korijn merged 4 commits intopygfx:mainfrom
panxinmiao:event_propagation

Conversation

@panxinmiao
Copy link
Copy Markdown
Contributor

After a UI event is handled by imgui, we generally don't want the event to be further processed by subsequent handlers (e.g., Pygfx).

For instance, in the following case, it is quite inconvenient

1.mp4

This PR allows imgui to capture canvas UI events and prevent them from further propagation, as shown below:

2.mp4

PS: The implementation might be a bit hacky, 😅 , and I’d be happy to hear your thoughts or suggestions.

@panxinmiao panxinmiao requested a review from Korijn as a code owner September 5, 2024 07:58
Copy link
Copy Markdown
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the general approach is good. I made a few comments.

We should also document this as part of the jupyter_rfb event spec, but I can do that later.

Comment thread wgpu/gui/base.py Outdated
Comment thread wgpu/gui/base.py Outdated
Comment thread wgpu/utils/imgui/imgui_renderer.py
@kushalkolar
Copy link
Copy Markdown
Contributor

kushalkolar commented Sep 5, 2024

Yay! Related: pygfx/pygfx#832

Nice that this PR seems to fix it in a general way

@panxinmiao
Copy link
Copy Markdown
Contributor Author

Updated. :)

@Korijn
Copy link
Copy Markdown
Collaborator

Korijn commented Sep 6, 2024

  • It's indeed a bit hacky to use this order argument and just fix the imgui renderer on -99... I wonder what a more general solution would look like. Is purely insertion order and the stop_propagation feature not enough?

If not, I'm fine with merging this.

@panxinmiao
Copy link
Copy Markdown
Contributor Author

panxinmiao commented Sep 9, 2024

  • It's indeed a bit hacky to use this order argument and just fix the imgui renderer on -99... I wonder what a more general solution would look like. Is purely insertion order and the stop_propagation feature not enough?

We need to provide a way to control the priority order of UI event propagation. For example, we would generally expect Imgui's event handler to be at the front of the queue. However, we cannot guarantee the order in which users initialize the ImguiRenderer and Pygfx Renderer, so we cannot control the insertion order in which the event handlers are added.

Copy link
Copy Markdown
Member

@almarklein almarklein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good. I can't think of a better way to control the order.

We can merge this before #573, so it ends up in the 0.17 release. Otherwise it will be part of a next release, which is also fine.

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.

4 participants