Fix indexToPosition when index == -1 #51
Conversation
|
Where exactly is the problem with the current version? |
|
@hinerm: Sounds like a job for a unit test! |
|
Please note, that the current implementation also has the property, that indexToPosition( index, dim, pos) satisfies positionToIndex( pos, dim ) == index for all values, also negative ones (not only for index = -1). This is a property that should hold with any new implementation. I also have reservations about possible performance implications of the proposed implementation as it incurs the cost of at least two object creations and at least one polymorphic virtual function call. So before considering to merge this I'd like to see a benchmark. |
The array-based indexToPosition methods were essentially all repeating one another. It was discovered that these methods did not handle the special "-1" index appropriately, which is supposed to signify that the position "will be at the origin after one fwd() call", essentially. Instead, these methods would use the standard index to position algorithm, which would create incorrect arrays (e.g. if the array was [1, 1, 5] the -1 index would give position [0, 0, 1] instead of [-1, 0, 0] as desired. All array-based indexToPosition methods now funnel through a single new SciJava PrimitiveArray signature. This method can operate on all arrays agnostically, thus the logic for creating a starting position when given index=-1 was added here as well. Note that the base primitive array accessor methods cause autoboxing, thus specialized casting methods were created to access the underlying primitive arrays directly.
The -1 index is used throughout ImgLib2 to signify a position one step before the starting position. However, the indexToPosition calculations in IntervalIndexer were not taking the -1 index into account, producing incorrect position arrays for this index.
This patch fixes the indexToPosition calculations. To avoid copying and pasting the fix throughout all signatures, the array-based methods were refactored to funnel through a single method using SciJava-Common PrimitiveArrays.