Greg: no patch is more urgent than the other. It just simplifies our work on Plastimatch by not having too many local changes.
Jc: Plastimatch is still not working because Plastimatch CLIs depend on a plastimatch.dll library

Hi Jc,

I think I mentioned in previous emails that the patch isn't acceptable. It breaks CUDA and OpenMP, and there is no description of the problem that it is supposed to fix.

Plastimatch builds fine for me using the out-of-directory build method. It would be really helpful if you could tell me the build procedure that you use to make the extensions. Then I can reproduce your problem and make a patch.

If that is not possible, maybe you can paste in your error messages?

Thanks!
Greg

Hi again,

I just saw the comment about DLL. Sorry, I missed that.

Is it sufficient to add an install target to the lib/Slicer-4.1/cli-modules and lib/Slicer-4.1/qt-loadable-modules directories?

Edit: I just submitted a patch which does this.

-Greg

Hi Greg,

Since I didn't work on Plastimatch myself (Julien did), I am sure he will be able to provide more comments.

I would recommend the following approach:
1) apply the patch provided by Julien
2) improve the CMake code so that Cuda/OpenMP are either optional or build within the extension using ExternalProjects. You could probably have a look at the example of SuperbuildExtension. See [1]

Hth
Jc

[1] https://github.com/Slicer/Slicer/tree/master/Extensions/Testing/SuperBuildLoadableExtensionTemplate

Hi, Thank you very much for your suggestion on Tuesday Jc.

I performed the following procedure on win32 platform:

(1) Configure plastimatch with -DSlicer_DIR option.
(2) Build solution with release target.
(3) Build PACKAGE target.
(4) Unzip the resulting zip file.
(5) Start slicer 4.
(6) Add cli-modules subdirectory as unzipped as an extra module path.
(7) Restart slicer.

Plastimatch module appears in the list. I did only superficial testing after that, but because the GUI is created correctly, it seems that slicer can find the DLLs OK.

Greg

That's a good news:)

If you provide me with a path to the source (URL and revision), I could build and verify on my side. If everything works as expected, I could add the extension to 4.1.1 release.

Hi, I just tested on linux, using the same procedure, and got a good result as well. :)

Here are scm download info. Revision can be HEAD, or if you recommend fixing a numeric revision, please use current HEAD which is 3607.

scm svn
scmurl http://forge.abcd.harvard.edu/svn/plastimatch/plastimatch/trunk

I tested building from s4ext, but it does not currently work for me. My repository requires username "anonymous", password is "". In Slicer 3 we used the following s3ext tags:

svnusername anonymous
svnpassword ""

I briefly looked at the Slicer 4 build scripts, and couldn't find a way to send the username and password. Is my understanding correct? If so, here are two ideas for fixing this:

(1) I can make a tarball instead of using svn
(2) I can provide a patch to SlicerFunctionExtractExtensionDescription.cmake and SlicerBlockBuildPackageAndUploadExtensions.cmake to send SVN_USERNAME/SVN_PASSWORD atoms to ExternalProject_Add()

Thanks!
Greg

Excellent :)

Good point. svnusername/svnpassword are not supported. Would be happy to review a patch. Ideally, would be great to get the patch before the end of afternoon so that it can be integrated in the final 4.1.1 release.

OK... probably not possible today, but still I will send the patch asap.

If we can get something into 4.1.1 that would be great, so I can demonstrate at the AAPM user group meeting.

Should I prepare the tarball instead?

Will try to get a patch in. Will keep you posted of my progress.

The extension build system doesn't yet support url to tarball.

Greg, could apply patch: find-package-Git-correct-case.patch

It basically change line 82 of FindSlicer.cmake from:

find_package (git)

to

find_package (Git)

Aha! Patched in revision 3608.

Few remarks:
1) Support for svnpassword/svnusername has been added: See http://www.na-mic.org/Bug/view.php?id=2152#bugnotes

2) I created a working example of extension description file for plastimatch: https://gist.github.com/2854051

3) Would be great to review the description file and:

• add url to an 128x128 pixels icon.
• add one or more url to screenshots
• check if the contributor(s) / organization(s) I listed are valid

Assuming you are logged in on github, you should be able to edit the file in place.

There are no category associated with the plastimatch loadable module. You should update the file qSlicerPlmSlicerBsplineModule.cxx.
See http://slicer.org/doc/html/classqSlicerAbstractCoreModule.html#ad60347317d4aa65f7739a76489d08841

Make also sure to properly implement the acknowledgementText and helpText methods ...

Greg, let me know if you need anything else from me. Thanks

Is that now done through the SlicerRT extension ?

Hi Jc, yes, we are working toward delivering plastimatch together with SlicerRt extension.

We seem to be almost finished. It should be ready for Slicer 4.2 release.

-Greg

Excellent. Thanks for the update

Considering Plastimatch is being integrated in SlicerRT extension, direct integration of Plastimatch within Slicer as an extension doesn't apply anymore. For that reason, I am resolving the issue.