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

Reconsider purpose and operation of Selection node #3551

Open
slicerbot opened this issue Mar 13, 2020 · 3 comments
Open

Reconsider purpose and operation of Selection node #3551

slicerbot opened this issue Mar 13, 2020 · 3 comments
Labels
Priority: Medium Issues that we plan to fix but there are available workarounds Type: Enhancement Improvement to functionality
Milestone

Comments

@slicerbot
Copy link
Collaborator

This issue was created automatically from an original Mantis Issue. Further discussion may take place here.

@lassoan lassoan added Priority: Medium Issues that we plan to fix but there are available workarounds Type: Enhancement Improvement to functionality labels Mar 18, 2020
@lassoan lassoan added this to the Backlog milestone Mar 20, 2020
@jamesobutler
Copy link
Contributor

It appears that @lassoan took some recent action on related stuff here by removing SetActiveVolumeNode/GetActiveVolumeNode in 72852d7

@lassoan
Copy link
Contributor

lassoan commented May 21, 2021

Removing Set/GetActiveVolume from Volumes logic was just removing unused code. The "active volume" ID is still stored in selection node as before.

A more closely related change is introduction of vtkMRMLApplicationLogic::EditNode(vtkMRMLNode* node) method. This method notifies the application GUI that a component requested editing of a node (by invoking an EditNodeEvent on the application logic, which is observed at the application level). This indirection is necessary because MRML widgets don't know about the application (they don't know how to a node can be edited in the application). Very similarly, a ViewNodeEvent could be invoked whenever a MRML core or widget component would like to request the application to show a node. This would be a simpler mechanism and would not result in Active...NodeID values lingering around in the selection node (which are really only meaningful for the time when Propagate...Selection methods are called).

Probably this mechanism could be replaced when slice viewers will be updated to be able to show any number of volume layers, because then notion of foreground/background/label layers will largely go away (layers will not be limited to these 3 anymore).

@pieper
Copy link
Member

pieper commented May 21, 2021

Yes, there's definitely room for improvement in this. We should find a time to discuss these changes in a developer call, or maybe a project week breakout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium Issues that we plan to fix but there are available workarounds Type: Enhancement Improvement to functionality
Development

No branches or pull requests

4 participants