ENH: Ingest ITKCuberille into Modules/Filtering#6205
Merged
hjmjohnson merged 115 commits intoMay 5, 2026
Conversation
QuadricDecimationQuadEdgeMeshFilter.
Refactor as an ITKv4 module.
Add Docker test configuration.
ENH: Add wrapping.
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.
BUG: Address KWStyle tests.
The defined directory is not correct when built as a Remote module.
BUG: Fix test temporary output dir.
COMP: Wsign-compare warning
…ITKNULLPTR STYLE: Null to ITK_NULLPTR
dzenanz
reviewed
May 4, 2026
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>
c04bb52 to
f6a46d2
Compare
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.
dzenanz
approved these changes
May 4, 2026
9658520
into
InsightSoftwareConsortium:main
18 of 19 checks passed
63 tasks
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.
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
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.
Ingests
Cuberillefrom the standalone remote module InsightSoftwareConsortium/ITKCuberille into ITK atModules/Filtering/Cuberille/using the v4 ingestion pipeline (PR #6204). Stacked on #6204 — that PR must merge first.Local validation:
pre-commit run --all-filesclean;CuberilleTestDriverbuilds; 25/25 Cuberille tests pass locally in 5.3s.Pipeline metrics
feedback_ingest_merge_topology.md)git log -- Modules/Filtering/Cuberillev4 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-globdeny-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)
Every commit on the rewritten history starts with a valid ITK prefix (
BUG:/COMP:/DOC:/ENH:/PERF:/STYLE:/WIP:). TheUtilities/Maintenance/RemoteModuleIngest/sanitize-history.pylog 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-mergepre-commit run --all-filesgate caught one CMake file that 0.24.0 considered clean but 0.19.3 wanted re-formatted. Working as designed.DOC:AddModules/Filtering/Cuberille/README.mdpointing at the archived upstream.COMP:RemoveModules/Remote/Cuberille.remote.cmake(now in-tree).ENH:EnableModule_Cuberille:BOOL=ONinpyproject.tomlconfigure-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:That step pushes the deletion +
MIGRATION_README.md → README.mdpromotion to upstream and flipsarchived=true. This PR makes no upstream changes; the upstreamITKCuberillerepository remains live until the operator manually runs Phase B.