ENH: Ingest ITKCuberille into Modules/Filtering#6205
ENH: Ingest ITKCuberille into Modules/Filtering#6205hjmjohnson merged 115 commits intoInsightSoftwareConsortium:mainfrom
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
|
| Filename | Overview |
|---|---|
| Modules/Filtering/Cuberille/include/itkCuberilleImageToMeshFilter.h | Public header with two issues: four global #define macros that pollute the preprocessor namespace, and logically incorrect >= / <= operators in the private VertexLookupNode class. |
| Modules/Filtering/Cuberille/include/itkCuberilleImageToMeshFilter.hxx | 1052-line header-only implementation; core algorithm looks correct. The four #define guards are consumed here but originate in the .h. |
| Modules/Filtering/Cuberille/test/CMakeLists.txt | 25 CuberilleTest01 variants and CuberilleIssue66 use bare add_test rather than ITK's itk_add_test; the remaining 3 tests use itk_add_test correctly. |
| Modules/Filtering/Cuberille/itk-module.cmake | Module registration looks correct; DEPENDS and TEST_DEPENDS are appropriately specified; EXCLUDE_FROM_DEFAULT and ENABLE_SHARED match other in-tree modules. |
| Modules/Remote/Cuberille.remote.cmake | Correctly deleted as part of ingestion; module is now in-tree. |
| Utilities/Maintenance/RemoteModuleIngest/sanitize-history.py | New ingestion tooling; content-sniffing, formatter wrappers, and commit-callback logic look well-structured and fail-soft. No security concerns. |
| Utilities/Maintenance/RemoteModuleIngest/ingest-module-v4.sh | Phase A driver script; set -euo pipefail throughout, proper preflight checks, clean argument parsing — no obvious issues. |
| pyproject.toml | Adds -DModule_Cuberille:BOOL=ON to the CI configure line, enabling the new in-tree module for Python wheel builds. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[upstream ITKCuberille repo] -->|Phase A: ingest-module-v4.sh| B[git-filter-repo: path whitelist + deny-glob]
B --> C[sanitize-history.py: clang-format / black / gersemi / prefix]
C --> D[Mode-A merge into ITK main]
D --> E[Modules/Filtering/Cuberille/]
D --> F[Remove Modules/Remote/Cuberille.remote.cmake]
D --> G[pyproject.toml: enable Module_Cuberille]
E --> H[itk-module.cmake: DEPENDS / TEST_DEPENDS]
E --> I[include/itkCuberilleImageToMeshFilter.h+.hxx]
E --> J[test/ + wrapping/]
D -->|Phase B: deferred post-merge| K[archive-remote-module.sh]
K --> L[upstream: delete files + flip archived=true]
Reviews (1): Last reviewed commit: "ENH: Enable Module_Cuberille in configur..." | Re-trigger Greptile
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.
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.
9658520
into
InsightSoftwareConsortium:main
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.
…est-cuberille ENH: Ingest ITKCuberille into Modules/Filtering
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.
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.