InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

ENH: Migrate two-argument `Transform*Point` call sites to the return-value overloads

Open
#6,743 0 comments 0 reactions 0 assignees View on GitHub
type:Enhancement type:Style
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

303 in-tree call sites still use the ITKv4 two-argument output-parameter
form of `Transform*Point` and silently discard the `bool` return. The
single-argument return-value overloads have existed since #868 (2019).

Measured counts on current main

Over `Modules/` and `Examples/` (`*.h`, `*.hxx`, `*.cxx`):

| Metric | Count |
|---|---|
| All `Transform(Continuous)?IndexToPhysicalPoint(` occurrences | 352 |
| Two-arg calls in statement position (return discarded) | **303** |
| — of which `TransformContinuousIndexToPhysicalPoint` | 69 |
| — of which `TransformIndexToPhysicalPoint` | 234 |
| In `*/test/` | 156 |
| In `*/include/` or `*/src/` | 137 |

Representative sites

```
Modules/Registration/Montage/include/itkTileMontage.hxx:381
Modules/Registration/Montage/include/itkTileMontage.hxx:395
Modules/Registration/RegistrationMethodsv4/include/itkImageRegistrationMethodv4.hxx:953
Modules/Registration/RegistrationMethodsv4/include/itkImageRegistrationMethodv4.hxx:989
Modules/Registration/RegistrationMethodsv4/include/itkTimeVaryingBSplineVelocityFieldImageRegistrationMethod.hxx:528
Modules/Registration/RegistrationMethodsv4/include/itkTimeVaryingBSplineVelocityFieldImageRegistrationMethod.hxx:799
Modules/Registration/Metricsv4/test/itkImageToImageMetricv4RegistrationTest.cxx:138
```

Proposed conversion and constraints

```cpp
// before
PointType p;
field->TransformIndexToPhysicalPoint(index, p);

// after
const auto p = field->TransformIndexToPhysicalPoint(index);
```

This also removes the declare-then-assign pattern at the same time.

Constraints:
- Sites that actually test the `bool` (inside `if`/`while`/an assignment)
must be left alone — the counts above exclude them.
- Suggested batching: one PR per module group so review stays tractable.
- `TransformPhysicalPointToContinuousIndex` should be swept in the same
pass where the return value is likewise unused.

Contributor guide

Open the contributing guide

Research direction

Start with the representative call sites in Modules/Registration/Montage/include/itkTileMontage.hxx, Modules/Registration/RegistrationMethodsv4/include/itkImageRegistrationMethodv4.hxx, and the listed metric test. Repeat the stated grep over Modules/ and Examples/ to identify statement-position two-argument calls, preserving sites that test the bool return. Done means migrating the unused-return calls, including the related TransformPhysicalPointToContinuousIndex sweep, in reviewable module batches.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-vision
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.