From 73b9fb7ef46eda92e0767380a3b0fac365e330e5 Mon Sep 17 00:00:00 2001
From: Max Zeyen <max.zeyen@kitware.com>
Date: Tue, 7 Jan 2020 17:00:38 +0100
Subject: [PATCH] Fixing The Cell Binning

Cells could not belong to multiple bins causing wrong
binning behavior in cases where the cell bounding box
intersects bin boundaries.
By inflating the cell bounding box by a small tolerance value,
the cell will be associated correctly to all bins it intersects.

Adding A Test For StaticCellLocator Edge Cases

Adding a test to cover edge cases of the methods
FineCellsAlongLine and FindClosesPointWithinRadius.

Adding Test Data

Adding the testing data for the new vtkStaticCellLocator test.
---
 Common/DataModel/Testing/Cxx/CMakeLists.txt   |  12 +-
 .../Cxx/TestStaticCellLocatorEdgeCases.cxx    | 105 ++++++++++++++++++
 .../Testing/Data/test_surface.vtp.sha512      |   1 +
 Common/DataModel/vtkStaticCellLocator.cxx     |  26 +++--
 4 files changed, 131 insertions(+), 13 deletions(-)
 create mode 100644 Common/DataModel/Testing/Cxx/TestStaticCellLocatorEdgeCases.cxx
 create mode 100644 Common/DataModel/Testing/Data/test_surface.vtp.sha512

diff --git a/Common/DataModel/Testing/Cxx/CMakeLists.txt b/Common/DataModel/Testing/Cxx/CMakeLists.txt
index a5675347e4a..6c27cb1bee1 100644
--- a/Common/DataModel/Testing/Cxx/CMakeLists.txt
+++ b/Common/DataModel/Testing/Cxx/CMakeLists.txt
@@ -103,7 +103,12 @@ vtk_add_test_cxx(vtkCommonDataModelCxxTests data_tests
   TestQuadraticPolygonFilters.cxx
   )
 # add to the list but don't define a test
