Skip to content

rgbd: restore fetching colors for pointcloud in coloredkinfu - #4184

Open
PeterBowman wants to merge 1 commit into
opencv:5.xfrom
PeterBowman:colored-kinfu
Open

rgbd: restore fetching colors for pointcloud in coloredkinfu#4184
PeterBowman wants to merge 1 commit into
opencv:5.xfrom
PeterBowman:colored-kinfu

Conversation

@PeterBowman

Copy link
Copy Markdown

Cherry-picked from 04cafeb (authored by @paroj). As pointed out by @alalek in #3121 (comment), this 4.x patch never reached 5.x because of merge conflicts.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaces a released public pure-virtual method rather than extending it, which breaks source compatibility for external ColoredKinFu implementations. OpenCV 5.0.0 and this PR's exact base expose getCloud(OutputArray, OutputArray) const. A minimal derived class that implements that method with override compiles against both headers; against exact head 6154ffca6036bdf16b5fd06b5049ea3c50fbe507 it fails because the only virtual now has three parameters. The default argument preserves ordinary two-argument calls, but it cannot preserve an override's signature.

Please keep the released two-output virtual intact and add the color-producing API additively (an overload or a separately named method), then add a compile-time compatibility probe for the existing derived-class signature. A controlled old-header/new-library vtable probe also shows the practical mismatch: the exact-base implementation ignores the unused fourth argument, while this head treats it as the new colors output—filling a valid sentinel with 4,389 entries and segfaulting when that stale argument is invalid.

The intended feature itself works in an exact-head build. An external synthetic-plane oracle returns 4,389 aligned points, normals, and CV_32FC4 colors; the legacy two-argument call still returns the same point/normal counts. The complete enabled RGB-D test binary passes 7/7 tests, git diff --check is clean, and the patch merges cleanly with current 5.x. GitHub exposes no checks for this head.

Reviewed with OpenAI Codex assistance; I independently built the exact revisions and verified the public API, source-compatibility, mixed-vtable, runtime color-output, and test results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants