Fix triangle splitting crash where FP error causes binning differences

Ultimately the root problem here is that when we do the binning stage we use a
different method to test the first stage and the second. In the first stage we
use bin() to get an integer bin associated with a point in space. In the second
we get the floating point division point on the axis we have chosen to bin on
and do FP comparisons instead. Under most circumstances this is fine.

Occasionally these binning differences will result in unprofitable splits
happening but still building a usable tree.

In the test case I've given, the unprofitable splits we erroneously choose
fill up the split limit and we don't end up splitting the triangles the
previous stage had intended to choose. As a result, we can't split those
triangles. Then when we do the real bifurcation, everything is on the same
side. This resulted in an assertion failure in production (1).

(1) We never noticed that the RelWithDebInfo build in MSVC still has assertions
enabled. I can provide another change disabling these since I think it's
unintentional.

Author: jordan-xbox

kernels/builders/heuristic_spatial_array.h

@@ -283,11 +283,9 @@ namespace embree
 
                 if (likely(splits <= 1)) continue; /* todo: does this ever happen ? */
 
-                //int bin0 = split.mapping.bin(prims0[i].lower)[split.dim];
-                //int bin1 = split.mapping.bin(prims0[i].upper)[split.dim];
-                //if (unlikely(bin0 < split.pos && bin1 >= split.pos))
-
-                if (unlikely(prims0[i].lower[split.dim] < fpos && prims0[i].upper[split.dim] > fpos))
+                const int bin0 = split.mapping.bin(prims0[i].lower)[split.dim];
+                const int bin1 = split.mapping.bin(prims0[i].upper)[split.dim];
+                if (unlikely(bin0 < split.pos && bin1 >= split.pos))
                 {
                   assert(splits > 1);
 

tutorials/verify/verify.cpp

@@ -1776,6 +1776,64 @@ namespace embree
     }
   };
 
+  struct TriangleSplitRegression : public VerifyApplication::Test
+  {
+    TriangleSplitRegression (int isa)
+      : VerifyApplication::Test("TriangleSplitRegression", isa, VerifyApplication::TEST_SHOULD_PASS) {}
+
+    VerifyApplication::TestReturnValue run(VerifyApplication* state, bool silent)
+    {
+      std::string cfg = state->rtcore + ",isa="+stringOfISA(isa) + ",max_spatial_split_replications=1.5,max_triagles_per_leaf=1";
+      RTCDeviceRef device = rtcNewDevice(cfg.c_str());
+      errorHandler(nullptr,rtcGetDeviceError(device));
+      SceneFlags sflags = { RTC_SCENE_FLAG_NONE, RTC_BUILD_QUALITY_HIGH };
+      VerifyScene scene(device,sflags);
+      AssertNoError(device);
+
+      RTCBuildArguments arguments = rtcDefaultBuildArguments();
+
+      Ref<SceneGraph::TriangleMeshNode> mesh = new SceneGraph::TriangleMeshNode(nullptr, BBox1f(0, 1));
+      mesh->triangles = {
+        { 0,1,2 },
+        { 3,4,5 },
+        { 6,7,8 },
+        { 6,7,8 },
+      };
+
+      mesh->positions.push_back({});
+      auto& pos = mesh->positions.front();
+
+      const float
+        a = -6.4000024f, // Lower limit, matters for determining binning space
+        b = 25.6f,       // Needs to be very close to one of the binning positions so the classifiers disagree
+        c = 34.f,        // Just needs to be between b and d
+        d = 57.6000022f; // Upper limit, matters for determining binning space
+
+      pos.push_back(Vec3fa(b, 0, 0));
+      pos.push_back(Vec3fa(d, 0, 0));
+      pos.push_back(Vec3fa(d, 1, 0));
+
+      pos.push_back(Vec3fa(b, 0, 0));
+      pos.push_back(Vec3fa(c, 0, 0));
+      pos.push_back(Vec3fa(c, 1, 0));
+
+      pos.push_back(Vec3fa(a, 0, 0));
+      pos.push_back(Vec3fa(d, 0, 0));
+      pos.push_back(Vec3fa(d, 1, 0));
+
+      auto geom = scene.addGeometry(RTC_BUILD_QUALITY_HIGH, mesh.dynamicCast<SceneGraph::Node>());
+
+      RTCGeometry hgeom = rtcGetGeometry(scene, geom);
+      AssertNoError(device);
+
+      rtcEnableGeometry(hgeom);
+      rtcCommitScene (scene);
+      AssertNoError(device);
+
+      return VerifyApplication::PASSED;
+    }
+  };
+
   struct UpdateTest : public VerifyApplication::IntersectTest
   {
     SceneFlags sflags;
@@ -6210,6 +6268,10 @@ namespace embree
         groups.top()->add(new DisableAndDetachGeometryTest(to_string(sflags),isa,sflags));
       groups.pop();
 
+      push(new TestGroup("triangle_split_epsilon",true,true));
+      groups.top()->add(new TriangleSplitRegression(isa));
+      groups.pop();
+
       push(new TestGroup("update",true,true));
       for (auto sflags : sceneFlagsDynamic) {
         for (auto imode : intersectModes) {