Hello,

This thread relates to the MeshDecimator bug noted in Will Smith's post here and my post here.

After some debugging, I have found the issue isn't actually with canCollapseRegionEdge() etc. The problem occurs (at least in my code) because SurfaceMesh::extractSubset() is missing this line:



As a result the extracted subset is missing region coordinates, causing MeshDecimator::fillInitialVertexMetadata() to incorrectly flag all vertices as touching region edges which they actually aren't.

Actually with a bit more testing, although this fixed my decimation problems, it hasn't fixed the one pointed out by Will. I will try to look into that as well.

It seems as though the collapse issue pointed out by Will can be fixed by changing the minimum dot product threshold in MeshDecimator<PositionMaterial>::canCollapseNormalEdge

All I did was change

to

Awesome thanks for the help , I'll try these changes soon.

Great, thanks for looking into these. I'll get your changes applied to the repository as soon as I can.

@Will - it would be good to know if they do fix your problem.

Isn't that just equal to 1 because of float numbers only having 7-8 digits precision?
if so, perhaps it might bring problems in some other areas.

I actually originally tested it by just setting it to 1.0f which worked. 0.9 recurring shouldn't get rounded up, but rather just stay as 0.99999998f I think... ?

collapseChangesFaceNormals function actually has the opposite effect to what I would expect. I expected a lower min dot product value to be safer, but all that did was make even more edges be incorrectly merged.

I think the function collapseChangesFaceNormals needs to be looked at in more detail. According to the comments it only needs to check for flipped normal, so the threshold value could probably be taken out altogether.

I haven't really had time to look at it though. Maybe another day.

We commited the change to extractSubset(), to make sure the region is copied across. Didn't commit the 0.999... change yet, I'd like to get back on my other PC to look at that.



Ah, so you would expect the provided value to be a maximum error, such that a value of 1.0 would allow more collapses than a value of 0.0f. This does make some sense, but actually the value is treated as a threshold such that if two surfaces can collapse then they must have similar normals and hence a high dot product.



It's also used for decimation of the smooth marching cubes surfaces, and so the threshold is used in this case. But yes, you could have a separate version for the cubic case.

Sorry, what I mean is... since a normal triangle and a collapsed triangle on a cubic mesh should have the same normal, I would expect a lower threshold value to allow for more floating point error as the angle between the vectors increases.

I am guessing it was floating point error allowing the corner to incorrectly collapse.... so I was confused as to why increasing the value prevented the collapse.

But now that I think about it, it was the width of the error margin that allowed the invalid collapse in the first place, right? Since the dot product resulting from the normal comparison in Will's example case would be quite close to 1.0.... so that is why it only occurs on an straight voxel edge length of 28 or more, and raising the threshold value appears to fix it.....

I am not really sure what the best way would be to account for floating point error while simultaneously preventing invalid collapses when the normals are similar such as in Will's case. Probably as you said, a separate function for cubic meshes.

Sorry, I'm not really having much time to look at this, and to be honest I'm a little hazy as to how it worked

If it seems that changing the threshold to 1.0 (or 0.999999...) then I'm happy to make this change as this hardcoded value is only used when decimating cubic surfaces. I don't have any test cases, but I'm also not using this feature so if it helps you and Will then that's good enough for me.

Overall though I'm intending to remove this decimation code and replace it with a version which does the decimation as the mesh is extracted. Maybe the smooth version remains, and if it does then it can at least be cleaned up with the cubic stuff out of the way.