ENH: Ingest ITKFPFH into Modules/Registration#6360
Merged
hjmjohnson merged 51 commits intoJun 1, 2026
Conversation
… for optimization
ENH: Removing distances till docker images are not updated
Remove log messages
ENH: Change radius to double type
WIP: Removing not required dependencies
…acro Added two new macro's, intended to replace the old 'itkTypeMacro' and 'itkTypeMacroNoParent'. The main aim is to be clearer about what those macro's do: add a virtual 'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro', 'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it was not used anyway. Note that originally 'itkTypeMacro' did not use its 'superclass' parameter either, looking at commit 699b66c, Will Schroeder, June 27, 2001: https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
The test previously only printed results. Verify the feature container is non-null, holds 33 bins per point, and contains only finite, non-negative histogram weights so regressions are actually caught.
Replace the raw WSB_subsampled.vtk point cloud committed in-tree with a .cid content link (uploaded to ITKTestingData); test data is fetched via ExternalData instead of shipping a binary in the source tree.
Remove Modules/Remote/FPFH.remote.cmake now that the module lives at Modules/Registration/FPFH, and enable Module_Fpfh in the pixi CI configure task so the in-tree module is built and tested.
github-actions
Bot
added
type:Infrastructure
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}
ComputeFPFHFeature stored squared neighbor distances as float while ComputeSPFHFeature used double; the narrowed float was then the divisor in the weighted SPFH accumulation, perturbing the normalized histogram weights. Use double in both for a consistent descriptor.
Replace C-style casts with static_cast and remove itk:: prefixes on names already in scope within namespace itk (CrossProduct, Math::pi, SizeValueType, MultiThreaderBase, Vector, PointsLocator, VectorContainer), per ITK code-review conventions.
FindClosestNPoints already caps results at the requested count, so the std::min clamp was a no-op and the neighbors were never sorted; correct the misleading comment and use the filtered count directly.
Member
Author
|
ghostflow findings (all in pre-ITK upstream commits)
These cannot be resolved without either squashing (which drops the upstream merge topology and |
In-tree ITK_SOURCE_DIR is always defined; only the itk_module_impl() branch is reachable.
dzenanz
approved these changes
May 29, 2026
Member
dzenanz
left a comment
dzenanz
left a comment
There was a problem hiding this comment.
LGTM, but it would be good if Pranjal reviewed too.
Member
|
Maybe edit |
Member
|
(spotted via ghostflow warnings) |
hjmjohnson
merged commit 06172c1
into
InsightSoftwareConsortium:main
19 of 20 checks passed
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
3 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
ITKFPFHremote module (Fast Point Feature Histograms) into the ITK source tree atModules/Registration/FPFH/, preserving upstreamgit blameand merge topology. Part of the remote-module consolidation tracked in #6160.This module computes FPFH point-cloud descriptors and pairs with the recently ingested RANSAC module (#6275) for point-set registration.
Built and tested locally (macOS, Release): all FPFH tests pass —
FpfhKWStyleTest,FpfhInDoxygenGroup, the newPointFeature.BasicObjectMethodsGoogleTest, and the data-drivenitkPointFeatureTest.Ingest details
https://github.com/InsightSoftwareConsortium/ITKFPFH.gitingest-module-v4.sh): whitelistfilter-repo+ per-commit clang-format/black + commit-prefix sanitization. Upstream merge topology preserved (6 merges).ITKCommon,ITKRegistrationCommon,ITKTestKernel,ITKMetaIO,ITKIOMeshBase. Module isEXCLUDE_FROM_DEFAULTand enabled in the pixi CI configure task.WSB_subsampled.vtkwas uploaded to ITKTestingData (Add FPFH WSB_subsampled.vtk test input data ITKTestingData#62) and is referenced via a.cidExternalData content link rather than shipped as a raw binary.Fixes applied on top of the faithful import (one topic commit each)
<cmath>+std::floor(was Cmath.h+ unqualifiedfloor);static_castfor C-style casts; dropped redundantitk::qualifiers insidenamespace itk; removed a redundant neighbor-count clamp and fixed a misleading comment.size_tand the parallel-array index toitk::SizeValueTypeto avoid overflow on very large point sets; useddouble(notfloat) for the FPFH neighbor-distance divisor to match the SPFH path.