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

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

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