imglib / imglib2-ij

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

ImageJFunctions.wrap mixes up channels with slices, and slices with time frames #29

Open acardona opened 5 years ago

acardona commented 5 years ago

When calling ImageJFunctions.wrap( RandomAccessibleInterval ) for a RandomAccessibleInterval with 3 dimensions, the third dimension is interpreted as channels rather than Z slices. For a 4D, the 4th dimension is interpreted as stack slices (the Z axis) rather than time frames.

This issue is bad enough that I am not using wrap anymore, because it makes further work with these virtual stacks hard. For example, the ImagePlus.setPosition( channels, slices, frames ) is entirely messed up, in that one has to use channels for slices, and slices for frames.

A solution would be to edit the private method ImageJFunctions.makeImagePlus to stop interpreting the 3rd dimension as channels by default. I am not sure what motivated this, given that a wrap over an RGB image shows it as a single-slice ColorProcessor.

According to git blame, the author of ImageJFunctions.makeImagePlus is @tpietzsch . I wonder if a comment could be made about the mix up between channels and slices, and slices with frames.

I'd like to rewrite the method ImageJFunctions.makeImagePlus to do the right thing. Perhaps some of the methods that call it within ImageJFunctions could specify the number of channels that they want as an argument. Otherwise, the default number of channels should be 1, and the 3rd dimension be interpreted by default as the Z axis.

ctrueden commented 5 years ago

CZT is the canonical ImageJ1 order. E.g., from the ImagePlus javadoc:

    /** Sets the 3rd, 4th and 5th dimensions, where 
    <code>nChannels</code>*<code>nSlices</code>*<code>nFrames</code> 
    must be equal to the stack size. */
    public void setDimensions(int nChannels, int nSlices, int nFrames) {

And:

    /** Sets the current hyperstack position and updates the display,
        where 'channel', 'slice' and 'frame' are one-based indexes. */
    public void setPosition(int channel, int slice, int frame) {
...
            setSlice((frame-1)*nChannels*nSlices + (slice-1)*nChannels + channel);

So the rasterization order is definitely (channel, slice, frame) <-> planeIndex, with channel fastest moving. This looks consistent with ImageJFunctions.makeImagePlus to me. What am I missing?

I'd like to rewrite the method ImageJFunctions.makeImagePlus to do the right thing.

That would break backwards compatibility very thoroughly, would it not?

On the other hand, adding more methods allowing to override the dimension order would preserve compatibility. But would make things more complicated.

Is there some reason you cannot simply use Views.permute before wrapping? Or even use net.imagej.ImgPlus since it directly supports axis metadata and uses it when converting via the ConvertService?

acardona commented 5 years ago

Thanks for the comments, @ctrueden . So your approach would be to insert an additional dimension, with size 1, to account for the channels, prior to calling ImageJFunctions.wrap.

Given that that is cumbersome, I'd rather add a new ImageJFuctions.wrap(RandomAccessibleInterval, title, n_channels) that would do just that. I'd be calling it almost always with n_channels = 1.

ctrueden commented 5 years ago

@acardona How about something like:

wrap(RandomAccessibleInterval image, String title, int cIndex, int zIndex, int tIndex)

So that one can explicitly state the dimensional axis index for each of C, Z and T?

So for your XYZT images you might write:

ImagePlus imp = ImageJFunctions.wrap(img, "My Image", -1, 2, 3);

And it would take care of adding and/or permuting dimensions accordingly.

@tpietzsch @maarzt What do you think?

acardona commented 5 years ago

I am not sure I understand the -1 for the channel index? You are thinking in terms of permutations of axes?

ctrueden commented 5 years ago

The idea is to identify the index of the C dimension within your Interval. Since your data is 4D, with dimensions (X, Y, Z, T), there is no C dimension. So you say -1 for that one. And then an extra dimension would be internally added and permuted to the correct place.

Other examples:

The last one, with interleaved channels, brings up an interesting point, though: what if X and Y are not the first two dimensions? More properly, we probably want to have:

wrap(RandomAccessibleInterval image, String title, int xIndex, int yIndex, int cIndex, int zIndex, int tIndex)

I.e. to include all 5 dimensions. But a signature without xIndex and yIndex could default them to 0 and 1 respectively, maybe (although if implemented it that way, my cxyt example above would not work as written...).

maarzt commented 5 years ago

@acardona You can already use the following solution:

ImagePlus result = ImgToVirtualStack.wrap( new ImgPlus<>( img, "title", new AxisType[] { Axes.X, Axes.Y, Axes.Z } ) );

Do we want to make this part of ImageJFunctions?

imagejan commented 5 years ago

Why do you want to put any new functionality into ImageJFunctions when we can already use ImgPlus and ConvertService to achieve the goal?

acardona commented 5 years ago

Hi @maarzt, the attractiveness of ImageJFunctions.wrap(RandomAccessibleInterval, String) is how simple it is, and how little the end user needs to know about the underlying engineering prowess that makes it all work. The fact that channels take precedence over Z slices is simply annoying from my end-user perspective, for my use cases. For me, channels belong to separate datasets. Showing them together is something I rarely need to do, and if anything, something that e.g. the BigDataViewer can do by giving it two datasets to display at the same time. Almost all data sets I work with are single-channel. That's why I advocate for a simple solution: something like a new method named ImageJFunctions.wrap(RandomAccessibleInterval, String, int) where int is the number of channels. Or even ImageJFunctions.wrapSingleChannel(RandomAccessibleInterval, String). It's not about beauty of names. It's about a function I use every day many times. On the name, I'd import such a static method with an alias, in jython:

from net.imglib2.img.display.imagej.ImageJFunctions.wrapSingleChannel as wrap

... and that'd be that.

Short of that, I have to create a function that inserts an additional dimension with value 1 just to handle my always-empty the channels dimension. Kind of defeats the simplicity of ImageJFunctions.wrap.

tpietzsch commented 5 years ago

I don't like the ImageJFunctions.wrap(RandomAccessibleInterval, String, int) idea so much. It works for numChannels=1 but it is unclear what happens with other values... What would the numChannels be if you do have channels? This assumes a bit the flattening of higher dimensions into one dimension that IJ1 does. But this is very uncommon for imglib2 images. So, if I had a XYCZ image with 5 channels, if I give numChannels = 2, what should that mean? If I have XYZC, then what do I do?

I like @maarzt solution. I would put it into BdvFunctions, yes. Maybe add shortcuts (something like bigdataviewer-vistools AxisOrder) enum, so that you could say BdvFunctions.show(img, "", AxisOrder.XYZ) instead if BdvFunctions.show(img, "", new AxisType[] { Axes.X, Axes.Y, Axes.Z }. A question is what to do when a different number of Axes is specified than numDimensions of the image, but it should be possible do do something reasonable there (or just throw an exception).

imagesc-bot commented 3 years ago

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

https://forum.image.sc/t/imglib2-force-wrapped-imageplus-rai-dimensions-to-xyczt/56461/1