[Plib-devel] A Second Look Into SSG

[Fay J. F C. AAC/W. <joh...@eg...>]

Gentlemen,

	I have been looking a bit farther into SSG and have some more
comments.

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

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

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

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

(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?

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.

(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?

(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.

(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.

(10) In "ssgVTable.cxx" on line 41 there is a note that "ssgVTable" is
obsolete.  Do we want to put a copy of this note in "ssg.h" on line 1307?
Do we want to deprecate the class in favor of "ssgVtxTable"?

(11) In "ssgVtxTable.cxx" on lines 338 and 364, the formulae look good to
me.  Does somebody else want to check them and take out the comment?

(12) In "ssgVtxTable.cxx" on lines 346, 362, and 385, do we want to change
the "assert" calls to "ulError" with an error message?  This also holds,
perhaps a little less strongly, with lines 365, 370, and 378.

(13) In "ssgVtxTable.cxx" on line 540, what if "preDraw" is null?

(14) In "ssgVtxTable.cxx" on line 656ff, it would be more efficient to
transform the vertex array all at once and then "getTriangle" from the
transformed array.  I don't know whether this function is an execution time
driver.

(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 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?

John F. Fay
joh...@eg...
850-729-6330