Slicer icon indicating copy to clipboard operation
Slicer copied to clipboard

Extract MRML source code and create a dedicated repository

Open jcfr opened this issue 5 years ago • 8 comments

Is your feature request related to a problem? Please describe.

Re-using Libs/MRML in other projects is challenging as it can not be built independently.

Describe the solution you'd like

Similarly to what was done for vtkAddon, a dedicated repository vtkSegmentationCore will be created in the Slicer GitHub organization.

Process is documented in this commit

Additional context

See https://github.com/Slicer/Slicer/pull/4765#issuecomment-603969354

jcfr avatar Mar 31 '20 21:03 jcfr

To simplify the distribution of MRML-based capabilities in an independent package, we should split MRMLCore into two libraries with MRMLCore depending on MRMLBase.

For example, functionality specific to Slicer based on vtkITK wouldn't be factored out into the independent MRML project.

jcfr avatar Feb 29 '24 19:02 jcfr

I like the idea but are core and base clear enough? Maybe something like MRMLITK to be explicit about what dependency is being introduced.

pieper avatar Feb 29 '24 19:02 pieper

Waiting we settle on a more definitive naming, we could indeed look into introducing MRMLITK at the Slicer level

jcfr avatar Feb 29 '24 19:02 jcfr

For example, functionality specific to Slicer based on vtkITK wouldn't be factored out into the independent MRML project

Why vtkITK would be excluded? It contains really useful classes. It is a pain to use ITK in C++, and vtkITK makes it really easy.

What is the plan for making those classes available in the new MRML package that depend on vtkITK?

lassoan avatar Mar 02 '24 05:03 lassoan

Based on our last discussion, didn't we mention we would keep these in Slicer along with keeping the MRML classes using ITK inside MRMLCore ?

The set of classes that would be factored out into the independent MRML package would then depend on ITK.

jcfr avatar Mar 02 '24 05:03 jcfr

Since there are already two flavors of vtkITK bridges that need to be consolidated, avoiding to introduce yet another one would be sensible:

  • https://github.com/InsightSoftwareConsortium/ITKVtkGlue
  • https://github.com/InsightSoftwareConsortium/ITK/tree/master/Modules/Bridge/VtkGlue

jcfr avatar Mar 02 '24 05:03 jcfr

I believe vtkITK plays a different role than the other to methods you pointed to @jcfr - they both provide classes for converting between vtkImageData and itk::Image so you can have a mixed pipeline. The vtkITK classes use itk internally but expose a pure vtk interface making them cleaner to use in vtk based code like MRML.

The ones in vtkITK that I would like to see retired are the vtkITKArchetype*Reader classes, which are very complex and mainly support uses that we don't encourage, like reading dicom directly or interpreting jpg stacks as volumes where we have better dedicated tools. The transform code is probably the best argument for keeping vtkITK code in MRML.

pieper avatar Mar 02 '24 16:03 pieper

Fully agree with @pieper in all points.

I would just add that a useful aspect of vtkITK is that it connects the ITK filters to the VTK pipeline in a very easy-to-use way, but - perhaps even more important - feature is that it removes all the unnecessary template usage from the public API of ITK filters. Templates are amazing - for ITK developers; but all those templates on the public API are just terrible for ITK users. This problem was solved by SimpleITK and Python wrapping, but vtkITK still has an important role: vtkITK makes ITK available to VTK users in the simplest, most familiar way, in both C++ and Python.

If we wanted to replace vtkITK then it could be possible by adding VTK/ITK glue to SimpleITK and make it generate VTK classes.

lassoan avatar Mar 03 '24 03:03 lassoan