ggt-devel

[ggt-devel] gmtl Intersection.h / Triangle oddity

[Sebastian M. <seb...@gm...>]

Hello,

I'm currently using gmtl a lot for intersection testing. I've spotted 
some strange things in Intersection.h, where instead of DATA_TYPE float 
was used when returning intersection parameters (like tangent space for 
triangle/ray-intersection).
When using double type Tri and Ray I had some numerical instabilities, 
which lead to artifacts in my routines. Changing the return values (e.g. 
u,v,t to DATA_TYPE& instead of float&) solved those problems.

The second question regards the triangle/linesegment intersection itself.
It is stated, it will only work for CCW-oriented triangles, it however 
doesn't state, that intersection can only be done from one side.
Assume a correctly assembled triangle: If intersection is done with a 
ray of direction (0,0,-1) it returns a correct result.
Using a origin below the triangle and reversing the ray direction 
results in not hitting the triangle.

There are two possible solutions to this:
1. Make the triangle intersection use a slightly other method (like 
moeller trumbore original does with precalculating the inverse 
determinant), which works excellent for me.
2. Make a second function using the latter method.

Another thing I've come across is the mData members in most of the class 
used in gmtl.
This make life a lot easier regarding serialization (I'm using 
non-intrusive serialization offered by boost), but it is not done 
consequently in all classes.
For instance Tri<DATA_TYPE> doesn't declare its vertices public.
So should this be done the same way for every class?

I'd really like to submit the changes I've made so far, if you folks 
tell me how to provide you the sources.

cheers
Sebastian

Re: [ggt-devel] gmtl Intersection.h / Triangle oddity

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

Sebastian Messerschmidt wrote:
> Hello,
> 
> I'm currently using gmtl a lot for intersection testing. I've spotted 
> some strange things in Intersection.h, where instead of DATA_TYPE float 
> was used when returning intersection parameters (like tangent space for 
> triangle/ray-intersection).
> When using double type Tri and Ray I had some numerical instabilities, 
> which lead to artifacts in my routines. Changing the return values (e.g. 
> u,v,t to DATA_TYPE& instead of float&) solved those problems.

This is a recurring problem in GMTL. It was generally tested with float, and 
once in a while, you will come across a place where the code mistakenly uses 
float explicitly instead of DATA_TYPE. I encourage you to fix any such 
issues that you encounter. The nice thing is that when you fix it for 
double, it should get fixed for all other data types.

> The second question regards the triangle/linesegment intersection itself.
> It is stated, it will only work for CCW-oriented triangles, it however 
> doesn't state, that intersection can only be done from one side.
> Assume a correctly assembled triangle: If intersection is done with a 
> ray of direction (0,0,-1) it returns a correct result.
> Using a origin below the triangle and reversing the ray direction 
> results in not hitting the triangle.
> 
> There are two possible solutions to this:
> 1. Make the triangle intersection use a slightly other method (like 
> moeller trumbore original does with precalculating the inverse 
> determinant), which works excellent for me.
> 2. Make a second function using the latter method.

Someone more familiar with the specifics of this will have to comment. I 
usually just make sure that things compile. :)

> Another thing I've come across is the mData members in most of the class 
> used in gmtl.
> This make life a lot easier regarding serialization (I'm using 
> non-intrusive serialization offered by boost), but it is not done 
> consequently in all classes.
> For instance Tri<DATA_TYPE> doesn't declare its vertices public.
> So should this be done the same way for every class?

It could be, although I have often wondered about the reasoning behind 
having the data members public. I realize the value of being able to pass a 
pointer directly to glMultMatrix[fd](), but a const pointer can be retrieved 
with an inline method call.

I will be interested to see how your serialization code will handle the 
array serialization. I expect that you will be able to get the compiler to 
figure out the array size in order to do the serialization generically.

> I'd really like to submit the changes I've made so far, if you folks 
> tell me how to provide you the sources.

Check out the CVS HEAD of the source tree and post unified diffs:

    cvs diff -u File.h > patch

Then, post the patch as an attachment. Do not paste the text of the patch 
into a message as critical formatting details can get munged by mail clients.

  -Patrick


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

Re: [ggt-devel] gmtl Intersection.h / Triangle oddity

[Sebastian M. <seb...@gm...>]

