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