ENH: Let PointSet::Clone() do a "deep copy", instead of no copy at all

Documentation/docs/migration_guides/itk_6_migration_guide.md

@@ -97,3 +97,13 @@ Accessing outdated ITKv5 migration scripts
 git worktree add .../ITKv5.4 v5.4.0
 ls ../ITKv5/Utilities/ITKv5Preparation
 ```
+
+Class changes
+-------------
+
+The `Clone()` member function of `itk::PointSet` now does a "deep copy" of its
+data, creating a new instance that has a copy of the points, the point data and
+the region information properties of the original PointSet object. With previous
+ITK versions, `PointSet::Clone()` did not copy any data. (It previously just
+created a default-constructed PointSet object, like `PointSet::CreateAnother()`
+does.)

Modules/Core/Common/include/itkPointSet.h

@@ -162,6 +162,9 @@ class ITK_TEMPLATE_EXPORT PointSet : public PointSetBase<typename TMeshTraits::P
   void
   PrintSelf(std::ostream & os, Indent indent) const override;
 
+  LightObject::Pointer
+  InternalClone() const override;
+
 }; // End Class: PointSet
 } // end namespace itk
 

Modules/Core/Common/include/itkPointSet.hxx

@@ -143,6 +143,23 @@ PointSet<TPixelType, VDimension, TMeshTraits>::Graft(const DataObject * data)
   this->SetPointData(pointSet->m_PointDataContainer);
 }
 
+template <typename TPixelType, unsigned int VDimension, typename TMeshTraits>
+LightObject::Pointer
+PointSet<TPixelType, VDimension, TMeshTraits>::InternalClone() const
+{
+  LightObject::Pointer lightObject = Superclass::InternalClone();
+
+  if (auto * const clone = dynamic_cast<Self *>(lightObject.GetPointer()))
+  {
+    if (m_PointDataContainer)
+    {
+      clone->m_PointDataContainer = PointDataContainer::New();
+      clone->m_PointDataContainer->CastToSTLContainer() = m_PointDataContainer->CastToSTLConstContainer();
+    }
+    return lightObject;
+  }
+  itkExceptionMacro("downcast to type " << this->GetNameOfClass() << " failed.");
+}
 } // end namespace itk
 
 #endif

Modules/Core/Common/include/itkPointSetBase.h

@@ -67,6 +67,8 @@ class ITK_TEMPLATE_EXPORT PointSetBase : public DataObject
   /** \see LightObject::GetNameOfClass() */
   itkOverrideGetNameOfClassMacro(PointSetBase);
 
+  itkCloneMacro(Self);
+
   /** Convenient type alias obtained from TPointsContainer template parameter. */
   using PointType = typename TPointsContainer::Element;
   using CoordRepType = typename PointType::CoordRepType;
@@ -200,6 +202,9 @@ class ITK_TEMPLATE_EXPORT PointSetBase : public DataObject
   void
   PrintSelf(std::ostream & os, Indent indent) const override;
 
+  LightObject::Pointer
+  InternalClone() const override;
+
   // If the RegionType is ITK_UNSTRUCTURED_REGION, then the following
   // variables represent the maximum number of region that the data
   // object can be broken into, which region out of how many is

Modules/Core/Common/include/itkPointSetBase.hxx

@@ -350,6 +350,31 @@ PointSetBase<TPointsContainer>::VerifyRequestedRegion()
   return retval;
 }
 
+template <typename TPointsContainer>
+LightObject::Pointer
+PointSetBase<TPointsContainer>::InternalClone() const
+{
+  LightObject::Pointer lightObject = Superclass::InternalClone();
+
+  if (auto * const clone = dynamic_cast<Self *>(lightObject.GetPointer()))
+  {
+    if (m_PointsContainer)
+    {
+      clone->m_PointsContainer = TPointsContainer::New();
+      clone->m_PointsContainer->CastToSTLContainer() = m_PointsContainer->CastToSTLConstContainer();
+    }
+
+    clone->m_MaximumNumberOfRegions = m_MaximumNumberOfRegions;
+    clone->m_NumberOfRegions = m_NumberOfRegions;
+    clone->m_RequestedNumberOfRegions = m_RequestedNumberOfRegions;
+    clone->m_BufferedRegion = m_BufferedRegion;
+    clone->m_RequestedRegion = m_RequestedRegion;
+
+    return lightObject;
+  }
+  itkExceptionMacro("downcast to type " << this->GetNameOfClass() << " failed.");
+}
+
 // Destructor. Must be defined here (rather than inside the class definition) because it is pure virtual.
 template <typename TPointsContainer>
 PointSetBase<TPointsContainer>::~PointSetBase() = default;

Modules/Core/Common/test/itkPointSetGTest.cxx

@@ -73,6 +73,44 @@ TestSetPointsByCoordinates(TPointSet & pointSet)
     }
   }
 }
+
+
+template <typename TPointSet>
+void
+TestCloneDoesDeepCopy(const TPointSet & pointSet)
+{
+  const auto clone = pointSet.Clone();
+
+  // Using a const reference to the clone, because the non-const overloads of GetPointData() and GetPoints() might
+  // potentially modify the point set! (Specifically when the container pointers were initially null.)
+  const TPointSet & constClone = itk::Deref(clone.get());
+
+  if (const auto * const pointData = pointSet.GetPointData())
+  {
+    const auto * const clonedPointData = constClone.GetPointData();
+
+    ASSERT_NE(clonedPointData, nullptr);
+    EXPECT_NE(clonedPointData, pointData) << "It should not just be a shallow copy!";
+    EXPECT_EQ(clonedPointData->CastToSTLConstContainer(), pointData->CastToSTLConstContainer());
+  }
+  else
+  {
+    EXPECT_EQ(constClone.GetPointData(), nullptr);
+  }
+
+  if (const auto * const points = pointSet.GetPoints())
+  {
+    const auto * const clonedPoints = constClone.GetPoints();
+
+    ASSERT_NE(clonedPoints, nullptr);
+    EXPECT_NE(clonedPoints, points) << "It should not just be a shallow copy!";
+    EXPECT_EQ(clonedPoints->CastToSTLConstContainer(), points->CastToSTLConstContainer());
+  }
+  else
+  {
+    EXPECT_EQ(constClone.GetPoints(), nullptr);
+  }
+}
 } // namespace
 
 
@@ -113,3 +151,22 @@ TEST(PointSet, GraftDoesShallowCopy)
 
   check(*pointSet);
 }
+
+
+// Tests that `PointSet::Clone` copies the points and the data. So it does a "deep copy".
+TEST(PointSet, CloneDoesDeepCopy)
+{
+  // First check an empty point set:
+  TestCloneDoesDeepCopy(*itk::PointSet<int>::New());
+
+  // Then check a non-empty 2-D point set with `double` data:
+  using PixelType = double;
+  using PointSetType = itk::PointSet<PixelType, 2>;
+  using PointType = PointSetType::PointType;
+
+  const auto pointSet = PointSetType::New();
+  pointSet->SetPoints(itk::MakeVectorContainer<PointType>({ PointType(), itk::MakeFilled<PointType>(1.0f) }));
+  pointSet->SetPointData(itk::MakeVectorContainer<PixelType>({ 0.0, 1.0, 2.0 }));
+
+  TestCloneDoesDeepCopy(*pointSet);
+}