Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageJFunctions wrap supports volatile types #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bogovicj
Copy link

No description provided.

@bogovicj
Copy link
Author

bogovicj commented Jun 14, 2023

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:

  • VolatileARGBTypes are wrapped as ARGB
  • VolatileUnsignedByteTypes are wrapped as byte
  • VolatileUnsignedShortTypes are wrapped as short
  • VolatileShortType and VolatileIntTypes are also wrapped as short (despite the risks)
  • other volatile types will be wrapped to float as they are currently

@tpietzsch
Copy link
Member

I'm not sure where to comment. (here, or saalfeldlab/n5-viewer#34, or imglib/imglib2#332)

I'm still trying to track down the code for saalfeldlab/n5-viewer#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 saalfeldlab/n5-viewer#33, but apparently there are still some pure-volatile SourceAndConverter<T> used somewhere.

Anyway... While I didn't dig deeper on this PR or imglib/imglib2#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
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants