Add SlicerVESPA extension

[AlexyPellegrini]

New extension

• Extension has a reasonable name (not too general, not too narrow, suggests what the extension is for)
• Repository name is Slicer+ExtensionName
• Repository is associated with 3d-slicer-extension GitHub topic so that it is listed here. To edit topics, click the settings icon in the right side of "About" section header and enter 3d-slicer-extension in "Topics" and click "Save changes". To learn more about topics, read https://help.github.com/en/articles/about-topics
  • N/A will be host on Gitlab
• Extension description summarizes in 1-2 sentences what the extension is usable (should be understandable for non-experts)
• Any known related patents must be mentioned in the extension description.
• LICENSE.txt is present in the repository root. MIT (https://choosealicense.com/licenses/mit/) or Apache (https://choosealicense.com/licenses/apache-2.0/) license is recommended. If source code license is more restrictive for users than MIT, BSD, Apache, or 3D Slicer license then the name of the used license must be mentioned in the extension description.
• Extension URL and revision (scmurl, scmrevision) is correct, consider using a branch name (main, release, ...) instead of a specific git has to avoid re-submitting pull request whenever the extension is updated
• Extension icon URL is correct (do not use the icon's webpage but the raw data download URL that you get from the download button - it should look something like this: https://raw.githubusercontent.com/user/repo/main/SomeIcon.png)
• Screenshot URLs (screenshoturls) are correct, contains at least one
• Homepage URL points to valid webpage containing the following:
  • Extension name
  • Short description: 1-2 sentences, which summarizes what the extension is usable for
  • At least one nice, informative image, that illustrates what the extension can do. It may be a screenshot.
  • Description of contained modules: at one sentence for each module
  • Tutorial: step-by-step description of at least the most typical use case, include a few screenshots, provide download links to sample input data set
  • Publication: link to publication and/or to PubMed reference (if available)
  • License: We suggest you use a permissive license that includes patent and contribution clauses. This will help protect developers and ensure the code remains freely available. We suggest you use the Slicer License or the Apache 2.0. Always mention in your README file the license you have chosen. If you choose a different license, explain why to the extension maintainers. Depending on the license we may not be able to host your work. Read here to learn more about licenses.
  • Content of submitted s4ext file is consistent with the top-level CMakeLists.txt file in the repository (description, URLs, dependencies, etc. are the same)
• Hide unused features in the repository to reduce noise/irrelevant information:
  • Click Settings and in repository settings uncheck Wiki, Projects, and Discussions (if they are currently not used)
    • N/A will be host on Gitlab
  • Click the settings icon next to About in the top-right corner of the repository main page and uncheck Releases and Packages (if they are currently not used)
    • N/A will be host on Gitlab

[pieper]

Does this extension build C-GAL? If so I understand that would make the extension GPL.

[vollia]

Dear @pieper ,

The SlicerVESPA extension has its own license, under the BSD3 terms. The code of this extension is only under this license.
As this extension build against CGAL, the resulting executable is subject to both the BSD3 license of the extension AND the GPLv3 due to the CGAL library. So the resulting executable is indeed under the GPL, but not the extension per sei.

Also, if someone has the commercial license of CGAL (it is under dual licensing) and build the extension, the resulting executable may not be GPL any more as the terms of the commercial license will replace it).

[pieper]

Thanks for the clarification @CharlesGueunet . As a rule we don't rely on GPL code for Slicer application (e.g. see this discussion). Is there a way the GPL code could be build as executables that are independently downloaded?

[lassoan]

If we only build CLI executables (ensured by the EXECUTABLE_ONLY option) then the GPL requirements are fulfilled. I don't think that installing the extension would restrict usage and distribution of Slicer.

We just need to clearly document in the license file, extension description, and readme file that GPL code is used internally because most companies would want to avoid any relationship with any copyleft-licensed code.

[lassoan]

I see only one limitation with this, which may be very significant for medical applications: tivoization. I.e., you can only use the CGAL executables on the system if the user can replace them. But in a regulatory approved medical device it may not be feasible to give the user such low-level access.

So, the non-permissive license warnings should be very clear and visible, and we should work towards replacing CGAL with software that uses permissive license (Boolean operation -> vtkbool, IsotropicRemesher -> ACVD, AlphWrapping -> WrapSolidify using ACVD for remeshing; subidivision, filling, and smoothing tools in VTK are probably about as good as CGAL).

[vollia]

If my understanding is correct, Vespa (and the related CGAL) will already be contained in a specific CLI with its specific libraries. Slicer will call them but will not be impacted by the GPL.

you can only use the CGAL executables on the system if the user can replace them.

I am not sure what you means here. Can you provide an example of what such a replacement would be ?

We just need to clearly document in the license file, extension description, and readme file that GPL code is used internally because most companies would want to avoid any relationship with any copyleft-licensed code.
...
So, the non-permissive license warnings should be very clear and visible

We can improve the documentation on VESPA and on the extension as you need. The readme of the VESPA project already states this on the first lines.

I think some of the questions remarks may be shorter to discuss in a quick meet (I will write a summary here after), @lassoan do you think we can schedule a 20 min meet ?

[lassoan]

The issue is that gpl3 prohibits tivoization. In short: if the application is not linked to gpl3 code (so the user could replace/modify the gpl3 component) but the user has no access to the file system from outside then the gpl3 requirements are not fulfilled. For example, you cannot modify program files on a CT scanner, because the system boots directly into the application and you cannot exit the application or switch to another. Only the manufacturer's service engineers can modify program files on the system.

Can you create a doodle poll and post the link? It would be nice if @pieper could join the meeting, too.

[vollia]

Here is a poll to find the best meeting time, everyone that would like to join can fill it:

https://www.when2meet.com/?19411506-Z8cWU

[lassoan]

It seems that next Thursday would be the first day when all interested people could join a meeting. We could discuss the overall long-term strategy.

In the meantime we can probably continue the discussion of specific questions here. For example, I'll review and test the extension and give feedback or ask questions.

[lassoan]

I've tested the extension on Windows. Build was successful and all modules worked vrery nicely. Even if somebody cannot use the modules due to licensing restrictions, it is useful to have them as a reference (e.g., see if vtkbool is as robust as CGAL's Boolean operations, compare speed and quality of ACVD remesher to CGAL, see if shrinkwrapping with vtkSmoothingFilter+ACVD is as good as Alpha wrapping, etc.).

