RE: [Plib-devel] A Second Look Into SSG

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

Wolfram,

	Here are my answers to your answers to my comments, part two.  I
notice that CVS hasn't gotten your changes (from two hours ago, I'm not
complaining) yet, so I'm not looking at them (obviously).

(6) The author of the "spherebotch" code in "ssgEntity.cxx" on lines 80ff is
Steve Baker, in version 1.1 of the file.  Steve, what do you want the code
to do here?  My guess is that the spheres are supposed to shimmer as the
numbers of polygons changes back and forth.  But this is only a guess.

(9) In the file "ssgBase.cxx" on line 41 in the function
"ssgBase::copy_from", there is a test:  "if ( this == src ) return ;" with a
comment "Avoid corrupting userdata when copying to self."  In the file
"ssgSimpleList.cxx" starting on line 27 we find another "copy_from" function
which (among other things) deletes a variable "list" and allocates a new
one.  Do we want the code to do this even if "this == src" or do we want to
return without doing anything in this case?

(15) In "ssgVtxTable.cxx" on line 704ff, we are in the function
"hot_triangles".  The problem at hand is to determine whether the x- and
y-coordinates of a specified point "P" are within the two-dimensional
triangle given by three other pairs of x- and y-coordinates.  Let me call
the coordinates of the specified point (x,y) and the coordinates of the
three corners of the triangle (x1,y1), (x2,y2), and (x3,y3).  The three
corners themselves I will call "P1", "P2", and "P3".  The derivation begins
by expressing the coordinates of the point as parametric coordinates in
terms of the three corners:

	P = P1 + A ( P2 - P1 ) + B ( P3 - P1 )

where A and B are my parametric coordinates.  In terms of the coordinates, I
can express this as a matrix operation (you will need a constant-spaced font
to view this properly)

	[ x-x1 ] = [ (x2-x1) (x3-x1) ] [ A ]
	[ y-y1 ] = [ (y2-y1) (y3-y1) ] [ B ]

The determinant of the matrix is given by

	D = ( x2 - x1 ) ( y3 - y1 ) - ( x3 - x1 ) ( y2 - y1 )

Calculating the determinant takes two multiplies.

Inverting the matrix gives

	[ A ] = [  (y3-y1) -(x3-x1) ] [ x-x1 ]
	[ B ] = [ -(y2-y1)  (x2-x1) ] [ y-y1 ] / D

To save operations and to avoid the possibility of dividing by zero, we
multiply both sides of the equation by the determinant:

	[ D A ] = [  (y3-y1) -(x3-x1) ] [ x-x1 ]
	[ D B ] = [ -(y2-y1)  (x2-x1) ] [ y-y1 ]

Multiplying this out takes four more multiplications and gives us the
products "DA" and "DB" as a result.  The point is in the triangle if A >= 0,
B >= 0, and (A+B) <= 1.  Since we don't want to divide by D, we have the
following (admittedly complicated) test:

if ( D >= 0.0 )
{
  if ( ( DA < 0.0 ) || ( DB < 0.0 ) || ( DA + DB > D ) )
    continue ;  /* Outside the triangle */
}
else  /* Negative determinant, change the direction of the inequalities */
{
  if ( ( DA > 0.0 ) || ( DB > 0.0 ) || ( DA + DB < D ) )
    continue ;  /* Outside the triangle */
}

John F. Fay
joh...@eg...
850-729-6330 
-----Original Message-----
From: pli...@li...
[mailto:pli...@li...] On Behalf Of Wolfram Kuss
Sent: Monday, September 13, 2004 6:53 AM
To: pli...@li...
Subject: Re: [Plib-devel] A Second Look Into SSG

>Trivial:
>
>(1) More tabs that should be blank spaces in ssgSimpleState.cxx:545;
>ssgStateSelector.cxx:44-46, 83; ssgVtxTable.cxx:31-56, 66-126, 319-322,
>363-383, 418, 447-452, 796-798, 810-819

I untabified ssgStateSelector.cxx. I have local changes in the other
files, so it would be too much hassle to get the "Clean version",
untabify and commit. 

>(2) In "ssgEntity.cxx" on lines 117 and 131, the words "gimbles,"
>"seperate," and "seperated" should be "gimbals," "separate," and
>"separated."

Fixed

>(3) In "ssgLeaf.cxx" we may want to check the indentation of lines 36, 51,
>and 53 (two spaces instead of the usual four).

Hopefully fixed, please check it.

>(4) In "ssgVtxTable.cxx" on lines 46-47 the same test is repeated.

Fixed.

>(5) In "ssgVtxTable.cxx" on line 314, should we account for the GL_POINTS,
>GL_LINES, GL_LINE_LOOP, and GL_LINE_STRIP cases inside the "switch"
>statement?

Well, if you find that more elegant, ok. Changed.

>Less Trivial:
>
>(6) In "ssgEntity.cxx" on lines 80ff, the local variable "spherebotch" is
>set to a pseudorandom (but repeatable) value from zero to nine and
>incremented through a "switch" statement.  This is strange; each entity
will
>get its own value from zero to nine, but the same entity will always get
the
>same value.  Further, "spherebotch" is incremented on line 82 and reset to
>zero on line 92.  It will be reinitialized each time through the function.
>It should be added to the private variables of the "ssgEntity" class.

That sounds good and the current implementation is obviously not what
the author wanted. Still, before a change we might want to hear what
the author wanted to achieve.

>(7) In "ssgEntity.cxx" on line 432, we set the variable
"bsphere_is_invalid"
>to true.  Do we want to call "dirtyBSphere" instead, so that we set the
>parent entities' b-spheres dirty as well?

Yes. Fixed.

>(8) In "ssgList.cxx" on line 102 I think there is a bug.  The "if ( next >=
>n )" should be "if ( next > n )".  In the extreme case, if both "next" and
>"n" are zero, we will wind up pointing to "next" equal to -1.  In general,
>if "next" is equal to "n", we wind up pointing at the element we just
>processed.

Sounds good to me. I have not changed it yet though.

>(9) In "ssgSimpleList.cxx" on line 29, do we want to test for "if ( this ==
>src ) return ;"?  The test is in "ssgBase.cxx" on line 41, also in the
>"copy_from" function.

Could you check filename/line number please?

>(15) In "ssgVtxTable.cxx" on line 704ff, I can do this test with six
>multiplies.  Is there enough of a demand for it that somebody is willing to
>put it into CVS for me?

I would guess the FGFS people would appreciate it!

>I have some more comments to "ssgVtxTable.cxx" and the geometry functions,
>specifically in terms of making them more efficient; does anybody want to
>hear them?

Certainly, I can not promise any time though.

Thank you for your work!

Bye bye,
Wolfram.

-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM. 
Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php
_______________________________________________
plib-devel mailing list
pli...@li...
https://lists.sourceforge.net/lists/listinfo/plib-devel