First, I want to apologize for for not following up on the matter after
discovering the bug. It was just before my holidays and after my
holidays I was swamped in Real Work[TM] and forgot about it.

On Fri, 11 Aug 2000 13:21:10 +0200, J. Grimbert wrote:
>   In the intersect_box() code of super.c, the aims of
>the function is to compute the length from the origin
>on the current ray of the two possible intersections with a
>box (-1,1) and to order them correctly (hence the max and min
>suffixes everywhere)
>
>The intersect_box() itself is made of three big IF block:
>one for the X axis, one for the Y axis and one for the Z axis.
>For each axis, the intersection is looked for the two planes of
>the axis (-1 and 1).

Yup.

>The reuse of tmin and tmax is a potential source of error, because in
>the first IF block, tmin and tmax should rather be called xtmax and
>xtmin (I guess the t is for temporary). 
>As well in the second IF block, they should be ytmax and ytmin;
>and in the third and last block, it should be ztmax and ztmin.

Possibly. But tmin and tmax are always just the distance of the
currently considered planes from P, so I guess it makes sense to reuse
the variables (or at least the variable name - personally I would have
declared those variables only in the blocks where they are used).

>At the end of every IF block, the current lengths found (*dmin and
>*dmax) are compared with the new intersection. If the intersection
>is better suited, the *dmin or/and *dmax are updated.

>The fact is that when the code reach these two lines, 
>the *dmin and *dmax are already correct,

I agree with this. The two lines are indeed wrong.

This can be seen most easily in the case where I discovered the bug. If
the ray is parallel to the x axis, intersect_box enters only the first
block, computes *dmin and *dmax correctly (without using tmin and tmax)
and then overwrites the correct values with the values for tmin and
tmax, which still have their default value (0.0).

AFAICS dmin and dmax are always computed correctly. I don't know what
the last two lines are supposed to do, they just don't make sense.
Probably a leftover from an earlier restructuring of the code.

	hp

-- 
   _  | Peter J. Holzer    | Nicht an Tueren mangelt es,
|_|_) | Sysadmin WSR       | sondern an der Einrichtung (aka Content).
| |   | hjp### [at] wsracat      |    -- Ale### [at] univieacat
__/   | http://www.hjp.at/ |       zum Thema Portale in at.linux

Post a reply to this message