Patrick Hartling wrote:
> Sebastian Messerschmidt wrote:
>   
>> Hello,
>>
>> I'm currently using gmtl a lot for intersection testing. I've spotted 
>> some strange things in Intersection.h, where instead of DATA_TYPE float 
>> was used when returning intersection parameters (like tangent space for 
>> triangle/ray-intersection).
>> When using double type Tri and Ray I had some numerical instabilities, 
>> which lead to artifacts in my routines. Changing the return values (e.g. 
>> u,v,t to DATA_TYPE& instead of float&) solved those problems.
>>     
>
> This is a recurring problem in GMTL. It was generally tested with float, and 
> once in a while, you will come across a place where the code mistakenly uses 
> float explicitly instead of DATA_TYPE. I encourage you to fix any such 
> issues that you encounter. The nice thing is that when you fix it for 
> double, it should get fixed for all other data types.
>   
O.K.. I've spotted those floats in several places and I will test those 
fixes as good as possible.
>   
>> The second question regards the triangle/linesegment intersection itself.
>> It is stated, it will only work for CCW-oriented triangles, it however 
>> doesn't state, that intersection can only be done from one side.
>> Assume a correctly assembled triangle: If intersection is done with a 
>> ray of direction (0,0,-1) it returns a correct result.
>> Using a origin below the triangle and reversing the ray direction 
>> results in not hitting the triangle.
>>
>> There are two possible solutions to this:
>> 1. Make the triangle intersection use a slightly other method (like 
>> moeller trumbore original does with precalculating the inverse 
>> determinant), which works excellent for me.
>> 2. Make a second function using the latter method.
>>     
>
> Someone more familiar with the specifics of this will have to comment. I 
> usually just make sure that things compile. :)
>
>   
>> Another thing I've come across is the mData members in most of the class 
>> used in gmtl.
>> This make life a lot easier regarding serialization (I'm using 
>> non-intrusive serialization offered by boost), but it is not done 
>> consequently in all classes.
>> For instance Tri<DATA_TYPE> doesn't declare its vertices public.
>> So should this be done the same way for every class?
>>     
>
> It could be, although I have often wondered about the reasoning behind 
> having the data members public. I realize the value of being able to pass a 
> pointer directly to glMultMatrix[fd](), but a const pointer can be retrieved 
> with an inline method call.
>
>   
The only reason I see so far is some optimization, but I'd rather lose 
some fancy data hiding if this allows us to do none-intrusive algorithms.
> I will be interested to see how your serialization code will handle the 
> array serialization. I expect that you will be able to get the compiler to 
> figure out the array size in order to do the serialization generically.
>
>   
It's not my code really. It's just about using boost to do all the dirty 
work for you.

For instance serializing the Vector class can be done like this:

template <class Archive, typename DATA_TYPE, unsigned SIZE>
void serialize( Archive & ar, gmtl::Vec<DATA_TYPE, SIZE> & v, const 
unsigned int )
{
        ar & v.mData;
}

This will handle all Vectors. Simple aye?

I've done this a bit different, since using the upper method will add 
one additional size_t/unsigned int
to the archive (to indicate the array length).

template <class Archive, typename DATA_TYPE, unsigned SIZE>
void serialize( Archive & ar, gmtl::Vec<DATA_TYPE, SIZE> & v, const 
unsigned int )
{
        for(size_t i = 0; i < SIZE; ++i)
        {
            ar & v.mData[i];
        }
}

This might be a bit slower performance wise, but it saves me some data 
to be paged. (I'm having like billions of vectors to be saved)
Sad thing is, that I am not sure about the loop unrolling in this function.


>> I'd really like to submit the changes I've made so far, if you folks 
>> tell me how to provide you the sources.
>>     
>
> Check out the CVS HEAD of the source tree and post unified diffs:
>
>     cvs diff -u File.h > patch
>
> Then, post the patch as an attachment. Do not paste the text of the patch 
> into a message as critical formatting details can get munged by mail clients.
>   
Can do :-)
I will will split this into multiple patches.

cheers
Sebastian

Re: [ggt-devel] gmtl Intersection.h / Triangle oddity

[Kevin M. <ke...@su...>]

compiler will generally loop unroll when there are constants, and the
iteration depth in your example is short...   in your case, up to 4 is
pretty short...
i'd look at the disassembly to be sure.   and you may need to play with
different optimization levels as well.  I'd expect it not to unroll in debug
mode.

for data hiding.   keeping mData public, would match the style everywhere
else in gmtl.   only reason not to do it, may be if we wanted to support
SIMD instructions in the future...   but for now, until that change happens
(which it may not unless someone gets ambitious), I'd make the mData's
public...    less code to write/maintain that way (no data() functions to
write and sprinkle around)...  and still little chance for user error, since
if they're accessing an "m" variable they know what they're doing anyway...

for the intersect function, be careful modifying the existing functions.  if
you're changing the meaning of the function (i.e. now it intersects from
both sides), this may break people's code who rely on that feature...
better to create a new intersect function with a different name.   or...
add a parameter to the existing function that defaults to the old way, but
can be set to make it behave in the new way...

- kevin

On Fri, Jan 16, 2009 at 2:56 AM, Sebastian Messerschmidt <
seb...@gm...> wrote:

