[Insight-developers] m_EndContinuousIndex[j]

[Michel Audette]

Hi Simon,

I went with Tom's suggestion, in a manner consistent with what you have
here. I have not checked it in yet. There seem to be other inconsistencies
between the various flavours of IsInside() that still cause inconsistencies
in some of tests, leading the latter to fail, and I hope to commit a few
fixes at once.

Thanks for your kind reply.

Best wishes,

Michel

On Fri, May 29, 2009 at 1:01 PM, Simon Warfield <
simon.warfield at childrens.harvard.edu> wrote:

>
>  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
>
> _______________________________________________
> 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
>



-- 
Michel Audette, Ph.D.
R & D Engineer,
Kitware Inc.,
Chapel Hill, N.C.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090529/3093c6b0/attachment.htm>