[Insight-developers] Significant changes coming to address bug 6558

[Tom Vercauteren]

Hi Michel,

It would be great to see such a fix in ITK 3.14! Here's a follow-up on
Simon's comments.


Point A
-----------
I agree with Simon that we should try to handle all interpolators in a
consistent manner. One way to do so could be to add extrapolation
capabilities to the interpolators. What needs to be done for this is
relatively easy. An example is given here:
  http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkVectorLinearInterpolateNearestNeighborExtrapolateImageFunction.txx?root=Insight&sortby=date&view=markup

Basically, the only thing it does is, within
EvaluateAtContinuousIndex, modify the input continuous index to be the
closest continuous index that is strictly inside the region (using the
convention you use, i.e. not in the half pixel border).

Also, this approach has the nice side effect of allowing the
implementation of
XXXInterpolateNearestNeighborExtrapolateImageFunction for almost free.
One only needs to either not check for IsInsideBuffer or inherit from
XXXInterpolateImageFunction and override IsInsideBuffer to always
return true.

Technically the easiest might be to factorize the modification of the
input index within a small function of InterpolateImageFunction (or
ImageFunction). Maybe something like
  ContinuousIndexType GetClosestStrictlyInsideContinuousIndex( const
ContinuousIndexType & )

Note that this might not be the best scheme for higher order
interpolators (e.g. BSplines) since at the border, they might do a
little better than NN extrapolation. However, there is no other
generic way I can think of for handling this. Also NN is already a
decent border condition.


Point B
-----------
For the vector interpolators, I also think that they are almost
useless if we rewrite the code a little bit. For example, all it took
for the linear interpolator to handle vector images is here:
  http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkLinearInterpolateImageFunction.txx?root=Insight&r1=1.41&r2=1.37&sortby=date
with the unit test
  http://www.itk.org/cgi-bin/viewcvs.cgi/Testing/Code/Common/itkLinearInterpolateImageFunctionTest.cxx?root=Insight&r1=1.3&r2=1.4&sortby=date

By the way, the same thing should be true for complex images. I just
saw these classes:
  http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkComplexBSplineInterpolateImageFunction.h?root=Insight&view=markup
  http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Review/itkComplexBSplineInterpolateImageFunction.txx?root=Insight&view=markup

I don't think we need them. BSplineInterpolateImageFunction should be
able to handle complex pixels as good as vectors or (real) scalars.


Point C
-----------
Sorry for the use of vnl_math_round in my initial patch. It should
have been vnl_math_rnd from the beginning (vnl_math_round has never
existed). This doesn't raise any error (on my computer) because this
is in a piece of code that is not compiled when loop unrolling is
turned on (which is the default).

That being said, as mentioned by Simon, I think we need to move away
from vnl_math_rnd and use something like vnl_math_rnd_halfintup
  https://apps.sourceforge.net/trac/vxl/ticket/23
throughout ITK.


Cheers,
Tom


On Tue, May 5, 2009 at 23:57, Michel Audette <michel.audette at kitware.com> wrote:
> Dear members of the Insight community,
>
> we are currently at work to solve bug 6558: Physical coordinates of a pixel
> - Severe inconsistency and bug in ImageBase. We've found that the
> implementation of pixel-centered positions had unintended and adverse
> effects on other classes, which we have also been correcting for the past
> two days. This fix appears to be a priority of many members of the
> community, so we feel it's important to provide the required functionality
> integrally.
>
> We will make available a patch to the Insight community to apply and comment
> on.
> This patch can be found at: http://public.kitware.com/Bug/view.php?id=6558.
> Feel free to comment on its validity either by email or in the SecondLife
> discussions on Friday afternoons.
>
> We plan on making the fix permanent in the upcoming release.
>
> Best wishes,
>
> Michel
>
> --
> Michel Audette, Ph.D.
> R & D Engineer,
> Kitware Inc.,
> Chapel Hill, N.C.
>
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>
>