WriteConvertedCursor constructor fails with ArrayIndexOutOfBoundsException

[gselzer]

The following test fails, illustrating the error:

    @Test
    public void testWriteConvertedIterableInterval() {
        final IterableInterval<UnsignedByteType>
            image = ArrayImgs.unsignedBytes(10, 10);
        // Conversion is defined as (value > 0)
        final SamplerConverter<? super UnsignedByteType, BitType > s = in -> new BitType(in.get().get() != 0);

        final WriteConvertedIterableInterval<UnsignedByteType, BitType> w = Converters.convert(image, s);
        w.cursor();
    }

The failure occurs here, as the source Cursor (created here) starts before the first index.

[tpietzsch]

You misunderstand how SamplerConverter needs to works.

Consider the case of accessing the red channel of an ARGBType image as UnsignedByteType.

Using a "normal" (read-only) Converter you do this:

static RandomAccessible< UnsignedByteType > redChannelRead( RandomAccessible< ARGBType > argbs )
{
    return Converters.convert(
            argbs,
            ( argb, r ) -> r.set( ( argb.get() & 0xff0000 ) >> 16 ),
            new UnsignedByteType() );
}

Here, the converters framework will create a new UnsignedByteType() for each RandomAccess. When the you call get() on the converted RandomAccess, it calls get() on the source RandomAccess, then calls your lambda to set the converted value on the RA's UnsignedByteType then returns that.

Writing the UnsignedByteType back to the red channel of the ARGBType cannot be driven by the RandomAccess like that. Again, the converted RandomAccess has an UnsignedByteType that it returns on get(). But if somebody calls .setByte(10) on that UnsignedByteType, the RandomAccess has no way of knowing about that.

So instead of the RandomAccess triggering the conversion, the UnsignedByteType has to do it. SamplerConverter.convert creates a special UnsignedByteType instance which knows about a specific Sampler<ARGBType> instance (the RandomAccess in the example). When that UnsignedByteType instance is written to (or read from), it get()s the current ARGBType from the source sampler (RandomAccess), does the conversion and writes the converted value to that ARGBType.

So instead of doing the conversion once, the SamplerConverter must create a Type that does the conversion everytime it is read from / written to.

The easiest way to accomplish this is by constructing a standard UnsignedByteType with a special underlying ByteAccess implementation that takes care of the conversion.

Have a look at the ARGBChannelSamplerConverter implementation for how this works concretely in the ARGB channel scenario.

I made an issue #358 to add javadoc to SamplerConverter to explain this better. (Feel free to take this comment and make it into a PR...)

I'll probably add another issue about making convenience wrappers for the standard types, so that for example for converting between T and UnsignedByteType you could just give "normal" converters in both directions, and the special UnsigneByteType with special ByteAccess stuff is created for you. I'm not sure yet that is a good idea performance-wise, but issue cannot hurt.

[tpietzsch]

not a bug.

[gselzer]

@tpietzsch thank you for your explanation, I think it augmented my existing knowledge about how the Converters class works, although now I'm confused as to the purpose of the WriteConverted classes. However with that being said, I think we're talking past each other here. Let me try to explain.

Here, the converters framework will create a new UnsignedByteType() for each RandomAccess. When the you call get() on the converted RandomAccess, it calls get() on the source RandomAccess, then calls your lambda to set the converted value on the RA's UnsignedByteType then returns that.

This is great, but as far as I can tell this does not happen for WriteConvertedCursor, which was the point I was trying to bring up over on Zulip. When you call WriteConvertedCursor.get(), you don't see the behavior you've described, because you do not call get() on the source Cursor, like ConvertedCursor.get() does. You see similar behavior in WriteConvertedRandomAccess.get().

This issue doesn't even deal with this incorrect behavior though, because I can't even create a WriteConvertedCursor given a WriteConvertedIterableInterval - it throws an AIOOBE due to the cursor starting at position -1. This is not a problem in ConvertedCursor, as it does not call the SamplerConverter before a call to ConvertedCursor.get().

[tpietzsch]

final SamplerConverter<? super UnsignedByteType, BitType > s = in -> new BitType(in.get().get() != 0);

Here, in is the Cursor. The SamplerConverter needs to create a BitType that accesses the value of in.get() when the value of the BitType is accessed. That is, at the time youi create this BitType you cannot assume that in.get() will work because (as you have observed) the Sampler in may not point to valid data at this point.

[tpietzsch]

Here is what you want:

final SamplerConverter<? super UnsignedByteType, BitType > s = in -> new BitType( new LongAccess()
{
    @Override
    public long getValue( final int index )
    {
        return in.get().get() != 0 ? -1L : 0L;
    }

    @Override
    public void setValue( final int index, final long value )
    {
        in.get().set( value != 0 ? 1 : 0 );
    }
} );

Although maybe it would be better here to use NativeBoolType instead of BitType

final SamplerConverter<? super UnsignedByteType, NativeBoolType > s = in -> new NativeBoolType( new BooleanAccess()
{
    @Override
    public boolean getValue( final int index )
    {
        return in.get().get() != 0;
    }

    @Override
    public void setValue( final int index, final boolean value )
    {
        in.get().set( value ? 1 : 0 );
    }
} );

[gselzer]

Here, in is the Cursor. The SamplerConverter needs to create a BitType that accesses the value of in.get() when the value of the BitType is accessed. That is, at the time youi create this BitType you cannot assume that in.get() will work because (as you have observed) the Sampler in may not point to valid data at this point.

Aha, this is enlightening. I now see your point, this is not at all the understanding I obtained from looking at the code. I believe that the Javadoc you suggested, and potentially as well some default SamplerConverters, would go a long way towards helping others. I'll make a PR for the former.