The bug herein is very unstable and it's one I've been chasing for 
years. Recent povr only bounding code changes caused the parse trace() 
to fail more consistently using Warren's recent Halloween stain test 
scene. And even so I found slight changes in code ordering would cause 
the bug to disappear.

The bug only shows up where one or more ray direction components is 
zero. Further, I think it comes to divide by zero exceptions being 
handled in a standard - reliable way - only where NOT bypassing certain 
exception handling with --ffast-math options.  I've no plans to run down 
particulars after taking a few shots in the dark as to the exact cause.

--- The simple fix.

Don't use -ffast-math / -O3. We currently 'ship' linux/unix builds using 
--ffast-math though most linux package developers use a conservative -O2 
generic build. People installing provided packages are likely OK. If 
compiling your own versions, avoid fast-math optimizations.

--- Alternate fix.

The more complex suggested code fix better preserving -O3/-ffast-math is 
shown below and it's what I've moved to with my povr branch.

--- Background

A change from v3.6 to v3.7 and onward was the introduction of some 
in-lined / template code culminating in a object->Intersect_BBox call. 
This code also at the root of a couple of other v3.7+ issues fixed in 
the povr branch.  Only one issue/pull req remains open (#357) IIRC. The 
inability to completely turn off bounding with -mb is another issue 
where I closed the pull req after it sitting a long time.

The aim of this code is to speed up operations like projected_through, 
the shadow cache, the parser's trace()(*) and subsurface feature 
calculations.  Despite the issues - where the one herein another - it 
largely accomplishes its performance aims per the targets.

I'm unsure the overhead cost / gains in more the more mainstream 
intersection determination.

This new-ish ray-bbox test code requires the calculation of a variant 
enum and invdir 3d vector. The code for this calculation exists in four 
places that I see. Two in trace.cpp and two in object.cpp. It's 
currently identical, four times repeated code.

----------- Current code

     BBoxVector3d origin;
     BBoxVector3d invdir;
     BBoxDirection variant;

     Vector3d tmp(1.0 / ray.GetDirection()[X],
         1.0 / ray.GetDirection()[Y], 1.0 / ray.GetDirection()[Z]);
     origin = BBoxVector3d(ray.Origin);
     invdir = BBoxVector3d(tmp);
     variant = (BBoxDirection)
         ((int(invdir[X] < 0.0) << 2) | (int(invdir[Y] < 0.0) << 1) |
         int(invdir[Z] < 0.0));

----------- Alternate code not triggering divide by zero exceptions.

     Vector3d rayDir = ray.GetDirection();

     // Get inverse direction, explicitly handling zero components.
     // Avoiding divide by zero exception to enable -ffast-math/-O3
     // use without occasional flaky issues where sometimes
     // (invdir[*] < 0.0) working and not, depending on compiler
     // options / code structure / cpu / flow. (v3.7/v3.8/v4.0)

     double tmpX = (std::abs(rayDir.x())<gkDBL_epsilon) ?
         std::copysign(1.0,rayDir.x())*HUGE_VAL : 1.0/rayDir.x();
     double tmpY = (std::abs(rayDir.y())<gkDBL_epsilon) ?
         std::copysign(1.0,rayDir.y())*HUGE_VAL : 1.0/rayDir.y();
     double tmpZ = (std::abs(rayDir.z())<gkDBL_epsilon) ?
         std::copysign(1.0,rayDir.z())*HUGE_VAL : 1.0/rayDir.z();

//  Vector3d tmp(tmpX,tmpY,tmpZ);
//  BBoxVector3d invdir = BBoxVector3d(tmp);
     // OK if BBox at doubles, otherwise compile error & use above
     BBoxVector3d invdir = Vector3d(tmpX,tmpY,tmpZ);
//  BBoxVector3d origin = BBoxVector3d(ray.Origin);
     BBoxVector3d origin = ray.Origin;  // OK while BBox at doubles
     BBoxDirection variant = (BBoxDirection)
         ((int(invdir[X] < 0.0) << 2) | (int(invdir[Y] < 0.0) << 1)
         | int(invdir[Z] < 0.0));

Bill P.

Post a reply to this message