> Patrick Hartling wrote:
> > Sebastian Messerschmidt wrote:
> >
> >> Hello,
> >>
> >> I'm currently using gmtl a lot for intersection testing. I've spotted
> >> some strange things in Intersection.h, where instead of DATA_TYPE float
> >> was used when returning intersection parameters (like tangent space for
> >> triangle/ray-intersection).
> >> When using double type Tri and Ray I had some numerical instabilities,
> >> which lead to artifacts in my routines. Changing the return values (e.g.
> >> u,v,t to DATA_TYPE& instead of float&) solved those problems.
> >>
> >
> > This is a recurring problem in GMTL. It was generally tested with float,
> and
> > once in a while, you will come across a place where the code mistakenly
> uses
> > float explicitly instead of DATA_TYPE. I encourage you to fix any such
> > issues that you encounter. The nice thing is that when you fix it for
> > double, it should get fixed for all other data types.
> >
> O.K.. I've spotted those floats in several places and I will test those
> fixes as good as possible.
> >
> >> The second question regards the triangle/linesegment intersection
> itself.
> >> It is stated, it will only work for CCW-oriented triangles, it however
> >> doesn't state, that intersection can only be done from one side.
> >> Assume a correctly assembled triangle: If intersection is done with a
> >> ray of direction (0,0,-1) it returns a correct result.
> >> Using a origin below the triangle and reversing the ray direction
> >> results in not hitting the triangle.
> >>
> >> There are two possible solutions to this:
> >> 1. Make the triangle intersection use a slightly other method (like
> >> moeller trumbore original does with precalculating the inverse
> >> determinant), which works excellent for me.
> >> 2. Make a second function using the latter method.
> >>
> >
> > Someone more familiar with the specifics of this will have to comment. I
> > usually just make sure that things compile. :)
> >
> >
> >> Another thing I've come across is the mData members in most of the class
> >> used in gmtl.
> >> This make life a lot easier regarding serialization (I'm using
> >> non-intrusive serialization offered by boost), but it is not done
> >> consequently in all classes.
> >> For instance Tri<DATA_TYPE> doesn't declare its vertices public.
> >> So should this be done the same way for every class?
> >>
> >
> > It could be, although I have often wondered about the reasoning behind
> > having the data members public. I realize the value of being able to pass
> a
> > pointer directly to glMultMatrix[fd](), but a const pointer can be
> retrieved
> > with an inline method call.
> >
> >
> The only reason I see so far is some optimization, but I'd rather lose
> some fancy data hiding if this allows us to do none-intrusive algorithms.
> > I will be interested to see how your serialization code will handle the
> > array serialization. I expect that you will be able to get the compiler
> to
> > figure out the array size in order to do the serialization generically.
> >
> >
> It's not my code really. It's just about using boost to do all the dirty
> work for you.
>
> For instance serializing the Vector class can be done like this:
>
> template <class Archive, typename DATA_TYPE, unsigned SIZE>
> void serialize( Archive & ar, gmtl::Vec<DATA_TYPE, SIZE> & v, const
> unsigned int )
> {
>        ar & v.mData;
> }
>
> This will handle all Vectors. Simple aye?
>
> I've done this a bit different, since using the upper method will add
> one additional size_t/unsigned int
> to the archive (to indicate the array length).
>
> template <class Archive, typename DATA_TYPE, unsigned SIZE>
> void serialize( Archive & ar, gmtl::Vec<DATA_TYPE, SIZE> & v, const
> unsigned int )
> {
>        for(size_t i = 0; i < SIZE; ++i)
>        {
>            ar & v.mData[i];
>        }
> }
>
> This might be a bit slower performance wise, but it saves me some data
> to be paged. (I'm having like billions of vectors to be saved)
> Sad thing is, that I am not sure about the loop unrolling in this function.
>
>
> >> I'd really like to submit the changes I've made so far, if you folks
> >> tell me how to provide you the sources.
> >>
> >
> > Check out the CVS HEAD of the source tree and post unified diffs:
> >
> >     cvs diff -u File.h > patch
> >
> > Then, post the patch as an attachment. Do not paste the text of the patch
> > into a message as critical formatting details can get munged by mail
> clients.
> >
> Can do :-)
> I will will split this into multiple patches.
>
> cheers
> Sebastian
>
>
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by:
> SourcForge Community
> SourceForge wants to tell your story.
> http://p.sf.net/sfu/sf-spreadtheword
> _______________________________________________
> ggt-devel mailing list
> ggt...@li...
> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>



-- 
kevin meinert | http://www.subatomicglue.com

Re: [ggt-devel] gmtl Intersection.h / Triangle oddity

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

Sebastian Messerschmidt wrote:
> Patrick Hartling wrote:
>> Sebastian Messerschmidt wrote:
>>   
>>> Hello,
>>>
>>> I'm currently using gmtl a lot for intersection testing. I've spotted 
>>> some strange things in Intersection.h, where instead of DATA_TYPE float 
>>> was used when returning intersection parameters (like tangent space for 
>>> triangle/ray-intersection).
>>> When using double type Tri and Ray I had some numerical instabilities, 
>>> which lead to artifacts in my routines. Changing the return values (e.g. 
>>> u,v,t to DATA_TYPE& instead of float&) solved those problems.
>>>     
>> This is a recurring problem in GMTL. It was generally tested with float, and 
>> once in a while, you will come across a place where the code mistakenly uses 
>> float explicitly instead of DATA_TYPE. I encourage you to fix any such 
>> issues that you encounter. The nice thing is that when you fix it for 
>> double, it should get fixed for all other data types.
>>   
> O.K.. I've spotted those floats in several places and I will test those 
> fixes as good as possible.

Thanks.

