-
-
Notifications
You must be signed in to change notification settings - Fork 382
fix(WidgetManager): cross-renderer widget usage #3204
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
base: master
Are you sure you want to change the base?
fix(WidgetManager): cross-renderer widget usage #3204
Conversation
Widgets should not remain in an active state if interaction occurs outside of the containing renderer.
Hi @floryst, this seems to work great I am getting some weird behaviour with the widget disappearing and my screen zooming out in some situations, but I am 99.99% sure that it is because of my own flawed self-implemented zooming logic. In general, clicking the widget works as expected, and I'm no longer getting the freezes on Firefox/Windows Thank you so much, look forward to this being merged if no other issues are encountered |
@finetjul what do you think about this approach vs the one I mentioned in the PR description? |
@lahdemaki we actually discussed it recently with @floryst. We discussed a slightly different approach. @floryst have you had a chance to work on the approach ? |
Update: I have a slightly different approach in the https://github.com/Kitware/vtk-js/tree/add-renderer-change-event branch, which adds a RendererChange event. Whenever a widget detects rendererChange, it deactivates itself. This approach doesn't work well with the vtkPaintWidget. In the scenario with two renderers, if the mouse leaves the scene the paint widget deactivates, requiring users to re-activate the widget when re-entering the scene. @finetjul I think a better approach would be to have each of a widget's event handlers filter only on events targeting the associated view widget's renderer. What do you think? |
Has there been any further discussion on this? |
I'll pick this back up this week. I'm opting to instead filter events rather than the current implementation, because disabling widgets when changing renderer focus is not ideal. |
Context
Widgets should not remain in an active state if interaction occurs outside of the containing renderer.
An alternative to deactivating all widgets in the widget manager in such a scenario is to filter out interaction events at the widget level that are not targeting the widget's renderer. Such an alternative has the benefit of allowing widgets to remain active for future in-renderer interactions.
I'm still figuring out if this change might break any existing expectations.
Addresses #3144.
Results
Changes
PR and Code Checklist
npm run reformat
to have correctly formatted codeTesting