Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colored Kinect Fusion #2878

Merged
merged 57 commits into from
Apr 7, 2021
Merged

Colored Kinect Fusion #2878

merged 57 commits into from
Apr 7, 2021

Conversation

DumDereDum
Copy link
Member

@DumDereDum DumDereDum commented Feb 24, 2021

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 other license that is incompatible with OpenCV
  • The PR is proposed to proper branch
  • There is reference to 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

/** @brief camera intrinsics */
CV_PROP_RW Matx33f intr;
CV_PROP_RW Matx33f rgb_intr;

Copy link
Member Author

Choose a reason for hiding this comment

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

add info about values

@@ -81,7 +86,7 @@ struct CV_EXPORTS_W Params

/** @brief camera intrinsics */
CV_PROP_RW Matx33f intr;

CV_PROP_RW Matx33f rgb_intr;
Copy link
Member Author

Choose a reason for hiding this comment

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

add info about value

@@ -40,7 +40,7 @@ struct CV_EXPORTS_W Params

/** @brief camera intrinsics */
CV_PROP_RW Matx33f intr;

CV_PROP_RW Matx33f rgb_intr;
Copy link
Member Author

Choose a reason for hiding this comment

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

add info about value

@@ -87,6 +88,15 @@ static const float cy = 204.f;
static const float k1 = 0.12f;
Copy link
Member Author

Choose a reason for hiding this comment

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

rename like depth_focal and etc

dir = fileList.substr(0, slashIdx);

if (!file.is_open())
throw std::runtime_error("Failed to write depth list");
Copy link
Member Author

Choose a reason for hiding this comment

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

rgb list, not depth list

std::string depthFname = cv::format("%04d.png", count);
std::string fullDepthFname = dir + '/' + depthFname;
if (!imwrite(fullDepthFname, depth))
throw std::runtime_error("Failed to write depth to file " + fullDepthFname);
Copy link
Member Author

Choose a reason for hiding this comment

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

the same

virtual void integrate(InputArray, float, const Matx44f&, const kinfu::Intr&, const int) override
{ CV_Error(Error::StsNotImplemented, "Not implemented"); };
virtual void integrate(InputArray _depth, InputArray _rgb, float depthFactor, const Matx44f& cameraPose,
const kinfu::Intr& intrinsics, const Intr& rgb_intrinsics, const int frameId = 0) override;
Copy link
Member Author

Choose a reason for hiding this comment

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

rename intrinsics as depth_intrinsics


// use depth instead of distance (optimization)
void ColoredTSDFVolumeCPU::integrate(InputArray _depth, InputArray _rgb, float depthFactor, const Matx44f& cameraPose,
const Intr& intrinsics, const Intr& rgb_intrinsics, const int frameId)
Copy link
Member Author

Choose a reason for hiding this comment

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

rename intrinsics as depth_intrinsics

{
v_float32x4 pv = (orig + dir * v_setall_f32(ts));
v_float32x4 nv = volume.getNormalVoxel(pv);
v_float32x4 cl = volume.getColorVoxel(pv);
Copy link
Member Author

Choose a reason for hiding this comment

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

rename cl as cv

int _u = (int)projected.get0();
int _v = (int)v_rotate_right<1>(projected).get0();
int rgb_u = (int)projected.get0();
int rgb_v = (int)v_rotate_right<1>(projected).get0();
Copy link
Contributor

Choose a reason for hiding this comment

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

Vectorized code diverges with scalar one in what coordinate to use for RGB

ColorType& b = voxel.b;

// update RGB
if (abs(((float)(r + g + b)) - (colorRGB[0] + colorRGB[1] + colorRGB[2])) < 1000 || weight < 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this extra comparison and why 1000?

Copy link
Member Author

Choose a reason for hiding this comment

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

it is the previous version of code
will be removed

@savuor savuor self-assigned this Apr 7, 2021
@savuor
Copy link
Contributor

savuor commented Apr 7, 2021

While there are things to do (speed up using OpenCL, rewrite raycasting to get better color filtering, fix demo to allow unregistered depth-rgb pairs) they have more sense to be implemented in upcoming PRs than in this one.
Here we have demo working and tests passing which is going to be enough for now.

@savuor
Copy link
Contributor

savuor commented Apr 7, 2021

👍

@alalek alalek merged commit da6a95e into opencv:master Apr 7, 2021
@alalek alalek mentioned this pull request Apr 9, 2021
@DumDereDum DumDereDum deleted the colored_kinfu branch April 19, 2021 07:22
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.

3 participants