>>> The second question regards the triangle/linesegment intersection itself.
>>> It is stated, it will only work for CCW-oriented triangles, it however 
>>> doesn't state, that intersection can only be done from one side.
>>> Assume a correctly assembled triangle: If intersection is done with a 
>>> ray of direction (0,0,-1) it returns a correct result.
>>> Using a origin below the triangle and reversing the ray direction 
>>> results in not hitting the triangle.
>>>
>>> There are two possible solutions to this:
>>> 1. Make the triangle intersection use a slightly other method (like 
>>> moeller trumbore original does with precalculating the inverse 
>>> determinant), which works excellent for me.
>>> 2. Make a second function using the latter method.
>>>     
>> Someone more familiar with the specifics of this will have to comment. I 
>> usually just make sure that things compile. :)
>>
>>   
>>> Another thing I've come across is the mData members in most of the class 
>>> used in gmtl.
>>> This make life a lot easier regarding serialization (I'm using 
>>> non-intrusive serialization offered by boost), but it is not done 
>>> consequently in all classes.
>>> For instance Tri<DATA_TYPE> doesn't declare its vertices public.
>>> So should this be done the same way for every class?
>>>     
>> It could be, although I have often wondered about the reasoning behind 
>> having the data members public. I realize the value of being able to pass a 
>> pointer directly to glMultMatrix[fd](), but a const pointer can be retrieved 
>> with an inline method call.
>>
>>   
> The only reason I see so far is some optimization, but I'd rather lose 
> some fancy data hiding if this allows us to do none-intrusive algorithms.
>> I will be interested to see how your serialization code will handle the 
>> array serialization. I expect that you will be able to get the compiler to 
>> figure out the array size in order to do the serialization generically.
>>
>>   
> It's not my code really. It's just about using boost to do all the dirty 
> work for you.
> 
> For instance serializing the Vector class can be done like this:
> 
> template <class Archive, typename DATA_TYPE, unsigned SIZE>
> void serialize( Archive & ar, gmtl::Vec<DATA_TYPE, SIZE> & v, const 
> unsigned int )
> {
>         ar & v.mData;
> }
> 
> This will handle all Vectors. Simple aye?

Indeed. This is the sort of thing that I was hoping to see. :) 
Boost.Serialization has a very nice API.

> I've done this a bit different, since using the upper method will add 
> one additional size_t/unsigned int
> to the archive (to indicate the array length).
> 
> template <class Archive, typename DATA_TYPE, unsigned SIZE>
> void serialize( Archive & ar, gmtl::Vec<DATA_TYPE, SIZE> & v, const 
> unsigned int )
> {
>         for(size_t i = 0; i < SIZE; ++i)
>         {
>             ar & v.mData[i];
>         }
> }
> 
> This might be a bit slower performance wise, but it saves me some data 
> to be paged. (I'm having like billions of vectors to be saved)
> Sad thing is, that I am not sure about the loop unrolling in this function.

I expect that Kevin is right about the compiler doing the loop unrolling 
given the involvement of compile-time constants. If you're really paranoid, 
though, you could try to get Boost.MPL involved. Take a look at 
gmtl/Misc/MatrixConvert.h to see a usage of Boost.MPL to do compile-time 
loop unrolling (with Boost.Lambda thrown in because I saw an opportunity to 
use it).

>>> I'd really like to submit the changes I've made so far, if you folks 
>>> tell me how to provide you the sources.
>>>     
>> Check out the CVS HEAD of the source tree and post unified diffs:
>>
>>     cvs diff -u File.h > patch
>>
>> Then, post the patch as an attachment. Do not paste the text of the patch 
>> into a message as critical formatting details can get munged by mail clients.
>>   
> Can do :-)
> I will will split this into multiple patches.

That sounds great.

  -Patrick


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

Re: [ggt-devel] gmtl Intersection.h / Triangle oddity

[Sebastian M. <seb...@gm...>]

Patrick Hartling wrote:
> Sebastian Messerschmidt wrote:
>   
>> Hello,
>>
>> I'm currently using gmtl a lot for intersection testing. I've spotted 
>> some strange things in Intersection.h, where instead of DATA_TYPE float 
>> was used when returning intersection parameters (like tangent space for 
>> triangle/ray-intersection).
>> When using double type Tri and Ray I had some numerical instabilities, 
>> which lead to artifacts in my routines. Changing the return values (e.g. 
>> u,v,t to DATA_TYPE& instead of float&) solved those problems.
>>     
>
> This is a recurring problem in GMTL. It was generally tested with float, and 
> once in a while, you will come across a place where the code mistakenly uses 
> float explicitly instead of DATA_TYPE. I encourage you to fix any such 
> issues that you encounter. The nice thing is that when you fix it for 
> double, it should get fixed for all other data types.
>
>   
I've encountered a problem, and I need someone's advice.
Given the following interface:
template<class DATA_TYPE>
bool intersect( const Tri<DATA_TYPE>& tri, const Ray<DATA_TYPE>& ray,
                      float& u ,float& v,  float& t )
where u and v are the tangent space u/v coordinates of the intersection 
as stated in the documentation.

Now imagine my proposal for the new interface:
template<class DATA_TYPE>
bool intersect( const Tri<DATA_TYPE>& tri, const Ray<DATA_TYPE>& ray,
                        DATA_TYPE& u, DATA_TYPE& v, DATA_TYPE& t )

and a specialization with DATA_TYPE = int.

I guess, that is not what you want. u,v,t are now integers as well ...


This might work fine for float, double but is rather awkward for integer 
(but consistent on the other side)
One possible solution is setting the u,v,t to double instead of float. 
That would at least reduce the problems related to precision.
I'm a bit confused what the right way would be ...

cheers
Sebastian

 

Re: [ggt-devel] gmtl Intersection.h / Triangle oddity

[Kevin M. <ke...@su...>]

