Improvements in wrapping RealPointCollections

[gselzer]

With this immediate fix, the following SciJava script now runs (where before I got the same error described in #299):

The question now is whether we should further improve the type variables within RealPoint -> PointRoi conversions to be more correct.

#@ Dataset i
#@output Dataset out

from java.util import ArrayList
from net.imglib2 import RealPoint
from net.imagej.roi import DefaultROITree
from net.imglib2.roi.geom.real import DefaultWritableRealPointCollection

arr = ArrayList([RealPoint([10, 10])])
pc = DefaultWritableRealPointCollection(arr)

tree = DefaultROITree()
tree.addROIs(ArrayList([pc]))
i.getProperties().put("rois", tree)

out = i

Closes #299

[gselzer]

But this still doesn't work - if you run the script, you'll get a point ROI on the input image. Then any call to RealPointCollectionWrapper.synchronize() (including closing the image) throws the following error.

(Fiji Is Just) ImageJ 2.14.0/1.54f; Java 1.8.0_322 [64-bit]; Windows 10 10.0; 361MB of 48709MB (<1%)

java.lang.ClassCastException: net.imglib2.RealPoint cannot be cast to net.imglib2.roi.util.RealLocalizableRealPositionable
    at net.imagej.legacy.convert.roi.point.RealPointCollectionWrapper.synchronize(RealPointCollectionWrapper.java:99)
    at net.imagej.legacy.convert.roi.MaskPredicateWrapper.getUpdatedSource(MaskPredicateWrapper.java:63)
    at net.imagej.legacy.convert.roi.AbstractMaskPredicateUnwrapConverter.convert(AbstractMaskPredicateUnwrapConverter.java:74)
    at org.scijava.convert.ConvertService.convert(ConvertService.java:68)
    at net.imagej.legacy.convert.OverlayToROITreeConverter.convert(OverlayToROITreeConverter.java:82)
    at org.scijava.convert.ConvertService.convert(ConvertService.java:68)
    at net.imagej.legacy.LegacyImageMap.synchronizeAttachmentsToDataset(LegacyImageMap.java:611)
    at net.imagej.legacy.LegacyImageMap.synchronizeAttachmentsToDataset(LegacyImageMap.java:583)
    at net.imagej.legacy.LegacyImageMap.lookupDisplay(LegacyImageMap.java:197)
    at net.imagej.legacy.DefaultLegacyHooks.unregisterImage(DefaultLegacyHooks.java:190)
    at ij.gui.ImageWindow.close(ImageWindow.java)
    at ij.ImagePlus.close(ImagePlus.java:476)
    at ij.plugin.Commands.closeImage(Commands.java:138)
    at ij.plugin.Commands.close(Commands.java:91)
    at ij.plugin.Commands.run(Commands.java:29)
    at ij.IJ.runPlugIn(IJ.java:216)
    at ij.Executer.runCommand(Executer.java:152)
    at ij.Executer.run(Executer.java:70)
    at java.lang.Thread.run(Thread.java:750)

...maybe we have to do better 😆

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/fiji-friends-weekly-dev-update-thread/103718/12

[gselzer]

Update on current improvements:

• The RealPointCollectionWrapper is now typed on a type variable, so it can wrap arbitraryRealLocalizable & RealPositionablesubclasses. As a part of this, we need to define how to add new points, so I added a new constructor which takes aSupplier` that can create new points, and deprecated the old version.
• Refactored the Converter plugin to use the new constructor. The Supplier is constructed by asking the ConvertService to create the point type from a float[] - this works for RealPoints, and anything that takes a float[] as the sole constructor argument, thanks to the DefaultConverter.
• Added tests to ensure the above behavior.

[ctrueden]

Thanks @gselzer! This is definitely a step forward.

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/fiji-friends-weekly-dev-update-thread/103718/17