ENH: Ingest ITKMeshToPolyData into Modules/Filtering#6335
Open
hjmjohnson wants to merge 92 commits into
Open
Conversation
ITKModuleTemplate otuput
To polydata
To polydata
ENH: Add CellData to PolyData
ENH: Add PolyData cell members
ENH: Add MeshToPolyDataFilter::GetOutput
Mesh to polydata
Todo: Add these to the ITKBridgeNumPy module.
Py vectorcontainer buffer
Vectorcontainer point numpy
BUG: Retain Points shape when converting to NumPy array
…esh-cells BUG: Only process mesh cells when present
github-actions
Bot
added
type:Infrastructure
Member
Author
|
@greptile review |
This comment was marked as resolved.
This comment was marked as resolved.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Brings MeshToPolyData from a configure-time remote fetch into the ITK source tree at Modules/Filtering/MeshToPolyData/ using the v3 whitelist filter-repo pipeline. Upstream repo: https://github.com/InsightSoftwareConsortium/ITKMeshToPolyData.git Upstream tip: 000ca8267ae4adcef07fdd308ef69de6e6100548 Ingest date: 2026-05-26 Whitelist passes (git filter-repo): - --path include --path src --path test --path wrapping - --path CMakeLists.txt --path itk-module.cmake - --to-subdirectory-filter Modules/Filtering/MeshToPolyData - --prune-empty always - (if present) second pass: invert CTestConfig.cmake Outcome: 204 upstream commits -> 73 surviving; 9 distinct authors preserved; git blame walks across the merge boundary to original authors. Content-link inventory: .md5=0 .shaNNN=0 .cid=2 Primary author: Matt McCormick <matt.mccormick@kitware.com> Co-authored-by: Dženan Zukić <dzenan.zukic@kitware.com> Co-authored-by: Hans J. Johnson <hans-johnson@uiowa.edu> Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu> Co-authored-by: Jon Haitz Legarreta Gorroño <jon.haitz.legarreta@gmail.com> Co-authored-by: Matt McCormick <matt@mmmccormick.com> Co-authored-by: Matthew McCormick <matt@mmmccormick.com> Co-authored-by: Pranjal Sahu <pranjalsahu5@gmail.com> Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com>
Inline the module DESCRIPTION rather than file(READ README.rst); the README.rst remains in the archived upstream repo and is not ingested.
MeshToPolyData now lives at Modules/Filtering/MeshToPolyData; drop the configure-time itk_fetch_module declaration.
Add -DModule_MeshToPolyData:BOOL=ON so the now in-tree module is built and tested by the CI configure step (EXCLUDE_FROM_DEFAULT module).
Run gersemi, clang-format, black and end-of-file-fixer over the ingested CMakeLists and Python test files to satisfy the in-tree pre-commit gate.
The output filename used argv[1] (the input mesh) instead of argv[2], so the round-trip writer overwrote the ExternalData input fixture. Fails on read-only data stores and pollutes the cache.
The strip loop computed the unsigned expression numPoints - 2 with the range check only in a debug assert. A strip header of 0 or 1 underflowed to ~4e9 in release builds, iterating far past the cell container. Skip strips with fewer than three points instead.
The vertex, line and polygon parsers trusted the per-cell point count and advanced the iterator without checking inputCellEnd, reading past the container when a cell header claimed more points than remained. Bound each point-copy loop by the container end.
Initialize() cleared only the points, point-data and cell-data containers, leaving the vertices, lines, polygons and triangle-strip containers populated. Re-executing a pipeline into a reused PolyData then mixed new points with stale cell connectivity.
The visitor's eight raw cell-container pointers had no initializers and no constructor; the triangle-strip pointers are never assigned, leaving indeterminate values. Add in-class nullptr initializers.
The cell-data copy indexed inputCellData via ElementAt(cellId) without checking existence; itk::Mesh permits cell data set for only some cells, so a sparse container produced an out-of-bounds access. Use GetElementIfIndexExists and copy only present entries.
GetNumberOfPixels() returns a 64-bit SizeValueType; storing it in an unsigned long truncates on Windows LLP64 for images larger than 2^32 pixels, under-reserving the point containers.
GetNumberOfPoints() is unsigned and the value is stored in a uint32_t container; the PolyLine overload used a signed int, unlike its sibling Visit overloads. Use const unsigned int to avoid the sign conversion.
Replace the empty-bodied constructor/destructor with = default and mark the destructor override; drop the empty private section.
itkPolyLineCellTest.py imports numpy but was registered unconditionally, unlike itkPyVectorContainerTest. A wrapped build without numpy reported a hard failure instead of skipping. Move both numpy tests under the probe.
The wasm/ subdirectory was not ingested, so the WASI/Emscripten add_subdirectory(wasm) referenced a nonexistent path and broke external WebAssembly builds. Remove the block.
Replace the pre-C++11 char-array sizeof SFINAE idiom with a std::void_t detection trait deriving from std::true_type/std::false_type.
hjmjohnson
force-pushed
the
ingest-MeshToPolyData
branch
from
f6c0ba7 to
035bb59
Compare
VectorContainer::Reserve calls CreateIndex and grows the container with zero-filled entries, whereas the visitors append real cell ids via push_back. The leading zeros inflated Size() and made the cell-data copy loop read cell 0 repeatedly, corrupting per-cell attributes for any input mesh carrying cell data. Use the STL capacity-hint reserve, matching the sibling geometry containers.
Covers the MeshToPolyDataFilter cell-data path (the fixture mesh cow.vtk carries no cell data). An all-triangle mesh with per-cell values must produce a PolyData whose cell data matches 1:1 in cell-id order; this fails if the cell-id containers are mis-sized.
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
6 participants
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 the
MeshToPolyDataremote module into the ITK source tree atModules/Filtering/MeshToPolyData/, preserving upstream authorship and merge history. Part of the remote-module consolidation effort (#6060).After the move, this PR also fixes 12 latent defects found by review in the ingested code (memory-safety on malformed input, a test that overwrote its own fixture, portability, and lint-gate items).
What the ingest did
git filter-repopipeline (include/,src/,test/,wrapping/,CMakeLists.txt,itk-module.cmake); 204 upstream commits → 73 surviving, 26 files.--no-ff --allow-unrelated-historiesmerge:git blamewalks across the boundary to the original 8 upstream authors; 24 upstream internal merges preserved..cid(cow.vtk, foot.mha); both verified resolvable via the IPFS gateway.itk-module.cmakeadapted for in-tree build: inlineDESCRIPTION, andITKMeshmoved fromCOMPILE_DEPENDStoDEPENDS(public headers exposeitk::Meshtypes; required for correct wrapping propagation, matching Cuberille/MeshNoise).Modules/Remote/MeshToPolyData.remote.cmakeremoved; module enabled inconfigure-ci.Post-ingest fixes (14 commits, from review + Greptile)
itkMeshToPolyDataFilterTestoutput toargv[2](was overwriting the input fixture)numPoints - 2, release OOB)PolyData::InitializeVisitCellsClasspointer membersSizeValueTypefor the pixel count (Win64 LLP64 truncation)Visit= defaultspecial members +overrideinImageToPointSetFilteritkPolyLineCellTeston numpywasm/add_subdirectoryHasCellTraitswithstd::void_treservenotReserve(Greptile P1: zero-fill corrupted cell-data copy)cow.vtkdoesn't)Local validation
pre-commit run --all-files: pass.Module_MeshToPolyData=ON): clean.ctest -R 'PolyData|ImageToPointSet|MeshToPolyData': 39/39 pass (incl. KWStyle + Doxygen-group checks).Follow-up (separate, after merge): archive the upstream
InsightSoftwareConsortium/ITKMeshToPolyDatarepo with a migration notice (per the ingestion workflow).