ENH: Enable right-click menu for fiducials #1253
Conversation
There was a problem hiding this comment.
Thank you, it looks really nice and it works well. It'll open up a lot of possibilities.
Since this feature is so powerful, there is a high chance that modules would want to customize what appears in the context menu. For example, I would add an "Edit properties..." menu item, but I guess that some modules or custom applications would not like to see this menu item. Do you have a preferred design for this? Maybe we could implement all menu items in markups plugin and use flags to configure which ones are shown. And for more customization, new plugin class (maybe derived from the default markups plugin class) for special markups could be created, which would display customized view menu (remove some too generic functions, add some application-specific ones)?
| @@ -29,6 +29,8 @@ set(${KIT}_TARGET_LIBRARIES | |||
| ${MRML_LIBRARIES} | |||
| ) | |||
|
|
|||
| set (${KIT}_INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR} CACHE INTERNAL "" FORCE) | |||
There was a problem hiding this comment.
Why is this needed? These include dirs should be added to cache automatically (you can see in Slicer-build\CMakeCache.txt that there are many ...ModuleLogic_INCLUDE_DIRS variables and they are not set using such code).
There was a problem hiding this comment.
Because the markup node header could not be included in the subject hierarchy plugin cxx without this.
There was a problem hiding this comment.
It is odd that this is needed, because even without this, my Slicer-build/CMakeCache.txt contains vtkSlicerMarkupsModuleMRML_INCLUDE_DIRS:INTERNAL=C:/D/S4/Modules/Loadable/Markups/MRML;C:/D/S4R/Slicer-build/Modules/Loadable/Markups/MRML.
It does not hurt to have this, so if you want you can go on and integrate as is, but it would be nice if you could try a clean build without this line and see if it fails.
There was a problem hiding this comment.
I started a clean build...
There was a problem hiding this comment.
It builds. I guess then probably there are several lines of the same kind in Slicer. In any case I removed this one.
@lassoan do you have any ideas what I can do to make it build without a clean build in general? Like re-run CMake or clean build just a few projects etc.
There was a problem hiding this comment.
I don't think anything can replicate the behavior of a clean build.
I only saw this kind of kit include dir caching line in Subject Hierarchy plugin and maybe one more place. They seemed to be special enough that I did not check if they were still needed, but you are right that those may be removed, too.
There was a problem hiding this comment.
This is probably an oversight. I suggest to remove it and if there are issues later one, I will have a look.
There was a problem hiding this comment.
For development it not acceptable having to do a complete 3-5 hour clean build when making such changes in CMake. There must be some kind of way to have a successful build without either adding this line or doing a full clean build...
There was a problem hiding this comment.
There is really no other way if you want to be 100% sure that you test a clean build without actually doing a clean build. If you are sure that your changes only affect Slicer then you may be able to delete just the Slicer binary folder and then VTK, CTK, etc., may not need to be rebuilt.
I have additional test builds set up for things like this, so I can continue my work, while another build is running in the background.
Modules/Loadable/SubjectHierarchy/Widgets/qSlicerSubjectHierarchyPluginLogic.cxx
Outdated
Show resolved
Hide resolved
Modules/Loadable/SubjectHierarchy/Widgets/qSlicerSubjectHierarchyPluginLogic.cxx
Outdated
Show resolved
Hide resolved
cpinter
force-pushed
the
right-click-in-views
branch
from
4131fd1
to
b36bd12
Compare
|
I like all the ideas about Edit properties and also the special markup types. Since this infrastructure will most probably remain the same, I think we could go ahead and integrate this. Once we have a concrete plan to extend it, I'm happy to help. |
I would recommend to have at least these actions:
Could you add them? It does not have to be in this pull request. It would be nice to have something like "Show in subject hierarchy tree", which would switch to Data module / Subject Hierarchy tab and select the node. |
|
OK I can add these easily. For Edit properties we can use the current infrastructure right? |
yes |
Right-clicking markups in views shows a context menu provided by subject hierarchy. The infrastructure to use the SH in-view context menu for other node types is also established. - Sink MenuEvent event from markups display node to display node base class - Display node MenuEvent handled in Subject hierarchy plugin logic. It shows a third type of SH menu called View menu, which offers actions that are applicable in the views - First plugin to support this is Markups that offers a menu to rename fiducial
cpinter
force-pushed
the
right-click-in-views
branch
from
b36bd12
to
5bd3982
Compare
|
This PR has been integrated in Slicer/Slicer@771bc70 I will push a second PR with the following changes:
|
|
For the record, the follow-up PR: Slicer/Slicer#1262 |
Right-clicking markups in views shows a context menu provided by subject hierarchy. The infrastructure to use the SH in-view context menu for other node types is also established.