RenderKit / embree

Embree ray tracing kernels repository.
Apache License 2.0
2.39k stars 390 forks source link

Out of bounds primitive split #328

Closed Hengoo closed 3 years ago

Hengoo commented 3 years ago

I have some issues with primitive Splitting and the BVH builder that is exposed with rtcBuildBVH. (https://github.com/embree/embree#rtcbuildbvh)

Embree sometimes calls the splitPrimitive function with a position that is exactly on, or even outside of the primitive bounds. I think both cases make no sense and should be catched before the split primitive function is called.

The first case, a split value that is on the bounds can be be observed with the tutorial. Add

    if (((&prim->lower_x)[dim] == pos) ||
      ((&prim->upper_x)[dim] == pos)) {
      // Split position is exactly on the bounds of the primitive
      // This case also happens with the random triangles from the example
      std::cout << "Split on the bounds with pos = " << pos << " and lower Bounds: " << (&prim->lower_x)[dim] << ", upper Bounds: " << (&prim->upper_x)[dim] << std::endl;
      //assert(false);
    }

To https://github.com/embree/embree/blob/master/tutorials/bvh_builder/bvh_builder_device.cpp#L23

For the second case I can also reproduce it with the Tutorial builder by loading some custom primitives and adding this code to the splitTriangle function:

    if (((&prim->lower_x)[dim] > pos) ||
      ((&prim->upper_x)[dim] < pos)) {
      // The split position is outside of the primitive bounds.
      // This only happens with the primitiveDump
      std::cout << "Out of bounds split with pos = " << pos << " and lower Bounds: " << (&prim->lower_x)[dim] << ", upper Bounds: " << (&prim->upper_x)[dim] << std::endl;
      //assert(false);
    }

To load the primitives: primitiveTest.txt replace the random bounding box generation https://github.com/embree/embree/blob/master/tutorials/bvh_builder/bvh_builder_device.cpp#L144 with:

      // Load primitives:
      std::vector<RTCBuildPrimitive> primitives;
      primitives.resize(528);
      std::ifstream file;
      file.open("primitiveTest", std::ofstream::in | std::ofstream::binary);
      file.read(reinterpret_cast<char*>(primitives.data()), primitives.size() * sizeof(int));
      file.close();

      const size_t N = 528;
      const size_t extraSpace = 1072 - 528;
      avector<RTCBuildPrimitive> prims;
      prims.resize(N);
      for (size_t i = 0; i < N; i++)
      {
        prims[i] = primitives[i];
      }

and place the file in the build folder (tutorials\bvh_builder)

And a bit related, what should we do in the splitPrimitive function when we know that the primitive should not be split? Create infinite bounding box? Very large bounding box? NaN? I would need this to avoid the out of bounds splits. And an other case is when I want to build a BVH that contains different primitives, where some for primitives should never be split. Like instances, or other thing that are extremely expensive and we never want to visit twice during traversal.

svenwoop commented 3 years ago

Could you please try out of branch issue_328_out_of_bounds_spatial_split fixes your issue.

svenwoop commented 3 years ago

If you want a primitive to not get spatially split, then just return empty bounds (inf,-inf) for the left bounds, and return primitive bounds in right bounds.

Hengoo commented 3 years ago

The out ouf bounds split issue seem to be fixed, but there is still an other problem that causes Embree call the setBounds function with empty bounds (inf, -inf) for the second child. (only when triangle splitting is enabled)

You can reproduce it with the BVH build example. Example.zip The zip contains a bvh_builder_device.cpp that you can copy over the existing one (https://github.com/embree/embree/blob/master/tutorials/bvh_builder/bvh_builder_device.cpp) and the file called primitiveTest_6085 needs to be in the build folder under tutorials\bvh_builder.

When you start the bvh build project it will crash at the assertions in SetBounds. (when you remove those it will hit the assert in Node::Create function that makes sure that numPrims == 1)

svenwoop commented 3 years ago

Could you please try the issue_328_out_of_bounds_spatial_split branch again. Think the issue is fixed now.

Hengoo commented 3 years ago

Yes it works great. Thanks!

svenwoop commented 3 years ago

This fix will be in the next release.