Skip to content

ENH: Ingest ITKCuberille into Modules/Filtering - #6205

Merged
hjmjohnson merged 115 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-cuberille
May 5, 2026
Merged

hjmjohnson merged 115 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-cuberille

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

Ingests Cuberille from the standalone remote module InsightSoftwareConsortium/ITKCuberille into ITK at Modules/Filtering/Cuberille/ using the v4 ingestion pipeline (PR #6204). Stacked on #6204 — that PR must merge first.

Local validation: pre-commit run --all-files clean; CuberilleTestDriver builds; 25/25 Cuberille tests pass locally in 5.3s.

Pipeline metrics
Metric Value
Upstream commits → rewritten 200 → 106
Upstream merges → preserved 59 → 30 (no linearization, per feedback_ingest_merge_topology.md)
Blobs scanned in sanitize 148 (cxx=103, py=1, cmake=28, text=5, skip=11)
Blobs reformatted 95 (clang-format / black / gersemi / trailing-ws / EOF)
Commit subjects auto-prefixed 36
Merges visible in git log -- Modules/Filtering/Cuberille 6
v4 deny-pass bug surfaced and fixed during this run

The first attempt at this ingest leaked test/Docker/{Dockerfile,build.sh,run.sh,test.sh} into ITK history, because the v4 ingest driver had only a whitelist pass — no --invert-paths --path-glob deny-pass for CI/packaging scaffolding that lives inside whitelisted directories. Fixed in PR #6204 by porting v3's deny-glob list (Docker, Jenkinsfile, .github, azure-pipelines, .clang-format inside the module subtree, etc.). Re-running this ingest after the fix produced a clean tree with no scaffolding leakage.

Auto-prefix decisions (sample)
prefix counts across the 36 changed subjects:
  ENH:   28
  STYLE:  4
  COMP:   2
  BUG:    1
  DOC:    1

Every commit on the rewritten history starts with a valid ITK prefix (BUG:/COMP:/DOC:/ENH:/PERF:/STYLE:/WIP:). The Utilities/Maintenance/RemoteModuleIngest/sanitize-history.py log records the (sha, chosen-prefix, original-subject) for each decision so the operator can audit before merge.

Follow-up commits in this PR

After the merge commit (ENH: Ingest ITKCuberille into Modules/Filtering), the standard cleanup commits land on top:

  • STYLE: Apply gersemi 0.19.3 to Cuberille wrapping test CMakeLists. The v4 sanitize pass runs gersemi 0.24.0; ITK's pre-commit pins 0.19.3. The post-merge pre-commit run --all-files gate caught one CMake file that 0.24.0 considered clean but 0.19.3 wanted re-formatted. Working as designed.
  • DOC: Add Modules/Filtering/Cuberille/README.md pointing at the archived upstream.
  • COMP: Remove Modules/Remote/Cuberille.remote.cmake (now in-tree).
  • ENH: Enable Module_Cuberille:BOOL=ON in pyproject.toml configure-ci.
Phase B (upstream archival) — deferred

Per the v4 design (PR #6204), the upstream archival step is intentionally separated. After this PR merges into ITK main, run:

~/src/ITK/Utilities/Maintenance/RemoteModuleIngest/archive-remote-module.sh \
    Cuberille --itk-pr-number <THIS-PR-NUMBER>

That step pushes the deletion + MIGRATION_README.md → README.md promotion to upstream and flips archived=true. This PR makes no upstream changes; the upstream ITKCuberille repository remains live until the operator manually runs Phase B.

root and others added 30 commits August 22, 2011 09:47
QuadricDecimationQuadEdgeMeshFilter.
Refactor as an ITKv4 module.
Add Docker test configuration.
COMP: Remove unused typedefs in the test.
Addresses:

/home/matt/src/ITK/Modules/Remote/Cuberille/include/itkCuberilleImageToMeshFilter.h: In instantiation of ‘class itk::CuberilleImageToMeshFilter<itk::Image<unsigned char, 3u>, itk::Mesh<unsigned char, 3u>, itk::LinearInterpolateImageFunction<itk::Image<unsigned char, 3u> > >’:
/home/matt/src/ITK/Modules/Remote/Cuberille/test/CuberilleTest01.cxx:123:16:   required from here
/home/matt/src/ITK/Modules/Core/Common/include/itkProcessObject.h:522:16: warning: ‘virtual void itk::ProcessObject::SetInput(const DataObjectIdentifierType&, itk::DataObject*)’ was hidden [-Woverloaded-virtual]
   virtual void SetInput(const DataObjectIdentifierType & key, DataObject *input);
                ^
In file included from /home/matt/src/ITK/Modules/Remote/Cuberille/include/itkCuberilleImageToMeshFilter.h:347:0,
                 from /home/matt/src/ITK/Modules/Remote/Cuberille/test/CuberilleTest01.cxx:27:
/home/matt/src/ITK/Modules/Remote/Cuberille/include/itkCuberilleImageToMeshFilter.hxx:53:1: warning:   by ‘void itk::CuberilleImageToMeshFilter<TInputImage, TOutputMesh, TInterpolator>::SetInput(const InputImageType*) [with TInputImage = itk::Image<unsigned char, 3u>; TOutputMesh = itk::Mesh<unsigned char, 3u>; TInterpolator = itk::LinearInterpolateImageFunction<itk::Image<unsigned char, 3u> >; itk::CuberilleImageToMeshFilter<TInputImage, TOutputMesh, TInterpolator>::InputImageType = itk::Image<unsigned char, 3u>]’ [-Woverloaded-virtual]
 CuberilleImageToMeshFilter<TInputImage,TOutputMesh,TInterpolator>
 ^
COMP: Address SetInput override virtual warning.
The defined directory is not correct when built as a Remote module.
BUG: Fix test temporary output dir.
hjmjohnson and others added 5 commits May 4, 2026 17:29
Brings Cuberille from a configure-time remote fetch into the ITK
source tree at Modules/Filtering/Cuberille/ using the v4 ingestion
pipeline (whitelist filter-repo + per-commit clang-format + black +
commit-prefix sanitization).

Upstream repo:  https://github.com/InsightSoftwareConsortium/ITKCuberille.git
Upstream tip:   4ba447f256a93428e56507178f92124bb91e3f3c
Ingest date:    2026-05-04
Whitelist:      Cuberille.list

Per-commit transforms applied across all 105 commits:
  - filter-repo --paths-from-file (whitelist)
  - filter-repo --to-subdirectory-filter Modules/Filtering/Cuberille
  - clang-format -style=file (ITK main's .clang-format) for *.cxx/.h/.hxx/...
  - black for *.py
  - heuristic ITK prefix added to commit subjects without one

Merge topology preserved: 59 -> 30 merge(s).

Primary author: Matt McCormick <matt.mccormick@kitware.com>

Co-authored-by: Davis Marc Vigneault <davis.vigneault@gmail.com>
Co-authored-by: Davis Vigneault <davis.vigneault@gmail.com>
Co-authored-by: DVigneault <davis.vigneault@gmail.com>
Co-authored-by: Dženan Zukić <dzenan.zukic@kitware.com>
Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hastings Greer <hastings.greer@kitware.com>
Co-authored-by: Jon Haitz Legarreta <jhlegarreta@vicomtech.org>
Co-authored-by: Jon Haitz Legarreta Gorroño <jon.haitz.legarreta@gmail.com>
Co-authored-by: Mathew Seng <mathewseng@gmail.com>
Co-authored-by: Matt McCormick <matt@mmmccormick.com>
Co-authored-by: root <root@insight-journal.org>
Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com>
Co-authored-by: Zach Williamson <zachary-williamson@uiowa.edu>
@hjmjohnson hjmjohnson changed the title ENH: Ingest ITKCuberille into Modules/Filtering (stacked on #6204) ENH: Ingest ITKCuberille into Modules/Filtering May 4, 2026
hjmjohnson added 5 commits May 4, 2026 17:38
The four implementation-control macros (DEBUG_PRINT,
USE_GRADIENT_RECURSIVE_GAUSSIAN, USE_ADVANCED_PROJECTION,
USE_LINESEARCH_PROJECTION) were defined unconditionally at the top of
itkCuberilleImageToMeshFilter.h. Any translation unit including the
public header silently inherited DEBUG_PRINT=0, masking a same-named
macro any consumer might want to test with #ifdef.

Move the four #define lines to itkCuberilleImageToMeshFilter.hxx so
they remain visible to the template implementation that depends on
them via #if, but are no longer emitted into consumer translation
units that only include the .h.

Greptile P1 on PR InsightSoftwareConsortium#6205.
Both operators applied >= / <= on m_Y in their first branch, which
short-circuited to true/false whenever the Y values were equal —
before the X tiebreaker could be evaluated. E.g. {Y=1,X=0} >=
{Y=1,X=5} resolved (1 >= 1) == true and returned true even though
the lhs is strictly less than the rhs.

Replace the first-branch comparison with strict > / < so equal-Y
cases fall through to the X tiebreaker, mirroring the existing
operator> and operator< definitions.

std::map calls only operator<, so existing callers do not observe
the bug, but the broken operators are class-level API and would
silently misbehave under direct use.

Greptile P1 on PR InsightSoftwareConsortium#6205.
Convert the 20 Cuberille tests registered via plain add_test() to
itk_add_test() so they pick up the standard ITK CTest labels and
properties (matching the three already-itk_add_test'd entries below
them).  Tests with bare add_test are invisible to dashboard-specific
test selectors that filter on those labels.

GTest conversion of these tests is a separate follow-up.

Greptile P2 on PR InsightSoftwareConsortium#6205.
DEBUG_PRINT was a debug-tracing toggle defined to 0 (compile-time disabled)
since the module's first commit; the eight #if DEBUG_PRINT...#endif blocks
they guarded contained std::cout traces from algorithm bring-up that were
never re-enabled.  Removing the macro and the dead code per the reviewer's
preference for cleaner module headers.  Behavior is unchanged because
all guarded blocks were #if 0.
Replaces three preprocessor toggles
  USE_GRADIENT_RECURSIVE_GAUSSIAN
  USE_ADVANCED_PROJECTION
  USE_LINESEARCH_PROJECTION
with class-level `static constexpr bool` members of the same intent.
Preprocessor `#if FLAG` becomes `if constexpr (Flag)` and a
`#if/#else` selecting between two GradientFilterType type aliases
becomes `std::conditional_t<...>`.

Eliminates global-namespace preprocessor pollution flagged in the
Greptile draft review (P1) and matches dzenanz's reviewer preference
for class-level configuration over file-level macros.  Default values
remain false (matching the prior `#define X 0`) so behavior is
unchanged.

The dead linesearch branch had a stray top-level `break;` outside any
loop (a long-standing bit-rot in the disabled-since-2014 alternative
path).  Replaced with `return;` (the void function's natural early-out)
so the branch parses cleanly under `if constexpr` instantiation rules.
@hjmjohnson
hjmjohnson requested a review from dzenanz May 4, 2026 23:01
@hjmjohnson
hjmjohnson merged commit 9658520 into InsightSoftwareConsortium:main May 5, 2026
18 of 19 checks passed
@hjmjohnson hjmjohnson added this to the ITK 6.0 Release Candidate 1 milestone May 5, 2026
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
The four implementation-control macros (DEBUG_PRINT,
USE_GRADIENT_RECURSIVE_GAUSSIAN, USE_ADVANCED_PROJECTION,
USE_LINESEARCH_PROJECTION) were defined unconditionally at the top of
itkCuberilleImageToMeshFilter.h. Any translation unit including the
public header silently inherited DEBUG_PRINT=0, masking a same-named
macro any consumer might want to test with #ifdef.

Move the four #define lines to itkCuberilleImageToMeshFilter.hxx so
they remain visible to the template implementation that depends on
them via #if, but are no longer emitted into consumer translation
units that only include the .h.

Greptile P1 on PR InsightSoftwareConsortium#6205.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Both operators applied >= / <= on m_Y in their first branch, which
short-circuited to true/false whenever the Y values were equal —
before the X tiebreaker could be evaluated. E.g. {Y=1,X=0} >=
{Y=1,X=5} resolved (1 >= 1) == true and returned true even though
the lhs is strictly less than the rhs.

Replace the first-branch comparison with strict > / < so equal-Y
cases fall through to the X tiebreaker, mirroring the existing
operator> and operator< definitions.

std::map calls only operator<, so existing callers do not observe
the bug, but the broken operators are class-level API and would
silently misbehave under direct use.

Greptile P1 on PR InsightSoftwareConsortium#6205.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Convert the 20 Cuberille tests registered via plain add_test() to
itk_add_test() so they pick up the standard ITK CTest labels and
properties (matching the three already-itk_add_test'd entries below
them).  Tests with bare add_test are invisible to dashboard-specific
test selectors that filter on those labels.

GTest conversion of these tests is a separate follow-up.

Greptile P2 on PR InsightSoftwareConsortium#6205.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…est-cuberille

ENH: Ingest ITKCuberille into Modules/Filtering
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 7, 2026
Fixes the four classes of ghostflow errors that surfaced on the first
Cuberille v4 ingest (PR InsightSoftwareConsortium#6205) — the only allowable remaining error
should be the unavoidable upstream root commit.

  - Strip *.orig / *.rej / *.BACKUP.* / *.LOCAL.* / *.REMOTE.* /
    *.BASE.* merge-conflict-artifact files from history (catches
    leftover-conflict-marker errors like the .h.orig on Cuberille).
  - sanitize-history.py: cap every commit subject at 78 chars (ghostflow
    / kw-commit-msg rule); excess words move into the body.
  - sanitize-history.py: insert a blank line between subject and body
    when the original commit message lacked one (two-line subject error).
  - sanitize-history.py: clear the executable bit (mode 100755 -> 100644)
    on text-file extensions so root-commit "executable permissions but
    file does not look executable" stops firing.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 8, 2026
Validated on Cuberille (InsightSoftwareConsortium#6205): ghostflow now reports only the
unavoidable upstream root-commit error.
- truncate_subject_to_body() for subjects >78 chars
- ensure_blank_line_after_subject() inserts missing blank line
- commit_callback clears exec-bit on text-classified blobs
- deny-pass strips *.orig / *.rej / *.BACKUP.* / *.LOCAL.* /
  *.REMOTE.* / *.BASE.*
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
The four implementation-control macros (DEBUG_PRINT,
USE_GRADIENT_RECURSIVE_GAUSSIAN, USE_ADVANCED_PROJECTION,
USE_LINESEARCH_PROJECTION) were defined unconditionally at the top of
itkCuberilleImageToMeshFilter.h. Any translation unit including the
public header silently inherited DEBUG_PRINT=0, masking a same-named
macro any consumer might want to test with #ifdef.

Move the four #define lines to itkCuberilleImageToMeshFilter.hxx so
they remain visible to the template implementation that depends on
them via #if, but are no longer emitted into consumer translation
units that only include the .h.

Greptile P1 on PR InsightSoftwareConsortium#6205.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
Both operators applied >= / <= on m_Y in their first branch, which
short-circuited to true/false whenever the Y values were equal —
before the X tiebreaker could be evaluated. E.g. {Y=1,X=0} >=
{Y=1,X=5} resolved (1 >= 1) == true and returned true even though
the lhs is strictly less than the rhs.

Replace the first-branch comparison with strict > / < so equal-Y
cases fall through to the X tiebreaker, mirroring the existing
operator> and operator< definitions.

std::map calls only operator<, so existing callers do not observe
the bug, but the broken operators are class-level API and would
silently misbehave under direct use.

Greptile P1 on PR InsightSoftwareConsortium#6205.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
Convert the 20 Cuberille tests registered via plain add_test() to
itk_add_test() so they pick up the standard ITK CTest labels and
properties (matching the three already-itk_add_test'd entries below
them).  Tests with bare add_test are invisible to dashboard-specific
test selectors that filter on those labels.

GTest conversion of these tests is a separate follow-up.

Greptile P2 on PR InsightSoftwareConsortium#6205.
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
…est-cuberille

ENH: Ingest ITKCuberille into Modules/Filtering
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request May 12, 2026
Fixes the four classes of ghostflow errors that surfaced on the first
Cuberille v4 ingest (PR InsightSoftwareConsortium#6205) — the only allowable remaining error
should be the unavoidable upstream root commit.

  - Strip *.orig / *.rej / *.BACKUP.* / *.LOCAL.* / *.REMOTE.* /
    *.BASE.* merge-conflict-artifact files from history (catches
    leftover-conflict-marker errors like the .h.orig on Cuberille).
  - sanitize-history.py: cap every commit subject at 78 chars (ghostflow
    / kw-commit-msg rule); excess words move into the body.
  - sanitize-history.py: insert a blank line between subject and body
    when the original commit message lacked one (two-line subject error).
  - sanitize-history.py: clear the executable bit (mode 100755 -> 100644)
    on text-file extensions so root-commit "executable permissions but
    file does not look executable" stops firing.
@hjmjohnson
hjmjohnson deleted the ingest-cuberille branch August 22, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module area:Python wrapping Python bindings for a class area:Remotes Issues affecting the Remote module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants