[Insight-developers] m_EndContinuousIndex[j]

[Simon Warfield]

Great! Thanks for getting these tests working.

Michel Audette wrote:
> 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 
> <mailto:simon.warfield at childrens.harvard.edu>> wrote:
>
>
>         Date: Thu, 28 May 2009 15:37:19 -0400
>         From: Michel Audette <michel.audette at kitware.com
>         <mailto: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 <http://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.
>


-- 
Simon