Skip to content

PERF: Use the faster TransformPhysicalPointToContinuousIndex overload#2901

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Use-faster-TransformPhysicalPointToContinuousIndex-overload
Nov 27, 2021
Merged

PERF: Use the faster TransformPhysicalPointToContinuousIndex overload#2901
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Use-faster-TransformPhysicalPointToContinuousIndex-overload

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker commented Nov 25, 2021

Replaced image->TransformPhysicalPointToContinuousIndex(point, index)
calls by image->TransformPhysicalPointToContinuousIndex<T>(point),
which is slightly faster, as it does not figure out whether or not the
point is inside the image.

Declared variables that are initialized by
image->TransformPhysicalPointToContinuousIndex<T>(point) const, when
possible.

Follow-up to pull request #2873
commit eb6ac88
"PERF: Use the faster TransformPhysicalPointToIndex overload in filter"

@github-actions github-actions Bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module type:Performance Improvement in terms of compilation or execution time labels Nov 25, 2021
Replaced `image->TransformPhysicalPointToContinuousIndex(point, index)`
calls by `image->TransformPhysicalPointToContinuousIndex<T>(point)`,
which is slightly faster, as it does not figure out whether or not the
point is inside the image.

Declared variables that are initialized by
`image->TransformPhysicalPointToContinuousIndex<T>(point)` `const`, when
possible.

Follow-up to pull request InsightSoftwareConsortium#2873
commit eb6ac88
"PERF: Use the faster TransformPhysicalPointToIndex overload in filter"
@N-Dekker N-Dekker force-pushed the Use-faster-TransformPhysicalPointToContinuousIndex-overload branch from ebcad0a to a17ba41 Compare November 25, 2021 20:03
@N-Dekker N-Dekker marked this pull request as ready for review November 25, 2021 22:37
@blowekamp
Copy link
Copy Markdown
Member

The previous method returned a bool with the results of the point/index was inside the image. The places changed here ignored the results, so there is no risk of change in behavior or safety of lack of out-of-bounds checking?

@N-Dekker
Copy link
Copy Markdown
Contributor Author

The previous method returned a bool with the results of the point/index was inside the image. The places changed here ignored the results, so there is no risk of change in behavior or safety of lack of out-of-bounds checking?

Thanks for asking, @blowekamp This pull request should not cause any change in behavior, because it only replaced calls to the old bool returning member function in those cases where the return value was ignored.

For example here (in ITK v5.3rc02):

ContinuousIndexType index;
m_Image->TransformPhysicalPointToContinuousIndex(point, index);
/* Call IsInsideBuffer to test against BufferedRegion bounds.
 * TransformPhysicalPointToContinuousIndex tests against
 * LargestPossibleRegion */
bool isInside = IsInsideBuffer(index);
return isInside;

https://github.com/InsightSoftwareConsortium/ITK/blob/v5.3rc02/Modules/Core/ImageFunction/include/itkImageFunction.h#L174-L180

Adjusted the TransformPhysicalPointToContinuousIndex call by this pull request to:

const ContinuousIndexType index = m_Image->template TransformPhysicalPointToContinuousIndex<TCoordRep>(point);

at https://github.com/InsightSoftwareConsortium/ITK/pull/2901/files#diff-15de01b61211603e7d6639a9825a5b9b95b38223b0d2eda58f7295bc82b1c976R174

Right?

@blowekamp
Copy link
Copy Markdown
Member

@N-Dekker Yes. I think we are in agreement here. I just wanted to check. Perhaps a comment in the commit message indicated that the is inside check was not used would have short circuited the necessity of this discussion.

PR LGTM.

@dzenanz dzenanz merged commit 6a8569e into InsightSoftwareConsortium:master Nov 27, 2021
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jul 1, 2022
Follow-up to ITK pull request InsightSoftwareConsortium/ITK#2901 commit InsightSoftwareConsortium/ITK@6a8569e "PERF: Use the faster `TransformPhysicalPointToContinuousIndex` overload"
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jul 1, 2022
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jul 1, 2022
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Jul 1, 2022
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Apr 4, 2023
Replaced a call to `TransformPhysicalPointToContinuousIndex` with its faster
overload, within `SLICImageFilter::BeforeThreadedGenerateData()`.

Follow-up to pull request InsightSoftwareConsortium#2901
commit 6a8569e "PERF: Use the faster
`TransformPhysicalPointToContinuousIndex` overload"
hjmjohnson pushed a commit that referenced this pull request Apr 5, 2023
Replaced a call to `TransformPhysicalPointToContinuousIndex` with its faster
overload, within `SLICImageFilter::BeforeThreadedGenerateData()`.

Follow-up to pull request #2901
commit 6a8569e "PERF: Use the faster
`TransformPhysicalPointToContinuousIndex` overload"
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Mar 1, 2024
Fixed warnings like:

> warning C4834: discarding return value of function with [[nodiscard]] attribute

`TransformPhysicalPointToContinuousIndex(fixedPoint)` is faster than `TransformPhysicalPointToContinuousIndex(fixedPoint, voxelCoord)`, see also ITK pull request InsightSoftwareConsortium/ITK#2901 "PERF: Use the faster `TransformPhysicalPointToContinuousIndex` overload"
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Replaced a call to `TransformPhysicalPointToContinuousIndex` with its faster
overload, within `SLICImageFilter::BeforeThreadedGenerateData()`.

Follow-up to pull request InsightSoftwareConsortium#2901
commit 9d67c47 "PERF: Use the faster
`TransformPhysicalPointToContinuousIndex` overload"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module type:Performance Improvement in terms of compilation or execution time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants