Skip to content

STYLE: Use image->TransformPhysicalPointToIndex(point), returning index #993

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.

Sign up for GitHub

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

Merged
hjmjohnson merged 1 commit into from
Jun 5, 2019
Merged

STYLE: Use image->TransformPhysicalPointToIndex(point), returning index #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 1 addition & 3 deletions Modules/Core/Mesh/include/itkWarpMeshFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ WarpMeshFilter< TInputMesh, TOutputMesh, TDisplacementField >

using InputPointType = typename InputMeshType::PointType;
using OutputPointType = typename OutputMeshType::PointType;
using IndexType = typename DisplacementFieldType::IndexType;
IndexType index;

OutputPointType displacedPoint;

Expand All @@ -119,7 +117,7 @@ WarpMeshFilter< TInputMesh, TOutputMesh, TDisplacementField >
while ( inputPoint != inPoints->End() )
{
const InputPointType & originalPoint = inputPoint.Value();
fieldPtr->TransformPhysicalPointToIndex(originalPoint, index);
const auto index = fieldPtr->TransformPhysicalPointToIndex(originalPoint);
displacement = fieldPtr->GetPixel(index);

for ( unsigned int i = 0; i < Dimension; i++ )
Expand Down
8 changes: 3 additions & 5 deletions Modules/Core/SpatialObjects/include/itkSpatialObjectToImageStatisticsCalculator.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ SpatialObjectToImageStatisticsCalculator< TInputImage, TInputSpatialObject,
maskImage->TransformIndexToPhysicalPoint( ind, pnt );
tPnt = maskSpatialObject->GetObjectToWorldTransform()
->TransformPoint( pnt );
m_Image->TransformPhysicalPointToIndex( tPnt, ind );
ind = m_Image->TransformPhysicalPointToIndex(tPnt);
Copy link
Copy Markdown
Member

@blowekamp blowekamp Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For cases like this without a new left hand side variable, do you think an extra copy operation is being introduced?

Copy link
Copy Markdown
Contributor Author

@N-Dekker N-Dekker Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your question @blowekamp I don't think returning an itk::Index object and copying this object into a variable (ind) is in any way more expensive than modifying the ind inside the function via pass-by-reference. itk::Index objects have cheap, fast copy-semantics (no dynamic memory allocation involved).

Copy link
Copy Markdown
Member

@blowekamp blowekamp Jun 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am quite curious if there is a performance difference. My intution ( experience) is that doing it in-place would be slightly better unless there is something similar to C++ RVO. It would be quite good to have concrete evidence of the performance difference.

Would you like to do the profiling?

Copy link
Copy Markdown
Contributor Author

@N-Dekker N-Dekker Jun 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just observed that the new (ITK5) TransformPhysicalPointToIndex(point) overload is more than 20% faster than the original TransformPhysicalPointToIndex(point, index). 😄

I also looked specifically at SpatialObjectToImageStatisticsCalculator::Update(). But honestly I could not observe a significant performance effect of this commit on calculator->Update(). Probably because the Update() does so many other things than just TransformPhysicalPointToIndex.

Copy link
Copy Markdown
Member

@blowekamp blowekamp Jun 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! It is always good to do this type of analysis to determine "best practices" and recommendations as we continue to move forward. Here we can say that the new method is recommended when the isInBounds is not needed.

mv[0] = m_Image->GetPixel(ind);
m_Sum += static_cast< AccumulateType >( mv[0] );
for ( unsigned int i = 1; i < Self::SampleDimension; i++ )
Expand All @@ -176,16 +176,14 @@ SpatialObjectToImageStatisticsCalculator< TInputImage, TInputSpatialObject,

Point< double, Self::ObjectDimension > ptMin;
Point< double, Self::ObjectDimension > ptMax;
IndexType indMin;
IndexType indMax;
SizeType size;
for ( unsigned int i = 0; i < Self::ObjectDimension; i++ )
{
ptMin[i] = bounds[i * 2];
ptMax[i] = bounds[i * 2 + 1];
}
m_Image->TransformPhysicalPointToIndex( ptMin, indMin );
m_Image->TransformPhysicalPointToIndex( ptMax, indMax );
auto indMin = m_Image->TransformPhysicalPointToIndex(ptMin);
auto indMax = m_Image->TransformPhysicalPointToIndex(ptMax);
IndexType imageIndex = m_Image->GetLargestPossibleRegion().GetIndex();
SizeType imageSize = m_Image->GetLargestPossibleRegion().GetSize();
for ( unsigned int i = 0; i < Self::ObjectDimension; i++ )
Expand Down
11 changes: 3 additions & 8 deletions Modules/Filtering/DisplacementField/include/itkDisplacementFieldTransform.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,7 @@ DisplacementFieldTransform<TParametersValueType, NDimensions>
::ComputeJacobianWithRespectToPosition( const InputPointType & point,
JacobianPositionType & jacobian ) const
{
IndexType idx;

this->m_DisplacementField->TransformPhysicalPointToIndex( point, idx );
const auto idx = m_DisplacementField->TransformPhysicalPointToIndex(point);
this->ComputeJacobianWithRespectToPosition( idx, jacobian );
}

Expand All @@ -177,8 +175,7 @@ DisplacementFieldTransform<TParametersValueType, NDimensions>
::ComputeInverseJacobianWithRespectToPosition( const InputPointType & point,
InverseJacobianPositionType & jacobian ) const
{
IndexType idx;
this->m_DisplacementField->TransformPhysicalPointToIndex(point, idx);
const auto idx = m_DisplacementField->TransformPhysicalPointToIndex(point);
this->ComputeJacobianWithRespectToPositionInternal( idx, jacobian, true );
}

Expand All @@ -190,9 +187,7 @@ DisplacementFieldTransform<TParametersValueType, NDimensions>
JacobianPositionType & jacobian,
bool useSVD ) const
{
IndexType idx;

this->m_DisplacementField->TransformPhysicalPointToIndex( point, idx );
const auto idx = m_DisplacementField->TransformPhysicalPointToIndex(point);
this->GetInverseJacobianOfForwardFieldWithRespectToPosition( idx, jacobian,
useSVD );
}
Expand Down
4 changes: 2 additions & 2 deletions Modules/Filtering/ImageGrid/include/itkShrinkImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ ShrinkImageFilter< TInputImage, TOutputImage >
// We wish to perform the following mapping of outputIndex to
// inputIndex on all points in our region
outputPtr->TransformIndexToPhysicalPoint(outputIndex, tempPoint);
inputPtr->TransformPhysicalPointToIndex(tempPoint, inputIndex);
inputIndex = inputPtr->TransformPhysicalPointToIndex(tempPoint);

// Given that the size is scaled by a constant factor eq:
// inputIndex = outputIndex * factorSize
Expand Down Expand Up @@ -217,7 +217,7 @@ ShrinkImageFilter< TInputImage, TOutputImage >
// We wish to perform the following mapping of outputIndex to
// inputIndex on all points in our region
outputPtr->TransformIndexToPhysicalPoint(outputIndex, tempPoint);
inputPtr->TransformPhysicalPointToIndex(tempPoint, inputIndex);
inputIndex = inputPtr->TransformPhysicalPointToIndex(tempPoint);

// Given that the size is scaled by a constant factor eq:
// inputIndex = outputIndex * factorSize
Expand Down
13 changes: 5 additions & 8 deletions Modules/Filtering/ImageIntensity/include/itkPolylineMask2DImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,12 @@ void PolylineMask2DImageFilter< TInputImage, TPolyline, TOutputImage >
VertexType endVertex;
VertexType pstartVertex;
VertexType tmpVertex;
ImageIndexType tmpIndex;

/* Check if the polyline coordinates are within the input image */
while ( piter != container->End() )
{
tmpVertex = piter.Value();
outputImagePtr->TransformPhysicalPointToIndex(tmpVertex, tmpIndex);
const auto tmpIndex = outputImagePtr->TransformPhysicalPointToIndex(tmpVertex);
if ( !outputImagePtr->GetBufferedRegion().IsInside(tmpIndex) )
{
itkExceptionMacro(<< "Polyline vertex is out of bounds (Vertex,Index): "
Expand Down Expand Up @@ -150,8 +149,6 @@ void PolylineMask2DImageFilter< TInputImage, TPolyline, TOutputImage >
tmpVertex = pstartVertex;
++piter;

ImageIndexType startImageIndex;
ImageIndexType endImageIndex;
ImageIndexType tmpImageIndex;
tmpImageIndex.Fill(0);

Expand All @@ -166,8 +163,8 @@ void PolylineMask2DImageFilter< TInputImage, TPolyline, TOutputImage >
startVertex = tmpVertex;
endVertex = piter.Value();

outputImagePtr->TransformPhysicalPointToIndex(startVertex, startImageIndex);
outputImagePtr->TransformPhysicalPointToIndex(endVertex, endImageIndex);
const auto startImageIndex = outputImagePtr->TransformPhysicalPointToIndex(startVertex);
const auto endImageIndex = outputImagePtr->TransformPhysicalPointToIndex(endVertex);

//itkDebugMacro(<<"Projection image (index,physical
// coordinate):"<<startImageIndex<<","<<startVertex<<std::endl);
Expand Down Expand Up @@ -213,8 +210,8 @@ void PolylineMask2DImageFilter< TInputImage, TPolyline, TOutputImage >
startVertex = tmpVertex;
endVertex = pstartVertex;

outputImagePtr->TransformPhysicalPointToIndex(startVertex, startImageIndex);
outputImagePtr->TransformPhysicalPointToIndex(endVertex, endImageIndex);
const auto startImageIndex = outputImagePtr->TransformPhysicalPointToIndex(startVertex);
const auto endImageIndex = outputImagePtr->TransformPhysicalPointToIndex(endVertex);

if ( endImageIndex[1] > startImageIndex[1] )
{
Expand Down
2 changes: 1 addition & 1 deletion Modules/Filtering/ImageIntensity/include/itkPolylineMaskImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ void PolylineMaskImageFilter< TInputImage, TPolyline, TVector, TOutputImage >
{
outputImagePtr->TransformIndexToPhysicalPoint(outputIt.GetIndex(), inputPoint);
outputPoint = this->TransformProjectPoint(inputPoint);
projectionImagePtr->TransformPhysicalPointToIndex(outputPoint, projectionImageIndex);
projectionImageIndex = projectionImagePtr->TransformPhysicalPointToIndex(outputPoint);

if ( !projectionImagePtr->GetBufferedRegion().IsInside(projectionImageIndex) )
{
Expand Down
2 changes: 1 addition & 1 deletion Modules/Filtering/LabelMap/include/itkAttributePositionLabelMapFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ AttributePositionLabelMapFilter<TImage, TAttributeAccessor, VPhysicalPosition>
{
point[i] = position[i];
}
this->GetOutput()->TransformPhysicalPointToIndex( point, idx );
idx = this->GetOutput()->TransformPhysicalPointToIndex( point );
}
else
{
Expand Down
3 changes: 1 addition & 2 deletions Modules/Numerics/Optimizersv4/include/itkObjectToObjectMetric.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,7 @@ ObjectToObjectMetric<TFixedDimension, TMovingDimension, TVirtualImage, TParamete
{
if( ! this->m_VirtualImage.IsNull() )
{
VirtualIndexType index;
this->m_VirtualImage->TransformPhysicalPointToIndex( point, index );
const auto index = m_VirtualImage->TransformPhysicalPointToIndex(point);
return this->GetVirtualRegion().IsInside( index );
}

Expand Down
6 changes: 2 additions & 4 deletions Modules/Registration/Common/include/itkBlockMatchingImageFilter.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,8 @@ BlockMatchingImageFilter< TFixedImage, TMovingImage, TFeatures, TDisplacements,
for ( SizeValueType idx = first, last = first + count; idx < last; idx++ )
{
FeaturePointsPhysicalCoordinates originalLocation = featurePoints->GetPoint( idx );
ImageIndexType fixedIndex;
fixedImage->TransformPhysicalPointToIndex( originalLocation, fixedIndex );
ImageIndexType movingIndex;
movingImage->TransformPhysicalPointToIndex( originalLocation, movingIndex );
const auto fixedIndex = fixedImage->TransformPhysicalPointToIndex(originalLocation);
const auto movingIndex = movingImage->TransformPhysicalPointToIndex(originalLocation);

// the block is selected for a minimum similarity metric
SimilaritiesValue similarity = NumericTraits< SimilaritiesValue >::ZeroValue();
Expand Down
3 changes: 1 addition & 2 deletions ...s/Registration/Metricsv4/include/itkImageToImageMetricv4GetValueAndDerivativeThreader.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,11 @@ ImageToImageMetricv4GetValueAndDerivativeThreader< ThreadedIndexedContainerParti
using ElementIdentifierType = typename TImageToImageMetricv4::VirtualPointSetType::MeshTraits::PointIdentifier;
const ElementIdentifierType begin = indexSubRange[0];
const ElementIdentifierType end = indexSubRange[1];
VirtualIndexType virtualIndex;
typename VirtualImageType::ConstPointer virtualImage = this->m_Associate->GetVirtualImage();
for( ElementIdentifierType i = begin; i <= end; ++i )
{
const VirtualPointType & virtualPoint = virtualSampledPointSet->GetPoint( i );
virtualImage->TransformPhysicalPointToIndex( virtualPoint, virtualIndex );
const auto virtualIndex = virtualImage->TransformPhysicalPointToIndex( virtualPoint );
this->ProcessVirtualPoint( virtualIndex, virtualPoint, threadId );
}
//Finalize per thread actions
Expand Down
4 changes: 2 additions & 2 deletions ...ation/Metricsv4/include/itkJointHistogramMutualInformationComputeJointPDFThreaderBase.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ JointHistogramMutualInformationComputeJointPDFThreaderBase< TDomainPartitioner,
{
JointPDFPointType jointPDFpoint;
this->m_Associate->ComputeJointPDFPoint( fixedImageValue, movingImageValue, jointPDFpoint );
JointPDFIndexType jointPDFIndex;
this->m_JointHistogramMIPerThreadVariables[threadId].JointHistogram->TransformPhysicalPointToIndex( jointPDFpoint, jointPDFIndex );
const auto jointPDFIndex =
m_JointHistogramMIPerThreadVariables[threadId].JointHistogram->TransformPhysicalPointToIndex( jointPDFpoint );
if( this->m_JointHistogramMIPerThreadVariables[threadId].JointHistogram->GetBufferedRegion().IsInside( jointPDFIndex ) )
{
typename JointHistogramType::PixelType jointHistogramPixel;
Expand Down