Enable USE_UMFPACK in surface module on macOS CI (#6078)

* Fix some macro bugs * Comment unused chunk_count & Remove some logic that is always false * Comment unused merge_count & Fix a logic that is always true * Fix a bug & a logic that is always true * Remove a logic that is always true * Fix a bug about virtual function hiding * Deleted unused NodeCountHelper function * Fix extra bugs about virtual function hiding * Fix a bug about virtual function hiding * Fix for finding CHOLMOD & UMFPACK libraries * Address reviews * Add missing build argument * Addressed reviews
PointCloudLibrary · Nov 9, 2024 · f282546 · f282546
1 parent da7f34a
commit f282546
Show file tree

Hide file tree

Showing 17 changed files with 75 additions and 99 deletions.
diff --git a/.ci/azure-pipelines/build/macos.yaml b/.ci/azure-pipelines/build/macos.yaml
@@ -3,7 +3,7 @@ steps:
     # find the commit hash on a quick non-forced update too
     fetchDepth: 10
   - script: |
-      brew install cmake pkg-config boost eigen flann glew libusb qhull vtk glew freeglut qt5 libpcap libomp google-benchmark cjson
+      brew install cmake pkg-config boost eigen flann glew libusb qhull vtk glew freeglut qt5 libpcap libomp suite-sparse zlib google-benchmark cjson
       brew install brewsci/science/openni
       git clone https://github.com/abseil/googletest.git $GOOGLE_TEST_DIR # the official endpoint changed to abseil/googletest
       cd $GOOGLE_TEST_DIR && git checkout release-1.8.1
@@ -18,6 +18,7 @@ steps:
         -DGTEST_INCLUDE_DIR="$GOOGLE_TEST_DIR/googletest/include" \
         -DQt5_DIR=/usr/local/opt/qt5/lib/cmake/Qt5 \
         -DPCL_ONLY_CORE_POINT_TYPES=ON \
+        -DBUILD_surface_on_nurbs=ON -DUSE_UMFPACK=ON \
         -DBUILD_simulation=ON \
         -DBUILD_global_tests=ON \
         -DBUILD_benchmarks=ON \

diff --git a/surface/include/pcl/surface/3rdparty/opennurbs/opennurbs_polyedgecurve.h b/surface/include/pcl/surface/3rdparty/opennurbs/opennurbs_polyedgecurve.h
@@ -195,8 +195,9 @@ class ON_CLASS ON_PolyEdgeSegment : public ON_CurveProxy
           const ON_UUID& object_id
           );
 
+  using ON_CurveProxy::Trim; // Explicitly introduce the base class Trim function
   const ON_BrepEdge* Edge() const;
-  const ON_BrepTrim* Trim() const;
+  const ON_BrepTrim* Trim() const; // overload, not override
   const ON_Brep*     Brep() const;
   const ON_BrepFace* Face() const;
   const ON_Surface*  Surface() const;

diff --git a/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_asdm.h b/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_asdm.h
@@ -88,6 +88,8 @@ namespace pcl
         updateCurve (double damp) override;
 
       protected:
+        using FittingCurve2dAPDM::addPointConstraint;
+        using FittingCurve2dAPDM::assembleClosestPoints;
 
         /** \brief Add minimization constraint: point-to-surface distance (squared-distance-minimization). */
         virtual void

diff --git a/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_atdm.h b/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_atdm.h
@@ -82,6 +82,9 @@ namespace pcl
       updateCurve (double damp) override;
 
     protected:
