search

imglib / imglib2-ij

Translation between ImgLib & ImageJ data structures (both 1.x and 2)
Other
4 stars 8 forks source link

ImageJFunctions wrap supports volatile types #35

Open bogovicj opened 1 year ago

bogovicj commented 1 year ago

This PR improves the behavior of ImageJFunctions.wrap( RandomAccessibleInterval ) when the RAI has a volatile type.

Currently, all volatile types are routed to the wrapFloat method, because for example: VolatileShortType does not extend an IntegerType, but rather AbstractVolatileNativeRealType.

New behavior:

tpietzsch commented 1 year ago

I'm not sure where to comment. (here, or https://github.com/saalfeldlab/n5-viewer/issues/34, or https://github.com/imglib/imglib2/issues/332)

I'm still trying to track down the code for https://github.com/saalfeldlab/n5-viewer/issues/34 that causes the wrong type to be used. Maybe @bogovicj you could point me in the right direction?

This is a symptom of some other problem. You should not need to ever ImageJFunctions.wrap( RandomAccessibleInterval ) when the RAI has a volatile type. The volatile type RAIs may not have data, and there is no mechanism for IJ to determine whether the data is valid or not, and no mechanism to keep updating the wrapper until it is.

There should always be a corresponding non-volatile RAI, transparently sharing the same data, but blocking on access until the data is valid. For BDV in general, the idea is that SourceAndConverter<T> has non-volatile type with the corresponding volatile version nested underneath (via public SourceAndConverter<? extends Volatile<T>> asVolatile()). The volatile version should be used only for rendering. For cropping, movie recording, etc, the non-volatile source should be used, otherwise there will be missing data.

I thought I had solved this for n5-viewer with https://github.com/saalfeldlab/n5-viewer/pull/33, but apparently there are still some pure-volatile SourceAndConverter<T> used somewhere.

Anyway... While I didn't dig deeper on this PR or https://github.com/imglib/imglib2/issues/332, I do think that this is a symptom of another problem that wouldn't be really fixed by this PR. It should instead be fixed by using non-volatile/volatile pairs somewhere. @bogovicj I'm happy to help investigate...

bogovicj commented 1 year ago

Thanks @tpietzsch ,

I'll investigate to see why it is that volatile RAIs are being sent to imagej. I appreciate you having a look, thanks again!