could have 2 template params: DATA_TYPE1 and DATA_TYPE2...  and do casts so
that it compiles for int...

or...

could add a comment, "this wont make sense for int...  cast to float version
to get a meaningful result..."
if you're going to int, you're probably not caring about performance much
anyways.

do whatever is fastest for float/double case...   i.e. look at the asm, make
sure that suggestion#1 above doesn't make extra code.

- kevin

On Mon, Jan 19, 2009 at 1:47 AM, Sebastian Messerschmidt <
seb...@gm...> wrote:

> Patrick Hartling wrote:
> > Sebastian Messerschmidt wrote:
> >
> >> Hello,
> >>
> >> I'm currently using gmtl a lot for intersection testing. I've spotted
> >> some strange things in Intersection.h, where instead of DATA_TYPE float
> >> was used when returning intersection parameters (like tangent space for
> >> triangle/ray-intersection).
> >> When using double type Tri and Ray I had some numerical instabilities,
> >> which lead to artifacts in my routines. Changing the return values (e.g.
> >> u,v,t to DATA_TYPE& instead of float&) solved those problems.
> >>
> >
> > This is a recurring problem in GMTL. It was generally tested with float,
> and
> > once in a while, you will come across a place where the code mistakenly
> uses
> > float explicitly instead of DATA_TYPE. I encourage you to fix any such
> > issues that you encounter. The nice thing is that when you fix it for
> > double, it should get fixed for all other data types.
> >
> >
> I've encountered a problem, and I need someone's advice.
> Given the following interface:
> template<class DATA_TYPE>
> bool intersect( const Tri<DATA_TYPE>& tri, const Ray<DATA_TYPE>& ray,
>                      float& u ,float& v,  float& t )
> where u and v are the tangent space u/v coordinates of the intersection
> as stated in the documentation.
>
> Now imagine my proposal for the new interface:
> template<class DATA_TYPE>
> bool intersect( const Tri<DATA_TYPE>& tri, const Ray<DATA_TYPE>& ray,
>                        DATA_TYPE& u, DATA_TYPE& v, DATA_TYPE& t )
>
> and a specialization with DATA_TYPE = int.
>
> I guess, that is not what you want. u,v,t are now integers as well ...
>
>
> This might work fine for float, double but is rather awkward for integer
> (but consistent on the other side)
> One possible solution is setting the u,v,t to double instead of float.
> That would at least reduce the problems related to precision.
> I'm a bit confused what the right way would be ...
>
> cheers
> Sebastian
>
>
>
>
>
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by:
> SourcForge Community
> SourceForge wants to tell your story.
> http://p.sf.net/sfu/sf-spreadtheword
> _______________________________________________
> ggt-devel mailing list
> ggt...@li...
> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>



-- 
kevin meinert | http://www.subatomicglue.com

[ggt-devel] Fix for broken isInVolume(AABOX, AABOX)

[Sebastian M. <seb...@gm...>]

Hi folks,

attached you'll find a patch for the isInVolume function testing two 
axis-aligned boxes.

The problem with the former algorithm had, was to rely on overlap test 
only, which means that intersecting boxes were considered containing 
each other.
I've rewritten the test, so it will work for all cases.

cheers
Sebastian "psy" Messerschmidt

P.S. There are more patches to come

Re: [ggt-devel] Fix for broken isInVolume(AABOX, AABOX)

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

On Feb 12, 2009, at 3:03 AM, Sebastian Messerschmidt wrote:

> Hi folks,
>
> attached you'll find a patch for the isInVolume function testing two  
> axis-aligned boxes.
>
> The problem with the former algorithm had, was to rely on overlap  
> test only, which means that intersecting boxes were considered  
> containing each other.
> I've rewritten the test, so it will work for all cases.

After applying this patch, I get a failure in the test suite. The test  
case (see gmtlTest::AABoxContainTest::testIsInVolumeAABox() in the  
file Test/TestSuite/TestCases/AABoxContainTest.cpp) checks to see if  
box2 contains box3 where these boxes are defined as follows:

    gmtl::AABoxf box2(gmtl::Point3f(30,30,30), gmtl::Point3f(40,40,40));
    gmtl::AABoxf box3(gmtl::Point3f(35,35,35), gmtl::Point3f(37,37,37));

Perhaps my expectation of gmtl::isInVolume() is wrong, but the  
documentation says that it should return true for the case when the  
second box (box3) is contained by the first (box2).

  -Patrick


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

Re: [ggt-devel] Fix for broken isInVolume(AABOX, AABOX)

[Kevin M. <ke...@su...>]

