Skip to content

BUG: Backport BresenhamLine integer-only algorithm and double precision fix#6054

Merged
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:release-5.4from
hjmjohnson:backport-bresenham-fix
Apr 14, 2026
Merged

BUG: Backport BresenhamLine integer-only algorithm and double precision fix#6054
hjmjohnson merged 2 commits intoInsightSoftwareConsortium:release-5.4from
hjmjohnson:backport-bresenham-fix

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Backport two BresenhamLine bug fixes from main to release-5.4. Part of #6051.

  1. 59e3c069 — Integer-only N-dimensional Bresenham for BuildLine(Index, Index), avoiding floating-point direction conversion entirely
  2. b7248b1c — Use double precision (not float) for ending index computation in BuildLine(Direction, length)
Cherry-pick adaptation

Required one API adaptation: itk::Math::Absolute() does not exist on release-5.4 (added on main as part of Math modernization). Replaced with itk::Math::abs() which is the release-5.4 equivalent.

Local build verified: compiles cleanly with the Path and Voronoi modules enabled.

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances area:Core Issues affecting the Core module labels Apr 13, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 13, 2026

Greptile Summary

This PR backports two BresenhamLine bug fixes from main to release-5.4: (1) a full integer-only N-dimensional Bresenham algorithm for BuildLine(Index, Index) that avoids floating-point direction conversion, and (2) a double-precision variable for the ending-index computation in BuildLine(Direction, length). The itk::Math::Absoluteitk::Math::abs substitution for release-5.4 API compatibility is correct.

Confidence Score: 5/5

Safe to merge; both algorithms are correct and the single finding is a minor precision opportunity, not a defect

Both bug fixes are algorithmically sound — the integer Bresenham correctly guarantees exact start/end points, and the double-precision variable genuinely improves the multiplication step. The only finding (P2) is that the division on line 44 still runs in float because Direction's components are float, so one further cast could be applied, but the code as-is is strictly better than before and introduces no regressions.

No files require special attention

Important Files Changed

Filename Overview
Modules/Core/Common/include/itkBresenhamLine.hxx Two backported bug fixes: integer-only Bresenham for BuildLine(Index,Index) and double-precision variable for BuildLine(Direction,length); logic is correct but the float→double precision improvement in the division on line 44 is incomplete due to LType using float components

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["BuildLine(Direction, length)"] --> B["Normalize direction"]
    B --> C["Find dominant axis (max |component|)"]
    C --> D["Compute euclideanLineLen = (length-1) / |Direction[mainAxis]| as double"]
    D --> E["Compute LastIndex: double × float → double, RoundHalfIntegerUp"]
    E --> F["Call BuildLine(StartIndex, LastIndex)"]
    F --> G["Convert IndexArray → OffsetArray"]

    H["BuildLine(p0, p1)"] --> I["Compute absDeltas and step (integer only)"]
    I --> J["Find mainAxis (max |delta|)"]
    J --> K["numPixels = maxAbsDelta + 1"]
    K --> L{"For s = 0..numPixels-1"}
    L --> M["Push currentIndex"]
    M --> N["Advance main axis by step[mainAxis]"]
    N --> O["Each secondary axis: accumulate 2×absDelta; if ≥ maxAbsDelta → step, subtract 2×maxAbsDelta"]
    O --> L
    L -->|done| P["Return IndexArray guaranteed to end at p1"]
Loading

Reviews (1): Last reviewed commit: "BUG: BresenhamLine - use double precisio..." | Re-trigger Greptile

Comment thread Modules/Core/Common/include/itkBresenhamLine.hxx Outdated
slavust added 2 commits April 14, 2026 06:28
Replace the floating-point delegation through BuildLine(Direction,
length) with a direct integer-only N-dimensional Bresenham that
guarantees exact start and end points by construction.

The previous implementation converted integer endpoints to a
floating-point direction vector, computed Chebyshev distance for
the line length, then reconstructed LastIndex via rounding. This
introduced endpoint error for lines where the dominant-axis
projection differed from the Euclidean length (e.g., {0,0,0} to
{250,250,1} would overshoot to {251,251,0}).

Reference: classic Bresenham extended to N-D as used by scikit-image
(line_nd), OpenCV (LineIterator), and VTK (vtkLine).

(cherry picked from commit 59e3c06)
…putation

Use explicit static_cast<double>() on both operands of the division
computing euclideanLineLen so the division evaluates in double
precision, not float (LType = Vector<float>).

Assisted-by: Greptile — identified float-precision division in BuildLine

(cherry picked from commit b7248b1)
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

@hjmjohnson thanks!

@thewtex thewtex added this to the ITK 5.4.6 milestone Apr 14, 2026
@hjmjohnson hjmjohnson merged commit 22f6d27 into InsightSoftwareConsortium:release-5.4 Apr 14, 2026
21 of 22 checks passed
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 type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants