Variable corruption crash with ICP

[perrykipkerrie]

Hi,

When upgrading from 2.14 to 2.15 we got a crash when using the ICP method. I've build the libs with use of VS 2017.

The crash is located on line 374 of lineqn.h with the exception: Run-Time Check Failure #2 - Stack around the variable 'e' was corrupted.

We are using the default, unmodified version of the Trimesh2 from this Github.

Callstack:

[Forceflow]

What is the exact call you're using to invoke the ICP method?

[perrykipkerrie]

trimesh::ICP(targetMesh, movingMesh, xf1, xf2, kdTarget, kdMoving, std::vector<float>(), std::vector<float>(), 0.0f, 1, do_scale, do_affine);

With the following parameters: ` do_scale = false do_affine = false trimesh::KDtree kdMoving = new trimesh::KDtree(movingMesh->vertices); trimesh::KDtree kdTarget = new trimesh::KDtree(targetMesh->vertices); trimesh::xform xf1; trimesh::xform xf2; trimesh::TriMesh movingMesh = new trimesh::TriMesh(); trimesh::TriMesh targetMesh = new trimesh::TriMesh();

for (int i = 0; i < this->movingVertices->getNum(); i++) { movingMesh->vertices.push_back(trimesh::point((this->movingVertices)[i][0], (this- movingVertices [i][1], (this->movingVertices)[i][2])); } for (int i = 0; i < this->targetVertices->getNum(); i++) { targetMesh->vertices.push_back(trimesh::point((this->targetVertices)[i][0], (this->targetVertices)[i] 1], (this->targetVertices)[i][2])); } `

[Forceflow]

I've pulled in some ICP-related fixes from upstream: https://github.com/Forceflow/trimesh2/pull/14 Long shot, but maybe fixed?

[perrykipkerrie]

Hmm, I will try it :)

[perrykipkerrie]

So far I can see i do not get any error messages yet. Thank you for your time.

[Forceflow]

So far I can see i do not get any error messages yet. Thank you for your time.

All credit goes to Szymon upstream, I just pulled in the changes.

[perrykipkerrie]

Hi Forceflow, i found a reproducer.

If you use the Mesh_Align.exe with the two times the same mesh it will cause a variable corruption.

What won't work:

• Aligning the same objects
• Aligning the same object but on different locations
• Aligning the same objects where some parts are deleted on one of the objects

What does work:

• Aligning the same objects where a bit of noise on the vertices was added on one objects.

What is remarkable is that the A matrix on rule 344 is filled with -nan(ind) values.

[perrykipkerrie]

I've found the problem. As the model is exact the same, the median_dist in rule 205 (ICP.cc) is 0, causing a devision by zero a few rules later.

[Forceflow]

Okay - that's a good repro.

I'd report this upstream, but thinking about this more has got me wondering: Why are you using ICP on two times the exact same mesh? Doesn't that defeat the point of using ICP?

Make me understand what the use case for this is, and I'll happily fix locally or upstream it to Szymon.

[perrykipkerrie]

Thank you for your quick reply!, I've send a mail to trimesh2 today aswell, as i remembered you saying you pull their changes.

I use the same mesh often for testing / debugging purposes, furthermore I use it to get the transformation between two meshes. This bug also arises when one of the 'same' models is cut and vertices are deleted.

[Forceflow]

How would you propose fixing this? Adding a tiny perturbation to the result?

[perrykipkerrie]

Hmmm I see the original author tries to overcome this within the code with a check (!if(median_dist)) but i do not think it triggers it. From which git do you pull? maybe i can create a pull request?

[Forceflow]

Hmmm I see the original author tries to overcome this within the code with a check (!if(median_dist)) but i do not think it triggers it. From which git do you pull? maybe i can create a pull request?

That's part of the reason this fork exists: AFAIK, Szymon only publishes by releasing a ZIP file every now and then. He's a busy man :)

You can make a PR against this repo if you want, but maybe it would be smart to keep a little "patches" folder that contains the diffs of what we did to original Trimesh2 code. So when I update to whenever 2.17 releases and this bug isn't fixed upstream, I can just patch here.

So

• ICP.cc with your fix
• new top-level folder called patches containing a .patch file of what you did

It's a bit cumbersome, but it keeps it easy for me. I'll document whatever I do upstream to get it working here too. It's not much lately, some freeglut fixes.

If you're not comfortable doing that, I'd be happy to do it for you as well 👍