Calculation of the normal of bvh::Triangle about bvh HOT 4 CLOSED

This normal is primarily designed to speed up the intersection routine, not really as a normal you would use for rendering. This normal is what you would get in a left-handed coordinate system, which might not be what you would expect. I think this can be changed by just inverting the two arguments to cross, and changing the sign of the dot products in the intersection routine, but there is a small price to pay in terms of performance, because you now have to perform more operations (I am not sure how much, probably just a few percent, depending on how many intersections you have). I can add an option to do that if you are willing to pay that price.

from bvh.

Commit 1f2bd69 allows to change the triangle's normal orientation.

from bvh.

Thank you for the quick fix.
In the mean time, I have done a quick benchmark test of the consequence of this change.
The new intersection algorithm is about 3 percent slower.
I don't know if that's acceptable.
The benchmark is only used to time the intersection algorithm, so in normal situations while traversing a proper BVH, the triangle intersection test might not be called so often.
I'm just getting acquainted with BVH's so I cannot judge.
I think you are the best person to judge if it's OK to keep the change or it would be better to revert it.
In any case, thanks again for your efforts!

FYI: here follows part of the code I used to benchmark.
I also keep track of the position where a ray did hit a triangle for comparison with other algorithms.
For the timing comparisons I ran the benchmark program 10 times for old, and 10 times for the new intersection algorithm and then I divided the mean of the timing results. That showed that the new algorithm was 3 % slower.

  typedef double real_t;

  // Create an array of triangles
  bvh::Triangle<real_t> triangle(bvh::Vector3<real_t>( 1.0, -1.0, 1.0),
                                 bvh::Vector3<real_t>(-1.0, -1.0, 1.0),
                                 bvh::Vector3<real_t>(-1.0,  1.0, 1.0));
  int nrOfRays = 10000000;
  std::vector<bvh::Ray<real_t>> rays;

  real_t rayDirX = 1.0;
  real_t rayIncrX = real_t(-2.0)/nrOfRays;


  real_t rayDirY = 1.0;
  real_t rayIncrY = real_t(-3.0)/nrOfRays;


  // Intersect a ray with the data structure
  for (int iRay = 0; iRay < nrOfRays; iRay++)
  {
    rays.emplace_back(bvh::Vector3<real_t>(0.0, 0.0, 0.0), // origin
                      bvh::Vector3<real_t>(rayDirX, rayDirY, 1.0), // direction
                      0.0,                    // minimum distance
                      100.0                   // maximum distance
                      );
    rayDirX += rayIncrX;
    rayDirY += rayIncrY;
  }

  rasp::Timer timer;

  typedef bvh::Triangle<real_t>::Intersection Intersection;


  struct MyIntersection
  {
    MyIntersection(const Intersection& hit, bvh::Triangle<real_t>& triangle)
      : distanceFromOrigin(hit.t)
    {
      hitPosition = triangle.p0 - hit.u * triangle.e1 + hit.v * triangle.e2;
    }

    bvh::Vector3<real_t> hitPosition;
    real_t distanceFromOrigin;
  };
  std::map<int, MyIntersection> intersectionMap;

  timer.start();
  for (int iRay = 0; iRay < nrOfRays; iRay++)
  {
    if (auto hit = triangle.intersect(rays[iRay]))
    {
      auto pair = std::pair<int, MyIntersection>(iRay, MyIntersection(*hit, triangle));
      intersectionMap.insert(std::move(pair));
    }
  }
  timer.stop();

  std::cout << "Triangle_hit using bvh's triangle: " << nrOfRays << " rays: "
            << std::setprecision(3) << std::fixed << timer.elapsedMsec() << " ms" << std::endl;
  std::cout << "NrOfHits: " << intersectionMap.size() << std::endl;

from bvh.

Well now the change is only enabled if you use bvh::Triangle<Scalar, false>, and it is disabled by default. So I don't think it's a problem since it's now the user's choice, and the API is still compatible with the previous version.

from bvh.