-list(APPEND data_tests TestPolyhedron2.cxx TestPolyhedronContouring.cxx TestPolyhedronCutter.cxx)
+list(APPEND data_tests
+  TestPolyhedron2.cxx
+  TestPolyhedronContouring.cxx
+  TestPolyhedronCutter.cxx
+  TestStaticCellLocatorEdgeCases.cxx
+  )
 
 vtk_add_test_cxx(vtkCommonDataModelCxxTests output_tests
   TestKdTreeRepresentation.cxx,NO_DATA
@@ -123,6 +128,11 @@ ExternalData_add_test(${_vtk_build_TEST_DATA_TARGET}
   COMMAND vtkCommonDataModelCxxTests TestPolyhedronCutter
   DATA{../Data/onePolyhedron.vtu} DATA{../Data/sliceOfPolyhedron.vtu}
   )
+ExternalData_add_test(${_vtk_build_TEST_DATA_TARGET}
+  NAME VTK::CommonDataModelCxxTests-TestStaticCellLocatorEdgeCases
+  COMMAND vtkCommonDataModelCxxTests TestStaticCellLocatorEdgeCases
+  DATA{../Data/test_surface.vtp}
+  )
 
 set(all_tests
   ${tests}
diff --git a/Common/DataModel/Testing/Cxx/TestStaticCellLocatorEdgeCases.cxx b/Common/DataModel/Testing/Cxx/TestStaticCellLocatorEdgeCases.cxx
new file mode 100644
index 00000000000..3c479fa7860
--- /dev/null
+++ b/Common/DataModel/Testing/Cxx/TestStaticCellLocatorEdgeCases.cxx
@@ -0,0 +1,105 @@
+/*=========================================================================
+
+  Program:   Visualization Toolkit
+  Module:    TestStaticCellLocatorEdgeCases.cxx
+
+  Copyright (c) Ken Martin, Will Schroeder, Bill Lorensen
+  All rights reserved.
+  See Copyright.txt or http://www.kitware.com/Copyright.htm for details.
+
+     This software is distributed WITHOUT ANY WARRANTY; without even
+     the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+     PURPOSE.  See the above copyright notice for more information.
+
+=========================================================================*/
+
+#include <vtkCell.h>
+#include <vtkGenericCell.h>
+#include <vtkIdList.h>
+#include <vtkLineSource.h>
+#include <vtkNew.h>
+#include <vtkStaticCellLocator.h>
+#include <vtkXMLPolyDataReader.h>
+
+int TestCell(vtkDataSet* ds, int cellId, double x1[3], double x2[3], double tol)
+{
+  double t = 0.0;
+  double x[3] = { 0.0, 0.0, 0.0 };
+  double pcoords[3] = { 0.0, 0.0, 0.0 };
+  int subId = 0;
+  vtkCell* cell = ds->GetCell(cellId);
+
+  return cell->IntersectWithLine(x1, x2, tol, t, x, pcoords, subId);
+}
+
+int TestStaticCellLocatorEdgeCases(int argc, char* argv[])
+{
+  if (argc < 2)
+  {
+    cout << "Not enough arguments.";
+    return EXIT_FAILURE;
+  }
+
+  //===========
+  // Test Setup
+  //===========
+  int numFailed = 0;
+  double tol = 1E-15; // tolerance only used in TestCell
+
+  vtkNew<vtkXMLPolyDataReader> reader;
+  char* fname = argv[1];
+  reader->SetFileName(fname);
+  reader->Update();
+  vtkDataSet* data = reader->GetOutput();
+
+  vtkNew<vtkStaticCellLocator> locator;
+  locator->SetDataSet(data);
+  locator->CacheCellBoundsOn();
+  locator->AutomaticOn();
+  locator->BuildLocator();
+
+  //========================
+  // Test FindCellsAlongLine
+  //========================
+  double x1[3] = { 0.437783024586950, 0.0263950841209563, 0.373722994626027 };
+  double x2[3] = { 0.442140196830658, 0.0256207765183134, 0.374080391702881 };
+  vtkNew<vtkIdList> cellList;
+  locator->FindCellsAlongLine(x1, x2, tol, cellList);
+
+  int found = 0;
+  for (vtkIdType i = 0; i < cellList->GetNumberOfIds(); ++i)
+  {
+    found |= TestCell(data, cellList->GetId(i), x1, x2, tol);
+  }
+
+  if (found == 0)
+  {
+    std::cerr << "FindCellsAlongLine: No valid cell intersections found!" << std::endl;
+    ++numFailed;
+  }
+
+  //==================================
+  // Test FindClosestPointWithinRadius
+  //==================================
+  vtkNew<vtkGenericCell> cell;
+  vtkIdType cellId;
+  double dist2 = 0.0;
+  int subId = 0;
+  double closestPoint[3] = { 0.0, 0.0, 0.0 };
+  double radius = 0.00058385;
+  double x3[3] = { 0.44179561594301064, -0.017842554788570667, 0.28626203407677214 };
+  found =
+    locator->FindClosestPointWithinRadius(x3, radius, closestPoint, cell, cellId, subId, dist2);
+
+  if (found == 0)
+  {
+    std::cerr << "FindClosestPointWithinRadius: No valid cells found within given radius!"
+              << std::endl;
+    ++numFailed;
+  }
+
+  //====================
+  // Final Tests Outcome
+  //====================
+  return (numFailed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/Common/DataModel/Testing/Data/test_surface.vtp.sha512 b/Common/DataModel/Testing/Data/test_surface.vtp.sha512
new file mode 100644
index 00000000000..03f8c1befea
--- /dev/null
+++ b/Common/DataModel/Testing/Data/test_surface.vtp.sha512
@@ -0,0 +1 @@
+3f9e6e31967135b33f5267497949fb484689c9992aef5341ee9de092beef1e9e50d0e3272d345e1437e3d84066a7e80bd7990e9fa9b33c394e05a52436d40490
diff --git a/Common/DataModel/vtkStaticCellLocator.cxx b/Common/DataModel/vtkStaticCellLocator.cxx
index 19919b63e78..9e80206124c 100644
--- a/Common/DataModel/vtkStaticCellLocator.cxx
+++ b/Common/DataModel/vtkStaticCellLocator.cxx
@@ -187,12 +187,13 @@ struct vtkCellBinner
     for (; cellId < endCellId; ++cellId, bds += 6)
     {
       this->DataSet->GetCellBounds(cellId, bds);
-      xmin[0] = bds[0];
-      xmin[1] = bds[2];
-      xmin[2] = bds[4];
-      xmax[0] = bds[1];
-      xmax[1] = bds[3];
-      xmax[2] = bds[5];
+      double tol = this->binTol;
+      xmin[0] = bds[0] - tol;
+      xmin[1] = bds[2] - tol;
+      xmin[2] = bds[4] - tol;
+      xmax[0] = bds[1] + tol;
+      xmax[1] = bds[3] + tol;
+      xmax[2] = bds[5] + tol;
 
       this->GetBinIndices(xmin, ijkMin);
       this->GetBinIndices(xmax, ijkMax);
@@ -378,12 +379,13 @@ struct CellProcessor : public vtkCellProcessor
 
     for (; cellId < endCellId; ++cellId, bds += 6)
     {
-      xmin[0] = bds[0];
-      xmin[1] = bds[2];
-      xmin[2] = bds[4];
-      xmax[0] = bds[1];
-      xmax[1] = bds[3];
-      xmax[2] = bds[5];
+      double tol = this->Binner->binTol;
+      xmin[0] = bds[0] - tol;
+      xmin[1] = bds[2] - tol;
+      xmin[2] = bds[4] - tol;
+      xmax[0] = bds[1] + tol;
+      xmax[1] = bds[3] + tol;
+      xmax[2] = bds[5] + tol;
 
       this->Binner->GetBinIndices(xmin, ijkMin);
       this->Binner->GetBinIndices(xmax, ijkMax);
-- 
GitLab