Re: [ggt-devel] Patch Intersection.h

[Patrick H. <pa...@13...>]

On Feb 13, 2009, at 10:31 AM, Sebastian Messerschmidt wrote:

> Patrick Hartling schrieb:
>> On Feb 12, 2009, at 3:54 AM, Sebastian Messerschmidt wrote:
>>
>>
>>> Hi,
>>>
>>> as I've written some weeks ago, I've fixed some float/double/
>>> DATA_TYPE issues and  corrected some intersection functions, that
>>> were return false positives.
>>>
>>> The changes in detail:
>>>
>>> Line 318:
>>> Fixed the box / lineseg intersection, which was missing an early out
>>> for the case were the ray hit the box. but the line didn't (some
>>> simple early out test by checking tin,tout between 0 and 1)
>>>
>>
>> This change breaks one of the tests (see
>> gmtlTest::IntersectionTest::testIntersectAABoxLineSeg() at line 199  
>> of
>> Test/TestSuite/TestCases/IntersectionTest.cpp). As far as I can tell,
>> the expectation of the test is correct. Specifically, there is a line
>> segment that starts within the axis-aligned bounding box and ends
>> outside the box. In this case, tIn has a value of -0.5, and tOut is
>> 0.5. Should the test be a logical AND instead of an OR?
>>
>>
> Okay, after rethinking it my intention got obvious again.
> Consider the following setup:
> {
>        // Unit box centered at the origin.
>        gmtl::AABoxf box(gmtl::Point3f(-0.5f, -0.5f, -0.5f),
>            gmtl::Point3f(0.5f, 0.5f, 0.5f));
>        // Unit line segment with its origin on the front edge of the  
> box
>        // and its endpoint on the back edge of the box.
>        gmtl::LineSegf seg(gmtl::Point3f(0.0f, 0.0f, 0.0f),
>            gmtl::Point3f(0.1f, 0.0f, -0.0f));
>        unsigned int num_hits;
>        float t_in, t_out;
>       CPPUNIT_ASSERT(!gmtl::intersect(box, seg, num_hits, t_in,  
> t_out));
>
> }
>
> With the old function, this caused a hit because the segment was  
> inside
> the box with tIn = tOut = 5,
> which in my opinion is wrong, because the line segment never enters or
> leaves the box.
> Do you agree?
> If so, just take the logical AND instead of or, which will do the  
> trick ;-)

Yes, I agree with that. I have added the test you presented and made  
that change. Thanks!

  -Patrick


--
Patrick L. Hartling
Senior Software Engineer, Priority 5
http://www.priority5.com/