Use the faster TransformPhysicalPointToIndex overload#2873
Merged
dzenanz merged 2 commits intoInsightSoftwareConsortium:masterfrom Nov 16, 2021
Merged
Conversation
github-actions
Bot
added
area:Core
dzenanz
approved these changes
Nov 15, 2021
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
The second is more of a style commit, than a performance one. Potentially split most of it into a separate style commit. Only calls inside large for loops (per pixel) count as real performance improvement!
| IndexType centerIndex; | ||
|
|
||
| outputImage->TransformPhysicalPointToIndex(center, centerIndex); | ||
| IndexType centerIndex = outputImage->TransformPhysicalPointToIndex(center); |
Member
There was a problem hiding this comment.
I am not sure how important performance is here, and other similar instances. But this improves style slightly, and saves a line of code.
hjmjohnson
approved these changes
Nov 15, 2021
Contributor
Author
@dzenanz I agree, it's mostly a style commit, it just might improve the performance. 😄 The first commit might improve the performance of user code, as it is part of a filter. Actually I was trying out the C++17 [[nodiscard]] attribute on the older (potentially slower) overload, Will adjust the commit message of the second commit, from PERF to STYLE. |
Replaced `image->TransformPhysicalPointToIndex(point, index)` call by `image->TransformPhysicalPointToIndex(point)`, which is slightly faster, as it does not figure out whether or not the point is inside the image. Follow-up to pull request InsightSoftwareConsortium#993 commit 0703516 "STYLE: Use image->TransformPhysicalPointToIndex(point), returning index"
Replaced `image->TransformPhysicalPointToIndex(point, index)` calls by `image->TransformPhysicalPointToIndex(point)`, which is slightly faster, as it does not figure out whether or not the point is inside the image. This is mostly a style improvement. It is recommended to only use the slightly slower overload, `TransformPhysicalPointToIndex(point, index)` when its `bool` return value is to be used. Follow-up to pull request InsightSoftwareConsortium#993 commit 0703516 "STYLE: Use image->TransformPhysicalPointToIndex(point), returning index"
N-Dekker
force-pushed
the
Use-faster-TransformPhysicalPointToIndex-overload
branch
from
d8d8473 to
919a3a6
Compare
Contributor
Author
|
I think this pull request is ready now, only CircleCi has failed, saying "Build timed out", and:
|
dzenanz
approved these changes
Nov 16, 2021
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
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. Follow-up to pull request InsightSoftwareConsortium#2873 commit eb6ac88 "PERF: Use the faster TransformPhysicalPointToIndex overload in filter"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
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. Follow-up to pull request InsightSoftwareConsortium#2873 commit eb6ac88 "PERF: Use the faster TransformPhysicalPointToIndex overload in filter"
N-Dekker
added a commit
to N-Dekker/ITK
that referenced
this pull request
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"
dzenanz
pushed a commit
that referenced
this pull request
Nov 27, 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"
N-Dekker
added a commit
to SuperElastix/elastix
that referenced
this pull request
Aug 29, 2024
Adjusted its `GPUGenerateData()` member function to call the faster `TransformPhysicalPointToIndex` overload.
Addresses MSVC compiler warnings saying:
itkGPUShrinkImageFilter.hxx(109,39): warning C4834: discarding return value of function with [[nodiscard]] attribute
Similar to pull request InsightSoftwareConsortium/ITK#2873 commit InsightSoftwareConsortium/ITK@eb6ac88 "Use the faster `TransformPhysicalPointToIndex` overload in filter", November 16, 2021.
N-Dekker
added a commit
to SuperElastix/elastix
that referenced
this pull request
Aug 29, 2024
Adjusted its `GPUGenerateData()` member function to call the faster `TransformPhysicalPointToIndex` overload.
Addresses MSVC compiler warnings saying:
itkGPUShrinkImageFilter.hxx(109,39): warning C4834: discarding return value of function with [[nodiscard]] attribute
Similar to pull request InsightSoftwareConsortium/ITK#2873 commit InsightSoftwareConsortium/ITK@eb6ac88 "Use the faster `TransformPhysicalPointToIndex` overload in filter", November 16, 2021.
hjmjohnson
pushed a commit
to hjmjohnson/ITK
that referenced
this pull request
May 6, 2026
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 264b885 "PERF: Use the faster TransformPhysicalPointToIndex overload in filter"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaced
image->TransformPhysicalPointToIndex(point, index)calls byimage->TransformPhysicalPointToIndex(point), which is slightly faster,as it does not figure out whether or not the point is inside the image.
Follow-up to pull request #993
commit 0703516
"STYLE: Use image->TransformPhysicalPointToIndex(point), returning index"