[Insight-developers] bug 0006558

[Simon Warfield]

Currently nearest neighbor interpolation in ITK takes a continuous index 
and rounds to the nearest integer index, and evaluates the image at that 
point.
However, the bounds check done to determine if a continuous index is 
inside the image region is incompatible with this, and seems to be wrong.

Attached are two patches which correct the bounds calculation to utilize 
rounding of the continuous index to the nearest integer index. This 
ensures the correct bounds of each image are considered to be inside the 
image region.

In addition, I think Tom's suggestion below should be used in functions 
that transform physical points to indexes.

The variable name ITK_USE_CENTERED_PIXEL_COORDINATES doesn't really 
reflect what would occur if Tom's suggestion was used.
Currently the use of pixel coordinates is a mix of centered and corner 
representations which are handled inconsistently, and the suggestion is 
to use a common definition across all the code. I suggest instead to 
call the variable
  ITK_USE_CENTERED_PIXEL_COORDINATES_CONSISTENTLY
or the alternative
  ITK_USE_INCONSISTENT_PIXEL_COORDINATE_REPRESENTATIONS

> On Fri, Oct 24, 2008 at 7:53 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>   
>> I agree that this is an important issue, but due to the timing of
>> 3.10, I think we should defer until 3.11. We want to get it right the
>> first time.
>>
>> Bill
>>
>> On Fri, Oct 24, 2008 at 12:55 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>>     
>>> Hi Tom,
>>>
>>>
>>> This would be indeed a good opportunity for fixing this bug.
>>>
>>>
>>> In order to fulfill our backward compatibility promise, however,
>>> my suggestion will be to also condition this fix using a CMake
>>> variable that will make possible for users to get the old
>>> behavior.
>>>
>>>
>>> Maybe something like:
>>>
>>>      ITK_USE_CENTERED_PIXEL_COORDINATES
>>>
>>>
>>> Please let us know what you think,
>>>
>>>
>>>  Thanks
>>>
>>>
>>>     Luis
>>>
>>>
>>>
>>> ------------------------
>>> Tom Vercauteren wrote:
>>>       
>>>> Hi Luis (and others),
>>>>
>>>> I saw the recent refactoring done on the Index <--> Point transforms.
>>>>
>>>> It looks great and easier to maintain (once we get rid of the ifdef
>>>> blocks). I'm just wondering whether this couldn't be a great
>>>> opportunity to fix this bug:
>>>> http://www.itk.org/Bug/view.php?id=6558
>>>>
>>>> What bothers me most is the casts that are similar to:
>>>>  index[i] = static_cast<typename IndexType::IndexValueType>(
>>>> continuous_index[i] );
>>>>
>>>> Using something similar to
>>>>   index[i] = static_cast<typename IndexType::IndexValueType>(
>>>> vnl_math_rnd( continuous_index[i] ) );
>>>> would make more sense to me.
>>>>
>>>> Then could do a performance optimization on vnl_math_rnd. I did such a
>>>> proposition to the vnl team but for some reason, it remained a
>>>> discussion only:
>>>>
>>>> http://sourceforge.net/mailarchive/forum.php?thread_name=28392e8b0808050138r31574c90wddd19c0072a7da3c%40mail.gmail.com&forum_name=vxl-maintainers
>>>>
>>>> Best,
>>>> Tom
>>>>
>>>> P.S.: For the record, optimizing vnl_math_rnd & co would also allow
>>>> for fixing these bugs
>>>> http://www.itk.org/Bug/view.php?id=2078
>>>> http://www.itk.org/Bug/view.php?id=5692
>>>> which have long been waiting for a fix
>>>>
>>>>         


-- 
Simon K. Warfield, Ph.D.


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch-itkImageFunction.txt
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20081114/baf76f95/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patch-itkImageRegion.txt
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20081114/baf76f95/attachment-0001.txt>