New HashTSDF implementation by DumDereDum · Pull Request #2698 · opencv/opencv_contrib

DumDereDum · 2020-10-06T13:25:24Z

This PR changes

Fixes memory leak caused by multiple pixNorm for each volume unit
Uses common buffer for voxel data making data allocation faster
Common pieces of code between TSDF and HashTSDF moved to tsdf_functions.cpp/hpp
A lot of minor changes

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

modules/rgbd/src/hash_tsdf.cpp

savuor · 2020-10-08T15:15:05Z

modules/rgbd/src/hash_tsdf.cpp

+
+        vu.pose = subvolumePose;
+        volumes.push_back(Mat(1, volumeDims.x * volumeDims.y * volumeDims.z, rawType<TsdfVoxel>()));
+        vu.index = HashS; HashS++;


push_back works here but one of the purposes of this PR is to make a code that'd be easy to port to OpenCL, while OpenCL has no such push_back ability: its buffers have fixed size.
What should be done instead:

volumes should be preallocated to some size

when this buffer is exhausted, a reallocation should be done to a new buffer of size*sizeFactor where sizeFactor=2.0 for example

done, but not the same

modules/rgbd/src/hash_tsdf.hpp

modules/rgbd/src/hash_tsdf.cpp

savuor · 2020-10-08T23:28:34Z

modules/rgbd/src/hash_tsdf.hpp

+    cv::Vec3i coord;
+    volumeIndex index;
+    cv::Matx44f pose;
+    Vec4i volDims;


Since all volume units have the same size, we shouldn't keep volDims for each of them.
Another question is should we keep pose for each of them since it can be calculated from coord and volume pose - I think, we shouldn't.

volDims - done

modules/rgbd/src/hash_tsdf.cpp

modules/rgbd/src/hash_tsdf.hpp

savuor · 2020-10-09T10:10:12Z

modules/rgbd/src/hash_tsdf.hpp

    float volumeUnitSize;
    bool zFirstMemOrder;
+    Point3i volumeDims;
+    Vec4i volDims;


Please rename one of them (or both?) to be more self-descriptive
For example, volDims to volStrides

modules/rgbd/src/hash_tsdf.cpp

savuor · 2020-10-13T18:06:26Z

modules/rgbd/src/tsdf.cpp

-
 // use depth instead of distance (optimization)
 void TSDFVolumeCPU::integrate(InputArray _depth, float depthFactor, const cv::Matx44f& cameraPose,
                              const Intr& intrinsics)


Since integrateVolumeUnit is in tsdf_functions, the TSDF integration should be rewritten using it. SIMD version is to be taken too.
If this amount of work is too big for that PR, then it's better to postpone it till next PR.

savuor · 2020-10-13T18:08:10Z

modules/rgbd/src/tsdf_functions.cpp

+    cv::Matx44f _pose, Point3i volResolution, Vec4i volStrides,
+    InputArray _depth, float depthFactor, const cv::Matx44f& cameraPose,
+    const cv::kinfu::Intr& intrinsics, InputArray _pixNorms, InputArray _volume)
+{


Do not forget SIMD version of the code

savuor · 2020-10-16T13:30:22Z

modules/rgbd/src/hash_tsdf.cpp

+    //! Out of bounds
+    CV_DbgAssert((volumeIdx[0] >= volResolution.x || volumeIdx[0] < 0) ||
+                 (volumeIdx[1] >= volResolution.y || volumeIdx[1] < 0) ||
+                 (volumeIdx[2] >= volResolution.z || volumeIdx[2] < 0))


Here compilation fails: no such thing as volResolution
Also semicolon ; missed

savuor · 2020-10-16T13:32:06Z

modules/rgbd/src/hash_tsdf.cpp

+    cv::Vec3i volumeUnitIdx          = volumeToVolumeUnitIdx(point);
+    VolumeUnitIndexes::const_iterator it = volumeUnits.find(volumeUnitIdx);
+
+    CV_DbgAssert (it == volumeUnits.end())


savuor · 2020-10-16T13:32:26Z

modules/rgbd/src/hash_tsdf.cpp

-    Ptr<TSDFVolumeCPU> volumeUnit = std::dynamic_pointer_cast<TSDFVolumeCPU>(it->second.pVolume);
+    VolumeUnitIndexes::const_iterator it = volumeUnits.find(volumeUnitIdx);
+
+    CV_DbgAssert (it == volumeUnits.end())


savuor · 2020-10-16T20:31:34Z

👍

DumDereDum added 7 commits October 6, 2020 16:21

create new variables

111e2c8

rewrite reset()

46fe5a3

first valid version of new HasHTSDF

4128df1

some warning fixes

3bebec6

create lambda raycast

4b50771

reduce time raycast

5dcda08

minor fix

a9acd1b

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.cpp Outdated Show resolved Hide resolved

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.hpp Outdated Show resolved Hide resolved

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.cpp Outdated Show resolved Hide resolved

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.cpp Outdated Show resolved Hide resolved

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.cpp Outdated Show resolved Hide resolved

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.cpp Outdated Show resolved Hide resolved

savuor reviewed Oct 8, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.hpp Outdated Show resolved Hide resolved

DumDereDum added 11 commits October 9, 2020 08:57

minor fix volDims

c329cdf

changed _atVolumeUnit, reduce memory consumption

fcc2238

delete older inmplemetation of atVolumeUnit

fe33797

changes _at

82303c0

AAA, I want to cry!

31a7593

it works!

186e072

it works twice o_o

040c6a8

minor fix

4bbf818

new adding to volumes

f4b52cc

delete volDims at strust VolumeUnit

30a64a2

new names of vars

864d755

savuor reviewed Oct 9, 2020

View reviewed changes

modules/rgbd/src/hash_tsdf.cpp Show resolved Hide resolved

rename one var

eeb7e79

savuor reviewed Oct 13, 2020

View reviewed changes

DumDereDum and others added 17 commits October 14, 2020 09:44

remove extra args at atVolumeUnit signature

3d76d0a

change frame params checking

a9bbd58

move volStrides to CPU class

0415828

inline convertion functions in tsdf_functions

c5c2032

minor fix

5f03c14

add SIMD version of integrateVolumeUnit

1e001e7

Merge branch 'master' into new_HashTSDF_implementation

de0e50d

fix something :)

5f0118f

docs fix

77c4f6a

warning fix

07ee90e

add degub asserts

7a706c2

replace vars initialization with reset()

3d2c0e2

remove volDims var

ffeec52

new resize buffer

ca6d445

minor vars name fix

1be50f7

docs fix

8ab9316

warning fix

e754f64

savuor reviewed Oct 16, 2020

View reviewed changes

DumDereDum added 3 commits October 16, 2020 16:49

minor fix

bae7dbf

minor fix 1

ffb7177

remove dbg asserts

a82f5ec

savuor self-assigned this Oct 16, 2020

alalek merged commit 0b5ded4 into opencv:master Oct 19, 2020

alalek mentioned this pull request Nov 27, 2020

Merge 4.x #2768

Merged

DumDereDum deleted the new_HashTSDF_implementation branch April 19, 2021 07:25

Conversation

DumDereDum commented Oct 6, 2020 • edited by savuor Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This PR changes

Pull Request Readiness Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

savuor Oct 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

savuor commented Oct 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DumDereDum commented Oct 6, 2020 •

edited by savuor

Loading

savuor Oct 16, 2020 •

edited

Loading