Skip to content

ENH: Update MRML interactor style to explicitly use the poked renderer#7315

Merged
jcfr merged 1 commit intoSlicer:mainfrom
jcfr:vtkMRMLThreeDViewInteractorStyle-explicit-use-of-poked-renderer
Oct 30, 2023
Merged

ENH: Update MRML interactor style to explicitly use the poked renderer#7315
jcfr merged 1 commit intoSlicer:mainfrom
jcfr:vtkMRMLThreeDViewInteractorStyle-explicit-use-of-poked-renderer

Conversation

@jcfr
Copy link
Member

@jcfr jcfr commented Oct 30, 2023

Instead of relying on the CurrentRenderer ivar internally updated by the "vtkInteractorStyle::FindPokedRenderer()", we explicitly retrieve and use the poked renderer with "vtkRenderWindowInteractor::FindPokedRenderer()"

Instead of relying on the CurrentRenderer ivar internally updated by
the "vtkInteractorStyle::FindPokedRenderer()", we explicitly retrieve and
use the poked renderer with "vtkRenderWindowInteractor::FindPokedRenderer()"
Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

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

It's not clear to me what is expected to behave differently after this change, but it seems logical.

@jcfr
Copy link
Member Author

jcfr commented Oct 30, 2023

Thanks for the review. This is helpful with upcoming change related to the refactoring of the event delegation.

@jcfr jcfr merged commit 9f95ec7 into Slicer:main Oct 30, 2023
@jcfr jcfr deleted the vtkMRMLThreeDViewInteractorStyle-explicit-use-of-poked-renderer branch October 30, 2023 18:29
@lassoan
Copy link
Contributor

lassoan commented Oct 30, 2023

There are several renderers drawing into the same render window area: the one that draws the 3D scene and those that display the ruler, orientation marker, markup labels, etc. For most cases, we should probably use the renderer that is explicitly set in the interactor style, not the one that is found by FindPokedRenderer.

Check out implementation of vtkRenderWindowInteractor::FindPokedRenderer. It just returns the first "interactive" renderer that draws in that region. Are you sure that it is the correct one?

What was the problem with the current behavior? In what cases you observed that the poked renderer was different from the current renderer? In those cases, was it correct to use the poked renderer?

Please test the new implementation very thoroughly when you operate in areas over the orientation marker, ruler, and annotation markup labels and interaction handles.

@cpinter
Copy link
Member

cpinter commented Nov 3, 2023

@jcfr These are very good points by @lassoan above; just pinging in case it went unnoticed.

@jcfr jcfr added the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Nov 5, 2023
@jcfr jcfr removed the backport:5.x Identify pull request expected to be backported to the current 5.x release branch. label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants