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/