+      using FittingCurve2dAPDM::addPointConstraint;
+      using FittingCurve2dAPDM::assembleClosestPoints;
+
       /** \brief Add minimization constraint: point-to-surface distance (tangent-distance-minimization). */
       virtual void
       addPointConstraint (const double &param, const Eigen::Vector2d &point, const Eigen::Vector2d &normal,

diff --git a/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_sdm.h b/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_sdm.h
@@ -87,6 +87,7 @@ namespace pcl
         updateCurve (double damp) override;
 
       protected:
+        using FittingCurve2dPDM::addPointConstraint;
 
         /** \brief Add minimization constraint: point-to-surface distance (squared-distance-minimization). */
         virtual void

diff --git a/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_tdm.h b/surface/include/pcl/surface/on_nurbs/fitting_curve_2d_tdm.h
@@ -89,6 +89,8 @@ namespace pcl
         updateCurve (double damp) override;
 
       protected:
+        using FittingCurve2dPDM::addPointConstraint;
+
         /** \brief Add minimization constraint: point-to-surface distance (tangent-distance-minimization). */
         virtual void
         addPointConstraint (const double &param, const Eigen::Vector2d &point, const Eigen::Vector2d &normal,

diff --git a/surface/include/pcl/surface/on_nurbs/fitting_surface_tdm.h b/surface/include/pcl/surface/on_nurbs/fitting_surface_tdm.h
@@ -50,6 +50,7 @@ namespace pcl
     class FittingSurfaceTDM : public FittingSurface
     {
     public:
+      using FittingSurface::assemble;
 
       /** \brief Parameters with TDM extensions for fitting */
       struct ParameterTDM : public FittingSurface::Parameter
@@ -94,6 +95,9 @@ namespace pcl
       updateSurf (double damp) override;
 
     protected:
+      using FittingSurface::assembleInterior;
+      using FittingSurface::assembleBoundary;
+      using FittingSurface::addPointConstraint;
 
       /** \brief Assemble point-to-surface constraints for interior points. */
       virtual void

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_3dm_settings.cpp b/surface/src/3rdparty/opennurbs/opennurbs_3dm_settings.cpp
@@ -3049,8 +3049,7 @@ static bool ON_3dmSettings_Read_v1_TCODE_VIEWPORT(ON_BinaryArchive& file, ON_3dm
   double clipdist = 0.0;
   double snapsize = 0.0;
 
-  int chunk_count = 0;// debugging counter
-  for ( chunk_count = 0; rc; chunk_count++ )
+  while ( rc )
   {
     rc = file.BeginRead3dmBigChunk( &tcode, &big_value );
     if (!rc )
@@ -3159,8 +3158,7 @@ bool ON_3dmSettings::Read_v1( ON_BinaryArchive& file )
   ON__INT64 big_value;
   rc = file.SeekFromStart(32)?true:false; // skip 32 byte header
 
-  int chunk_count = 0; // debugging counter
-  for ( chunk_count = 0; rc; chunk_count++ )
+  while ( rc )
   {
     rc = file.BeginRead3dmBigChunk( &tcode, &big_value );
     if ( !rc ) 

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_material.cpp b/surface/src/3rdparty/opennurbs/opennurbs_material.cpp
@@ -1505,7 +1505,7 @@ void ON_TextureMapping::Dump( ON_TextLog& text_log ) const
   case single:
     text_log.Print("single texture space\n");
     break;
-  case clspt_projection:
+  case divided:
     text_log.Print("divided texture space\n");
     break;
   default:
@@ -2359,7 +2359,7 @@ ON__UINT32 ON_TextureMapping::MappingCRC() const
       case ON_TextureMapping::cylinder_mapping:
       case ON_TextureMapping::sphere_mapping:
       case ON_TextureMapping::box_mapping:
-      case ON_TextureMapping::force_32bit_mapping_projection:
+      case ON_TextureMapping::force_32bit_mapping_type:
       default:
         break;
       }

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_math.cpp b/surface/src/3rdparty/opennurbs/opennurbs_math.cpp
@@ -1280,8 +1280,6 @@ ON_TransformPointList(
 
   if ( !ON_IsValidPointList( dim, is_rat, count, stride, point ) )
     return false;
-  if ( xform.m_xform == NULL )
-    return false;
   if (count == 0)
     return true;
 
@@ -1381,8 +1379,6 @@ ON_TransformPointList(
 
   if ( !ON_IsValidPointList( dim, is_rat, count, stride, point ) )
     return false;
-  if ( xform.m_xform == NULL )
-    return false;
   if (count == 0)
     return true;
 
@@ -1507,8 +1503,6 @@ ON_TransformVectorList(
 
   if ( !ON_IsValidPointList( dim, 0, count, stride, vector ) )
     return false;
-  if ( xform.m_xform == NULL )
-    return false;
   if (count == 0)
     return true;
 
@@ -1556,8 +1550,6 @@ ON_TransformVectorList(
 
   if ( !ON_IsValidPointList( dim, 0, count, stride, vector ) )
     return false;
-  if ( xform.m_xform == NULL )
-    return false;
   if (count == 0)
     return true;
 

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_mesh.cpp b/surface/src/3rdparty/opennurbs/opennurbs_mesh.cpp
@@ -3689,7 +3689,6 @@ bool ON_Mesh::CombineIdenticalVertices(
     ON_SimpleArray<int> remap_array(vertex_count);
 
     int remap_vertex_count = 0;
-    int merge_count = 0;
     int i0, i1, k;
 
     struct tagMESHPOINTS mp;
@@ -3738,8 +3737,6 @@ bool ON_Mesh::CombineIdenticalVertices(
       {
         if ( CompareMeshPoint( mp.p0+index[i0], mp.p0+index[i1], &mp ) )
           break;
-        else
-          merge_count++;
       }
       for ( /*empty*/; i0 < i1; i0++ )
       {

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_nurbssurface.cpp b/surface/src/3rdparty/opennurbs/opennurbs_nurbssurface.cpp
@@ -143,7 +143,7 @@ ON_BOOL32 ON_NurbsSurface::SetDomain(
             )
 {
   bool rc = false;
-  if ( m_order[dir] >= 2 && m_cv_count[dir] >= m_order[dir] && m_knot && t0 < t1 ) {
+  if ( m_order[dir] >= 2 && m_cv_count[dir] >= m_order[dir] && nullptr != m_knot[dir] && t0 < t1 ) {
     const double k0 = m_knot[dir][m_order[dir]-2];
     const double k1 = m_knot[dir][m_cv_count[dir]-1];
     if ( k0 == t0 && k1 == t1 )

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_nurbsvolume.cpp b/surface/src/3rdparty/opennurbs/opennurbs_nurbsvolume.cpp
@@ -533,7 +533,7 @@ ON__UINT32 ON_NurbsCage::DataCRC(ON__UINT32 current_remainder) const
       for ( j = 0; j < m_cv_count[1]; j++ )
       {
         cv = CV(i,j,0);
-        for (k = 0; i < m_cv_count[2]; k++ )
+        for (k = 0; k < m_cv_count[2]; k++ )
         {
           current_remainder = ON_CRC32(current_remainder,sizeof_cv,cv);
           cv += m_cv_stride[2];

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_object.cpp b/surface/src/3rdparty/opennurbs/opennurbs_object.cpp
@@ -840,7 +840,8 @@ void ON_ClassId::ConstructorHelper( const char* sClassName,
   {
     for ( ON_ClassId* p = m_p0; p; p = p->m_pNext )
     {
-      if ( !p->m_pBaseClassId && p->m_sBaseClassName ) {
+      if ( 0 == p->m_pBaseClassId && 0 != p->m_sBaseClassName[0] &&
+           0 == p->m_sBaseClassName[sizeof(p->m_sBaseClassName) / sizeof(p->m_sBaseClassName[0]) - 1] ) {
         if ( !strcmp( m_sClassName, p->m_sBaseClassName ) )
           p->m_pBaseClassId = this;
       }

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_point.cpp b/surface/src/3rdparty/opennurbs/opennurbs_point.cpp
@@ -668,60 +668,52 @@ ON_3dVector::PerpendicularTo(
 void ON_2dPoint::Transform( const ON_Xform& xform )
 {
   double xx,yy,ww;
-  if ( xform.m_xform ) {
-    ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][3];
-    if ( ww != 0.0 )
-      ww = 1.0/ww;
-    xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][3]);
-    yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][3]);
-    x = xx;
-    y = yy;
-  }
+  ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][3];
+  if ( ww != 0.0 )
+    ww = 1.0/ww;
+  xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][3]);
+  yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][3]);
+  x = xx;
+  y = yy;
 }
 
 void ON_3dPoint::Transform( const ON_Xform& xform )
 {
   double xx,yy,zz,ww;
-  if ( xform.m_xform ) {
-    ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3];
-    if ( ww != 0.0 )
-      ww = 1.0/ww;
-    xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]);
-    yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]);
-    zz = ww*(xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]);
-    x = xx;
-    y = yy;
-    z = zz;
-  }
+  ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3];
+  if ( ww != 0.0 )
+    ww = 1.0/ww;
+  xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]);
+  yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]);
+  zz = ww*(xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]);
+  x = xx;
+  y = yy;
+  z = zz;
 }
 
 void ON_4dPoint::Transform( const ON_Xform& xform )
 {
   double xx,yy,zz,ww;
-  if ( xform.m_xform ) {
-    xx = xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]*w;
-    yy = xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]*w;
-    zz = xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]*w;
-    ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3]*w;
-    x = xx;
-    y = yy;
-    z = zz;
-    w = ww;
-  }
+  xx = xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]*w;
+  yy = xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]*w;
+  zz = xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]*w;
+  ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3]*w;
+  x = xx;
+  y = yy;
+  z = zz;
+  w = ww;
 }
 
 void ON_2fPoint::Transform( const ON_Xform& xform )
 {
   double xx,yy,ww;
-  if ( xform.m_xform ) {
-    ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][3];
-    if ( ww != 0.0 )
-      ww = 1.0/ww;
-    xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][3]);
-    yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][3]);
-    x = (float)xx;
-    y = (float)yy;
-  }
+  ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][3];
+  if ( ww != 0.0 )
+    ww = 1.0/ww;
+  xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][3]);
+  yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][3]);
+  x = (float)xx;
+  y = (float)yy;
 }
 
 void ON_2fPoint::Rotate( 
@@ -767,32 +759,28 @@ void ON_3fPoint::Rotate(
 void ON_3fPoint::Transform( const ON_Xform& xform )
 {
   double xx,yy,zz,ww;
-  if ( xform.m_xform ) {
-    ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3];
-    if ( ww != 0.0 )
-      ww = 1.0/ww;
-    xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]);
-    yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]);
-    zz = ww*(xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]);
-    x = (float)xx;
-    y = (float)yy;
-    z = (float)zz;
-  }
+  ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3];
+  if ( ww != 0.0 )
+    ww = 1.0/ww;
+  xx = ww*(xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]);
+  yy = ww*(xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]);
+  zz = ww*(xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]);
+  x = (float)xx;
+  y = (float)yy;
+  z = (float)zz;
 }
 
 void ON_4fPoint::Transform( const ON_Xform& xform )
 {
   double xx,yy,zz,ww;
-  if ( xform.m_xform ) {
-    xx = xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]*w;
-    yy = xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]*w;
-    zz = xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]*w;
-    ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3]*w;
-    x = (float)xx;
-    y = (float)yy;
-    z = (float)zz;
-    w = (float)ww;
-  }
+  xx = xform.m_xform[0][0]*x + xform.m_xform[0][1]*y + xform.m_xform[0][2]*z + xform.m_xform[0][3]*w;
+  yy = xform.m_xform[1][0]*x + xform.m_xform[1][1]*y + xform.m_xform[1][2]*z + xform.m_xform[1][3]*w;
+  zz = xform.m_xform[2][0]*x + xform.m_xform[2][1]*y + xform.m_xform[2][2]*z + xform.m_xform[2][3]*w;
+  ww = xform.m_xform[3][0]*x + xform.m_xform[3][1]*y + xform.m_xform[3][2]*z + xform.m_xform[3][3]*w;
+  x = (float)xx;
+  y = (float)yy;
+  z = (float)zz;
+  w = (float)ww;
 }
 
 double ON_3fPoint::Fuzz( 

diff --git a/surface/src/3rdparty/opennurbs/opennurbs_rtree.cpp b/surface/src/3rdparty/opennurbs/opennurbs_rtree.cpp
@@ -1472,22 +1472,6 @@ std::size_t ON_RTree::SizeOf() const
 }
 
 
