fix the seg fault with the tracking lost. #84
Conversation
| @@ -37,9 +37,11 @@ void ITMColorTracker::TrackCamera(ITMTrackingState *trackingState, const ITMView | |||
| minimizeLM(*this, currentPara); | |||
| } | |||
|
|
|||
| const auto paraM = currentPara.GetM(); | |||
There was a problem hiding this comment.
Please can you replace auto with the actual type? We don't want to force an unnecessary C++11 dependency.
There was a problem hiding this comment.
I can do it. But according to the newest cmakefile (cmake/Flags.cmake), C++11 is already enabled.
There was a problem hiding this comment.
It's only enabled by default on Linux - we build with it there, but it's not a compulsory dependency. The change proposed will break the build on other platforms.
| @@ -128,7 +129,7 @@ void ITMSceneReconstructionEngine_CPU<TVoxel, ITMVoxelBlockHash>::AllocateSceneF | |||
| if (resetVisibleList) renderState_vh->noVisibleEntries = 0; | |||
|
|
|||
| M_d = trackingState->pose_d->GetM(); M_d.inv(invM_d); | |||
|
|
|||
There was a problem hiding this comment.
Please can you get rid of the whitespace changes?
| @@ -157,16 +163,16 @@ namespace ORUtils { | |||
| _CPU_AND_GPU_CODE_ inline Matrix4 &operator -= (const Matrix4 &mat) { for (int i = 0; i < 16; ++i) this->m[i] -= mat.m[i]; return *this; } | |||
|
|
|||
| _CPU_AND_GPU_CODE_ inline friend bool operator == (const Matrix4 &lhs, const Matrix4 &rhs) { | |||
| bool r = lhs[0] == rhs[0]; | |||
| bool r = lhs.m[0] == rhs.m[0]; | |||
There was a problem hiding this comment.
Were these changes motivated by your compiler having trouble with anonymous structs?
There was a problem hiding this comment.
This is not about anonymous struct. The Matrix class simply doesn't provide a subscript operator. I think this part of codes were not tested before.
| @@ -58,6 +58,7 @@ void ITMSceneReconstructionEngine_CPU<TVoxel, ITMVoxelBlockHash>::IntegrateIntoS | |||
| ITMRenderState_VH *renderState_vh = (ITMRenderState_VH*)renderState; | |||
|
|
|||
| M_d = trackingState->pose_d->GetM(); | |||
| if (HasNaN(M_d)) return ; // TODO: means the pose is invalid, need to fix that pose | |||
There was a problem hiding this comment.
Where was the segmentation fault occurring out of interest? Is it reproducible? This change here is only in the CPU version of the reconstruction engine - is there a corresponding one needed in the CUDA version?
There was a problem hiding this comment.
You can simply set one of M_d's element to be std::nan to see if it will happen on your machine. It's certainly reproducible on my machine. I can not test a fix on GPU now.
InfiniTAM/ORUtils/Matrix.h
Outdated
| @@ -49,6 +49,12 @@ namespace ORUtils { | |||
| T m[s*s]; | |||
| }; | |||
|
|
|||
| template<class T> | |||
There was a problem hiding this comment.
Please can you fix the indentation of this?
|
@sgolodetz I fixed the indentions. It was caused by mixing space and tabs. The codebase is using TAB instead of spaces. Also I remove auto. For the modification in GPU side, please refer the CPU implementation. |
Fixed the segmentation fault issue when the tracking is lost (got a invalid pose).
Fixed the operators (==, !=) codes for Matrix.
Added a HasNaN() function for matrix types.