A few questions/issues:

• It seems that the extension is not supported on macOS (for example, the .so file filename is hardcoded). Has the build tested on macOS?
• Add a note to the CMakeLists.txt that explains why CGAL-5.5.1-win64-auxiliary-libraries-gmp-mpfr.zip binaries (libmpfr-4.dll, libgmp-10.dll) are used instead of building from source. Building from source and static linking would be safer, because who knows what Python packages or other extensions bring in incompatible versions of these dlls.
• Why ligmp, libmpfr symlinks were necessary?
• It would be better to follow the superbuild template. It may mean that more targets are build than needed (unless you make it configurable what parts of CGAL are built), but that's much smaller issue than future maintenance issues (in 5 years you may not be around and somebody may need to update the extension to work with new CMake or VTK version or Python wrapper and then this extension may take 10x more effort to update, just because the first thing the maintainer has to do is to understand why this build structure is different from all other extensions and how differently it should be updated. If the SlicerVESPA build method is much better than the template then we need to update the template and most other extensions.
• libgmp-10.dll and libmpfr-4.dll are installed into the module folder. This is a problem, because all DLLs in that folder are expected to be modules. These DLLs should be installed in the Slicer_THIRDPARTY_BIN_DIR. (See for example https://github.com/IGSIO/SlicerIGSIO/blob/master/SuperBuild/External_IGSIO.cmake)
• For each module, in the CLI module XML file and in the README.md file, describe what the module is for (not everyone knows what "alpha wrapping" it is for in a few words and add a link to the algorithm (https://doc.cgal.org/latest/Alpha_wrap_3/index.html)

[AlexyPellegrini]

It seems that the extension is not supported on macOS (for example, the .so file filename is hardcoded). Has the build tested on macOS?

No, I have not tried to port it to Mac at all, but it shouldn't be hard to adapt windows specific code

Add a note to the CMakeLists.txt that explains why CGAL-5.5.1-win64-auxiliary-libraries-gmp-mpfr.zip binaries (libmpfr-4.dll, libgmp-10.dll) are used instead of building from source. Building from source and static linking would be safer, because who knows what Python packages or other extensions bring in incompatible versions of these dlls.

Building GMP and MPFR on Windows using MSVC is a nightmare, that why CGAL provides pre-built packages. The only real way to build them on Windows would be to rely on MSYS or Cygwin, but I don't want to constrain users to have them installed. I agree that a comment should be added to explain this.

Why ligmp, libmpfr symlinks were necessary?

Linker search for ligmp.so.6 and I didn't want to lose time for this so I just added symlinks. If you know a better, easier, solution, please let me know. It is probably possible to tweak the linker to search for the full name.

It would be better to follow the superbuild template.

I followed the template, at least I think I did. What is different ?

libgmp-10.dll and libmpfr-4.dll are installed into the module folder. This is a problem, because all DLLs in that folder are expected to be modules. These DLLs should be installed in the Slicer_THIRDPARTY_BIN_DIR.

I will fix that, thanks for the input.

For each module, in the CLI module XML file and in the README.md file, describe what the module is for (not everyone knows what "alpha wrapping" it is for in a few words and add a link to the algorithm

Yes, there is a lack of documentation in both the CLI module, but also VESPA.

[vollia]

It seems that next Thursday would be the first day when all interested people could join a meeting. We could discuss the overall long-term strategy.

Here is a link to a google event including the google meet room, if everyone is ok with that technology (more convenient to setup on our side).

Also, concerning vtkbool, I can ensure it is not as robust as CGAL. Using exact computation, CGAL is able to give a clean and exact cut/merge even with intricate concave shapes. Maybe some other project may be considered ?

[pieper]

The Clipper2 option sounds worth exploring. If it works well enough, a clean new implementation would be way better than perpetuating a GPL'd codebase with nightmare build systems. I agree though that having something that works well enough for now is better than not having it, but providing clear explanations of the limitations will be important.

[jcfr]

Last week I spent some time fixing up the build-system:

• https://gitlab.kitware.com/vtk-cgal/SlicerVESPA/-/merge_requests/2
• https://gitlab.kitware.com/vtk-cgal/vespa/-/merge_requests/40

More work is needed, especially in the vtk-cgal/vespa project where the generation of the config files need to be improved. Ideally support for externally building VTK modules should be supported by VTK proper, in the meantime logic from VTKExternalModule¹ may be re-used

Footnotes

1. https://github.com/KitwareMedical/VTKExternalModule ↩

[jcfr]

the google meet room

I will not be able to join the meeting.

[jcfr]

Waiting the build-system updates are finalized:

• https://gitlab.kitware.com/vtk/meshing/SlicerVESPA/-/merge_requests/3

[jcfr]

Unable to force push changes onto AlexyPellegrini/ExtensionsIndex@vespa-cli, the updated changes are published here main...jcfr:ExtensionsIndex:AlexyPellegrini-vespa-cli

Consider running the following command to update this pull request:

git clone git@github.com:AlexyPellegrini/ExtensionsIndex.git
cd ExtensionsIndex
git checkout vespa-cli
git remote add jcfr https://github.com/jcfr/ExtensionsIndex.git
git fetch jcfr
git reset --hard jcfr/AlexyPellegrini-vespa-cli
git push origin vespa-cli --force

[jcfr]

re: Update

Force pushing the topic is not possible. Consider rebasing.
See #1925 (comment)

[jcfr]

re: Update

Force pushing the topic is not possible. Consider rebasing 🙏

[jcfr]

re: Update

Force pushing the topic is not possible. Consider rebasing 🙏

[lassoan]

Due to the requests/questions not answered, I'm closing this issue now. It can be reopened anytime after the requested updates are completed.

[mauigna06]

I'd like to use SlicerVESPA's boolean operations on BoneReconstructionPlanner. I was able to create a repo named SlicerVESPA also that only exposes the boolean operations algorithms of CGAL, the implementation of this PR is more complete (exposing most CGAL's algorithms of interest for meshes) and not done with AI as mine.
How can I help to get the pull request merged?

[lassoan]

@mauigna06 please have Claude review and fix up the extension (address the problems that I described above) and test if it works well for you. After that, submit a pull request to the upstream repository. If the pull request does not get merged in 1-2 weeks then you can fork the repository and we'll update the URL in this pull request.. After that we can merge this pull request.

[AlexyPellegrini]

@jcfr started some of the work on remaining issues, you can find them in the two merge requests on SlicerVESPA GitLab repository https://gitlab.kitware.com/vtk/meshing/SlicerVESPA/-/merge_requests

[mauigna06]

My version of SlicerVESPA is already superbuilt and compiles well with its own Eigen, BOOST and CGAL. So I could also try to expose the missing algorithms (the others that are not boolean operations) to have CLI modules too. I do not have a preference between Alexy version or mine, but I think mine looks more similar to what you suggested Andras. I await your instruction on what to do next.

[lassoan]

https://gitlab.kitware.com/vtk/meshing/SlicerVESPA covers more features, so it would be better if you could combine it with your build script improvements.

@AlexyPellegrini what do you think? Would you be open to accept a merge request with build script changes? Would you be able to provide maintenance and user support for this extension?

[AlexyPellegrini]

Re, their are differences between our implementation and @mauigna06's one.
We did wrap the filters as CLI without any preprocessing while proposed implementation does lots of sanity checks and auto-corrections. This will obviously lead to differences that might be important for your use case.

Otherwise we always accept contributions, and it might be cleaner to keep all VESPA related repos in the VTK namespace on our gitlab. Starting from JCFR drafts should make integration easier.

I can not answer the maintenance and support question. I defer to @finetjul and @Thibault-Pelletier. Note that they are both on vacations so they will likely answer in a few weeks.

[mauigna06]

Ok. I've made a merge request where you suggested Alexy: https://gitlab.kitware.com/vtk/meshing/SlicerVESPA/-/merge_requests/4 and I started my implementation from JC's draft

The code compiles on Linux 22 and all 6 exposed modules work inside Slicer