is it for completely contained.   or just for intersection?   i forget... :(

---
kevin meinert | http://www.subatomicglue.com


On Thu, Feb 12, 2009 at 4:34 PM, Patrick Hartling <pa...@13...> wrote:

> On Feb 12, 2009, at 3:03 AM, Sebastian Messerschmidt wrote:
>
> > Hi folks,
> >
> > attached you'll find a patch for the isInVolume function testing two
> > axis-aligned boxes.
> >
> > The problem with the former algorithm had, was to rely on overlap
> > test only, which means that intersecting boxes were considered
> > containing each other.
> > I've rewritten the test, so it will work for all cases.
>
> After applying this patch, I get a failure in the test suite. The test
> case (see gmtlTest::AABoxContainTest::testIsInVolumeAABox() in the
> file Test/TestSuite/TestCases/AABoxContainTest.cpp) checks to see if
> box2 contains box3 where these boxes are defined as follows:
>
>    gmtl::AABoxf box2(gmtl::Point3f(30,30,30), gmtl::Point3f(40,40,40));
>    gmtl::AABoxf box3(gmtl::Point3f(35,35,35), gmtl::Point3f(37,37,37));
>
> Perhaps my expectation of gmtl::isInVolume() is wrong, but the
> documentation says that it should return true for the case when the
> second box (box3) is contained by the first (box2).
>
>  -Patrick
>
>
> --
> Patrick L. Hartling
> Senior Software Engineer, Priority 5
> http://www.priority5.com/
>
>
>
> ------------------------------------------------------------------------------
> Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco,
> CA
> -OSBC tackles the biggest issue in open source: Open Sourcing the
> Enterprise
> -Strategies to boost innovation and cut costs with open source
> participation
> -Receive a $600 discount off the registration fee with the source code:
> SFAD
> http://p.sf.net/sfu/XcvMzF8H
> _______________________________________________
> ggt-devel mailing list
> ggt...@li...
> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>

Re: [ggt-devel] Fix for broken isInVolume(AABOX, AABOX)

[Sebastian M. <seb...@gm...>]

Kevin Meinert schrieb:
>
> is it for completely contained.   or just for intersection?   i 
> forget... :(
>
Hi Kevin,

the test is for complete containment. I'll check it again.
> ---
> kevin meinert | http://www.subatomicglue.com
>
>
> On Thu, Feb 12, 2009 at 4:34 PM, Patrick Hartling <pa...@13... 
> <mailto:pa...@13...>> wrote:
>
>     On Feb 12, 2009, at 3:03 AM, Sebastian Messerschmidt wrote:
>
>     > Hi folks,
>     >
>     > attached you'll find a patch for the isInVolume function testing two
>     > axis-aligned boxes.
>     >
>     > The problem with the former algorithm had, was to rely on overlap
>     > test only, which means that intersecting boxes were considered
>     > containing each other.
>     > I've rewritten the test, so it will work for all cases.
>
>     After applying this patch, I get a failure in the test suite. The test
>     case (see gmtlTest::AABoxContainTest::testIsInVolumeAABox() in the
>     file Test/TestSuite/TestCases/AABoxContainTest.cpp) checks to see if
>     box2 contains box3 where these boxes are defined as follows:
>
>        gmtl::AABoxf box2(gmtl::Point3f(30,30,30),
>     gmtl::Point3f(40,40,40));
>        gmtl::AABoxf box3(gmtl::Point3f(35,35,35),
>     gmtl::Point3f(37,37,37));
>
>     Perhaps my expectation of gmtl::isInVolume() is wrong, but the
>     documentation says that it should return true for the case when the
>     second box (box3) is contained by the first (box2).
>
>      -Patrick
>
>
>     --
>     Patrick L. Hartling
>     Senior Software Engineer, Priority 5
>     http://www.priority5.com/
>
>
>     ------------------------------------------------------------------------------
>     Open Source Business Conference (OSBC), March 24-25, 2009, San
>     Francisco, CA
>     -OSBC tackles the biggest issue in open source: Open Sourcing the
>     Enterprise
>     -Strategies to boost innovation and cut costs with open source
>     participation
>     -Receive a $600 discount off the registration fee with the source
>     code: SFAD
>     http://p.sf.net/sfu/XcvMzF8H
>     _______________________________________________
>     ggt-devel mailing list
>     ggt...@li...
>     <mailto:ggt...@li...>
>     https://lists.sourceforge.net/lists/listinfo/ggt-devel
>
>
> ------------------------------------------------------------------------
>
> ------------------------------------------------------------------------------
> Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
> -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
> -Strategies to boost innovation and cut costs with open source participation
> -Receive a $600 discount off the registration fee with the source code: SFAD
> http://p.sf.net/sfu/XcvMzF8H
> ------------------------------------------------------------------------
>
> _______________________________________________
> ggt-devel mailing list
> ggt...@li...
> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>   

Re: [ggt-devel] Fix for broken isInVolume(AABOX, AABOX)

[Sebastian M. <seb...@gm...>]

Mea culpa,

I've accidentally merged in the wrong lines I were using for testing.
For testing I have analyzed the unit test, which is either wrong or my 
understanding of what the function is doing is incorrect.

The function description says:
" Tests if the given AABox is completely inside or on the surface of the 
given
  AABox container."
As I understand this, the container must contain the whole box, not just 
intersect it, as tested in the unit test:

gmtl::AABoxf box2(gmtl::Point3f(30,30,30), gmtl::Point3f(40,40,40));
gmtl::AABoxf box3(gmtl::Point3f(35,35,35), gmtl::Point3f(37,37,37));
// Test overlapping valid boxes
     ....
      CPPUNIT_ASSERT(gmtl::isInVolume(box3, box2)); <-- wrong in my eyes

This test is wrong in my understanding, since the smaller box #3 cannot 
contain a larger box,

They are surely intersecting, but to check this, there are specialized 
tests.


Attached you'll find the new patch.

cheers



> Kevin Meinert schrieb:
>   
>> is it for completely contained.   or just for intersection?   i 
>> forget... :(
>>
>>     
> Hi Kevin,
>
> the test is for complete containment. I'll check it again.
>   
>> ---
>> kevin meinert | http://www.subatomicglue.com
>>
>>
>> On Thu, Feb 12, 2009 at 4:34 PM, Patrick Hartling <pa...@13... 
>> <mailto:pa...@13...>> wrote:
>>
>>     On Feb 12, 2009, at 3:03 AM, Sebastian Messerschmidt wrote:
>>
>>     > Hi folks,
>>     >
>>     > attached you'll find a patch for the isInVolume function testing two
>>     > axis-aligned boxes.
>>     >
>>     > The problem with the former algorithm had, was to rely on overlap
>>     > test only, which means that intersecting boxes were considered
>>     > containing each other.
>>     > I've rewritten the test, so it will work for all cases.
>>
>>     After applying this patch, I get a failure in the test suite. The test
>>     case (see gmtlTest::AABoxContainTest::testIsInVolumeAABox() in the
>>     file Test/TestSuite/TestCases/AABoxContainTest.cpp) checks to see if
>>     box2 contains box3 where these boxes are defined as follows:
>>
>>        gmtl::AABoxf box2(gmtl::Point3f(30,30,30),
>>     gmtl::Point3f(40,40,40));
>>        gmtl::AABoxf box3(gmtl::Point3f(35,35,35),
>>     gmtl::Point3f(37,37,37));
>>
>>     Perhaps my expectation of gmtl::isInVolume() is wrong, but the
>>     documentation says that it should return true for the case when the
>>     second box (box3) is contained by the first (box2).
>>
>>      -Patrick
>>
>>
>>     --
>>     Patrick L. Hartling
>>     Senior Software Engineer, Priority 5
>>     http://www.priority5.com/
>>
>>
>>     ------------------------------------------------------------------------------
>>     Open Source Business Conference (OSBC), March 24-25, 2009, San
>>     Francisco, CA
>>     -OSBC tackles the biggest issue in open source: Open Sourcing the
>>     Enterprise
>>     -Strategies to boost innovation and cut costs with open source
>>     participation
>>     -Receive a $600 discount off the registration fee with the source
>>     code: SFAD
>>     http://p.sf.net/sfu/XcvMzF8H
>>     _______________________________________________
>>     ggt-devel mailing list
>>     ggt...@li...
>>     <mailto:ggt...@li...>
>>     https://lists.sourceforge.net/lists/listinfo/ggt-devel
>>
>>
>> ------------------------------------------------------------------------
>>
>> ------------------------------------------------------------------------------
>> Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
>> -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
>> -Strategies to boost innovation and cut costs with open source participation
>> -Receive a $600 discount off the registration fee with the source code: SFAD
>> http://p.sf.net/sfu/XcvMzF8H
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> ggt-devel mailing list
>> ggt...@li...
>> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>>   
>>     
>
>
> ------------------------------------------------------------------------------
> Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
> -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
> -Strategies to boost innovation and cut costs with open source participation
> -Receive a $600 discount off the registration fee with the source code: SFAD
> http://p.sf.net/sfu/XcvMzF8H
> _______________________________________________
> ggt-devel mailing list
> ggt...@li...
> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>
>   

Re: [ggt-devel] Fix for broken isInVolume(AABOX, AABOX)

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

Thanks, that fixes things up nicely. This patch has been checked in.

  -Patrick

On Feb 13, 2009, at 2:48 AM, Sebastian Messerschmidt wrote:

> Mea culpa,
>
> I've accidentally merged in the wrong lines I were using for testing.
> For testing I have analyzed the unit test, which is either wrong or  
> my understanding of what the function is doing is incorrect.
>
> The function description says:
> " Tests if the given AABox is completely inside or on the surface of  
> the given
> AABox container."
> As I understand this, the container must contain the whole box, not  
> just intersect it, as tested in the unit test:
>
> gmtl::AABoxf box2(gmtl::Point3f(30,30,30), gmtl::Point3f(40,40,40));
> gmtl::AABoxf box3(gmtl::Point3f(35,35,35), gmtl::Point3f(37,37,37));
> // Test overlapping valid boxes
>    ....
>     CPPUNIT_ASSERT(gmtl::isInVolume(box3, box2)); <-- wrong in my eyes
>
> This test is wrong in my understanding, since the smaller box #3  
> cannot contain a larger box,
>
> They are surely intersecting, but to check this, there are  
> specialized tests.
>
>
> Attached you'll find the new patch.
>
> cheers
>
>
>
>> Kevin Meinert schrieb:
>>
>>> is it for completely contained.   or just for intersection?   i  
>>> forget... :(
>>>
>>>
>> Hi Kevin,
>>
>> the test is for complete containment. I'll check it again.
>>
>>> ---
>>> kevin meinert | http://www.subatomicglue.com
>>>
>>>
>>> On Thu, Feb 12, 2009 at 4:34 PM, Patrick Hartling <pa...@13...  
>>> <mailto:pa...@13...>> wrote:
>>>
>>>    On Feb 12, 2009, at 3:03 AM, Sebastian Messerschmidt wrote:
>>>
>>>    > Hi folks,
>>>    >
>>>    > attached you'll find a patch for the isInVolume function  
>>> testing two
>>>    > axis-aligned boxes.
>>>    >
>>>    > The problem with the former algorithm had, was to rely on  
>>> overlap
>>>    > test only, which means that intersecting boxes were considered
>>>    > containing each other.
>>>    > I've rewritten the test, so it will work for all cases.
>>>
>>>    After applying this patch, I get a failure in the test suite.  
>>> The test
>>>    case (see gmtlTest::AABoxContainTest::testIsInVolumeAABox() in  
>>> the
>>>    file Test/TestSuite/TestCases/AABoxContainTest.cpp) checks to  
>>> see if
>>>    box2 contains box3 where these boxes are defined as follows:
>>>
>>>       gmtl::AABoxf box2(gmtl::Point3f(30,30,30),
>>>    gmtl::Point3f(40,40,40));
>>>       gmtl::AABoxf box3(gmtl::Point3f(35,35,35),
>>>    gmtl::Point3f(37,37,37));
>>>
>>>    Perhaps my expectation of gmtl::isInVolume() is wrong, but the
>>>    documentation says that it should return true for the case when  
>>> the
>>>    second box (box3) is contained by the first (box2).
>>>
>>>     -Patrick
>>>
>>>
>>>    --
>>>    Patrick L. Hartling
>>>    Senior Software Engineer, Priority 5
>>>    http://www.priority5.com/
>>>
>>>
>>>     
>>> ------------------------------------------------------------------------------
>>>    Open Source Business Conference (OSBC), March 24-25, 2009, San
>>>    Francisco, CA
>>>    -OSBC tackles the biggest issue in open source: Open Sourcing the
>>>    Enterprise
>>>    -Strategies to boost innovation and cut costs with open source
>>>    participation
>>>    -Receive a $600 discount off the registration fee with the source
>>>    code: SFAD
>>>    http://p.sf.net/sfu/XcvMzF8H
>>>    _______________________________________________
>>>    ggt-devel mailing list
>>>    ggt...@li...
>>>    <mailto:ggt...@li...>
>>>    https://lists.sourceforge.net/lists/listinfo/ggt-devel
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> ------------------------------------------------------------------------------
>>> Open Source Business Conference (OSBC), March 24-25, 2009, San  
>>> Francisco, CA
>>> -OSBC tackles the biggest issue in open source: Open Sourcing the  
>>> Enterprise
>>> -Strategies to boost innovation and cut costs with open source  
>>> participation
>>> -Receive a $600 discount off the registration fee with the source  
>>> code: SFAD
>>> http://p.sf.net/sfu/XcvMzF8H
>>> ------------------------------------------------------------------------
>>>
>>> _______________________________________________
>>> ggt-devel mailing list
>>> ggt...@li...
>>> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>>>
>>
>>
>> ------------------------------------------------------------------------------
>> Open Source Business Conference (OSBC), March 24-25, 2009, San  
>> Francisco, CA
>> -OSBC tackles the biggest issue in open source: Open Sourcing the  
>> Enterprise
>> -Strategies to boost innovation and cut costs with open source  
>> participation
>> -Receive a $600 discount off the registration fee with the source  
>> code: SFAD
>> http://p.sf.net/sfu/XcvMzF8H
>> _______________________________________________
>> ggt-devel mailing list
>> ggt...@li...
>> https://lists.sourceforge.net/lists/listinfo/ggt-devel
>>
>>
>
> Index: Containment.h
> ===================================================================
> RCS file: /cvsroot/ggt/GGT/modules/GMTL/gmtl/Containment.h,v
> retrieving revision 1.21
> diff -u -r1.21 Containment.h
> --- Containment.h	13 Jun 2007 19:42:29 -0000	1.21
> +++ Containment.h	13 Feb 2009 08:27:38 -0000
> @@ -399,17 +399,17 @@
>    {
>       return false;
>    }
> -
> -   // Test that the boxes are not overlapping on any axis
> -   if (container.mMax[0] < box.mMin[0] || container.mMin[0] >  
> box.mMax[0] ||
> -       container.mMax[1] < box.mMin[1] || container.mMin[1] >  
> box.mMax[1] ||
> -       container.mMax[2] < box.mMin[2] || container.mMin[2] >  
> box.mMax[2])
> +
> +
> +if (container.mMin[0] <= box.mMin[0] && container.mMax[0] >=  
> box.mMax[0] &&
> +	   container.mMin[1] <= box.mMin[1] && container.mMax[1] >=  
> box.mMax[1] &&
> +	   container.mMin[2] <= box.mMin[2] && container.mMax[2] >=  
> box.mMax[2])
>    {
> -      return false;
> +	   return true;
>    }
>    else
>    {
> -      return true;
> +	   return false;
>    }
> }
>
> ------------------------------------------------------------------------------
> Open Source Business Conference (OSBC), March 24-25, 2009, San  
> Francisco, CA
> -OSBC tackles the biggest issue in open source: Open Sourcing the  
> Enterprise
> -Strategies to boost innovation and cut costs with open source  
> participation
> -Receive a $600 discount off the registration fee with the source  
> code: SFAD
> http://p.sf.net/sfu/XcvMzF8H_______________________________________________
> ggt-devel mailing list
> ggt...@li...
> https://lists.sourceforge.net/lists/listinfo/ggt-devel

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