-static void NodeCountHelper( const ON_RTreeNode* node, std::size_t& node_count, std::size_t& wasted_branch_count, std::size_t& leaf_count )
-{
-  if ( 0 == node )
-    return;
-  node_count++;
-  wasted_branch_count += (ON_RTree_MAX_NODE_COUNT - node->m_count);
-  if ( node->m_level > 0 )
-  {
-    for ( int i = 0; i < node->m_count; i++ )
-    {
-      NodeCountHelper(node->m_branch[i].m_child,node_count,wasted_branch_count,leaf_count);
-    }
-  }
-  else
-    leaf_count += node->m_count;
-}
 
 void ON_RTree::RemoveAll()
 {

diff --git a/surface/src/on_nurbs/on_nurbs.cmake b/surface/src/on_nurbs/on_nurbs.cmake
@@ -49,8 +49,10 @@ set(ON_NURBS_SOURCES
 
 set(USE_UMFPACK 0 CACHE BOOL "Use UmfPack for solving sparse systems of equations (e.g. in surface/on_nurbs)")
 if(USE_UMFPACK)
+  find_package(CHOLMOD REQUIRED)
+  find_package(UMFPACK REQUIRED)
   set(ON_NURBS_SOURCES ${ON_NURBS_SOURCES} src/on_nurbs/nurbs_solve_umfpack.cpp)
-  set(ON_NURBS_LIBRARIES ${ON_NURBS_LIBRARIES} cholmod umfpack)
+  set(ON_NURBS_LIBRARIES ${ON_NURBS_LIBRARIES} SuiteSparse::CHOLMOD SuiteSparse::UMFPACK)
 else()
   set(ON_NURBS_SOURCES ${ON_NURBS_SOURCES} src/on_nurbs/nurbs_solve_eigen.cpp)
 endif()