[Insight-developers] m_EndContinuousIndex[j]

Simon Warfield simon.warfield at childrens.harvard.edu
Fri May 29 13:01:28 EDT 2009


> Date: Thu, 28 May 2009 15:37:19 -0400
> From: Michel Audette <michel.audette at kitware.com>
>
> Would it not be better to have
>       m_EndContinuousIndex[j]   = static_cast<CoordRepType>( m_EndIndex[j] +
> 0.5 - FLT_MIN );
>
> or something to that effect, so that something with 1/2 pixel spacing sees
> the value m_EndIndex[j] + 0.5 declared outside?
>
> Currently, and unless we make this change, our logic is allowing in an extra
> row and column as "inside", which is probably not what we had in mind in
> going to pixel-centered coordinates.
>   

> Hi Michel,
>
> Indeed, there is a slight inconsistency here. When I wrote my first
> version of this patch based on Simon's proposal, there was still no
> consensus on what the rounding functions should do with half-integers.
> Now it's clear (round half-integers upward) and we should make
> IsInsideBuffer really consistent.
>
> An alternative fix to the one you proposed would be to replace
>   if( index[j] > m_EndContinuousIndex[j] )
> by
>   if( index[j] >= m_EndContinuousIndex[j] )
> if ITK_USE_CENTERED_PIXEL_COORDINATES_CONSISTENTLY is on.
>
> Cheers,
> Tom

The code needs to define an interval of physical space that represents 
the `inside'.
Since we are now rounding the half-way case up, the old code is wrong. 
We need to change it.

I think considering the computed continuous index markers as defining a 
half-open interval is better than subtracting a small floating point 
number to approximately define explicitly a closed interval. For one 
thing, it doesn't depend on the numerical precision of the floating 
point number representation.

So I vote for:

  if( index[j] >= m_EndContinuousIndex[j] )



-- 
Simon 




More information about the Insight-developers mailing list