Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SetPointsByCoordinates to PointSet, add warning to SetPoints(PointsVectorContainer *) #4850

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion Modules/Core/Common/include/itkPointSet.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,15 @@ class ITK_TEMPLATE_EXPORT PointSet : public DataObject
void
SetPoints(PointsContainer *);

/** Set the points container using a 1D vector. */
/** Set the points container using a 1D vector.
\warning This member function is unsafe. It may just work, but it may also lead to undefined behavior. */
void
SetPoints(PointsVectorContainer *);

/** Sets the points by specifying its coordinates. */
void
SetPointsByCoordinates(const std::vector<CoordRepType> & coordinates);

/** Get the points container. */
PointsContainer *
GetPoints();
Expand Down
52 changes: 52 additions & 0 deletions Modules/Core/Common/include/itkPointSet.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,64 @@ PointSet<TPixelType, VDimension, TMeshTraits>::SetPoints(PointsVectorContainer *
{
itkExceptionMacro("Number of entries in given 1d array incompatible with the point dimension");
}

// Note: this cast is unsafe. It may lead to undefined behavior.
auto * pointsPtr = reinterpret_cast<PointsContainer *>(points);

m_PointsContainer = pointsPtr;
this->Modified();
}


template <typename TPixelType, unsigned int VDimension, typename TMeshTraits>
void
PointSet<TPixelType, VDimension, TMeshTraits>::SetPointsByCoordinates(const std::vector<CoordRepType> & coordinates)
{
itkDebugMacro("Setting the points to the specified coordinates");

const size_t numberOfCoordinates = coordinates.size();

if (numberOfCoordinates % PointDimension != 0)
{
itkExceptionMacro("Number of specified coordinates incompatible with the point dimension");
}

const size_t numberOfPoints = numberOfCoordinates / PointDimension;

if (m_PointsContainer == nullptr)
{
m_PointsContainer = PointsContainer::New();
}

using STLContainerType = typename PointsContainer::STLContainerType;

STLContainerType & points = m_PointsContainer->CastToSTLContainer();
points.clear();

if constexpr (std::is_same_v<STLContainerType, std::vector<PointType>>)
{
// STLContainerType is either an std::vector or an std::map. Only when it is an std::vector, it should be resized.
// std::map does not have a resize function.
points.resize(numberOfPoints);
dzenanz marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
static_assert(std::is_same_v<STLContainerType, std::map<PointIdentifier, PointType>>);
}

auto coordinateIterator = coordinates.cbegin();

for (PointIdentifier pointIdentifier{}; pointIdentifier < numberOfPoints; ++pointIdentifier)
{
PointType & point = points[pointIdentifier];
std::copy_n(coordinateIterator, PointDimension, point.begin());
coordinateIterator += PointDimension;
}

this->Modified();
}


template <typename TPixelType, unsigned int VDimension, typename TMeshTraits>
auto
PointSet<TPixelType, VDimension, TMeshTraits>::GetPoints() -> PointsContainer *
Expand Down
1 change: 1 addition & 0 deletions Modules/Core/Common/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1748,6 +1748,7 @@ set(ITKCommonGTests
itkOffsetGTest.cxx
itkOptimizerParametersGTest.cxx
itkPointGTest.cxx
itkPointSetGTest.cxx
itkRGBAPixelGTest.cxx
itkRGBPixelGTest.cxx
itkShapedImageNeighborhoodRangeGTest.cxx
Expand Down
82 changes: 82 additions & 0 deletions Modules/Core/Common/test/itkPointSetGTest.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*=========================================================================
*
* Copyright NumFOCUS
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0.txt
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*=========================================================================*/

// First include the header file to be tested:
#include "itkPointSet.h"
#include "../../QuadEdgeMesh/include/itkQuadEdgeMeshTraits.h"
#include <gtest/gtest.h>
#include <algorithm> // For equal.
#include <numeric> // For iota.

namespace
{
template <typename TPointSet>
void
TestSetPointsByCoordinates(TPointSet & pointSet)
{
using CoordRepType = typename TPointSet::CoordRepType;

static constexpr auto PointDimension = TPointSet::PointDimension;

for (unsigned int numberOfCoordinates{ 1 }; numberOfCoordinates < PointDimension; ++numberOfCoordinates)
{
// SetPointsByCoordinates is expected to throw an exception when the specified number of coordinates is not a
// multiple of PointDimension.
EXPECT_THROW(pointSet.SetPointsByCoordinates(std::vector<CoordRepType>(numberOfCoordinates)), itk::ExceptionObject);
}

for (const unsigned int numberOfPoints : { 2, 1, 0 })
{
std::vector<CoordRepType> coordinates(numberOfPoints * PointDimension);

// Just make sure that all coordinates have different values, for the purpose of the test.
std::iota(coordinates.begin(), coordinates.end(), CoordRepType());
{
const auto modifiedTime = pointSet.GetMTime();
pointSet.SetPointsByCoordinates(coordinates);
EXPECT_GT(pointSet.GetMTime(), modifiedTime);
}

using PointsContainerType = typename TPointSet::PointsContainer;
using PointIdentifier = typename TPointSet::PointIdentifier;
using PointType = typename TPointSet::PointType;

const typename PointsContainerType::ConstPointer points = pointSet.GetPoints();

ASSERT_NE(points, nullptr);
ASSERT_EQ(points->size(), numberOfPoints);

const typename PointsContainerType::STLContainerType & stlContainer = points->CastToSTLConstContainer();
auto coordinateIterator = coordinates.cbegin();

for (PointIdentifier pointIdentifier{}; pointIdentifier < numberOfPoints; ++pointIdentifier)
{
const PointType & point = stlContainer.at(pointIdentifier);
EXPECT_TRUE(std::equal(point.cbegin(), point.cend(), coordinateIterator));
coordinateIterator += PointDimension;
}
}
}
} // namespace


TEST(PointSet, SetPointsByCoordinates)
{
TestSetPointsByCoordinates(*itk::PointSet<int>::New());
TestSetPointsByCoordinates(*itk::PointSet<double, 2, itk::QuadEdgeMeshTraits<double, 2, bool, bool>>::New());
}