Bug in preprocessing: holes appears after mesh simplification

[spaceZt3]

After mesh simplification, there is a probability that holes will appear. Try it a few more times, and each time the results are different: sometimes holes are produced, sometimes no holes are produced. I think it's a data race problem when parallel edge collapse.

[Yixin-Hu]

Hi,

The mesh simplification is a preprocessing on the input and it does not touch the non-manifold edges/vertices of the mesh so it should not create holes.

Could you share your input and command with me? Thanks.

[nmnobre]

After mesh simplification, there is a probability that holes will appear. Try it a few more times, and each time the results are different: sometimes holes are produced, sometimes no holes are produced. I think it's a data race problem when parallel edge collapse.

Hey @spaceZt3,

I actually found a race condition in the preprocessing edge collapse algorithm myself yesterday!
I put together a fix in #58. It's currently the last commit, 628ef41, although that might change if I rebase...
Since you seem to be encountering a similar issue, could you please check if that fix works for you too?

Cheers,
-Nuno

[spaceZt3]

After mesh simplification, there is a probability that holes will appear. Try it a few more times, and each time the results are different: sometimes holes are produced, sometimes no holes are produced. I think it's a data race problem when parallel edge collapse.

Hey @spaceZt3,

I actually found a race condition in the preprocessing edge collapse algorithm myself yesterday! I put together a fix in #58. It's currently the last commit, 628ef41, although that might change if I rebase... Since you seem to be encountering a similar issue, could you please check if that fix works for you too?

Cheers, -Nuno

Thanks. @nmnobre I saw your commit. But I wonder why it can fix the data race problem? It just simply change "std::vector" to "std::vector".

[nmnobre]

Thanks. @nmnobre I saw your commit. But I wonder why it can fix the data race problem? It just simply change "std::vector" to "std::vector".

std::vector<bool> to std::vector<char> to be more precise. :)
The difference is with the latter you are guaranteed each thread accesses entirely different bytes, guaranteeing thread safety purely via cache coherence. Let me know if the fix works in your case as well or if there's still something else we need to nail down.

[spaceZt3]

Thanks. @nmnobre I saw your commit. But I wonder why it can fix the data race problem? It just simply change "std::vector" to "std::vector".

std::vector<bool> to std::vector<char> to be more precise. :) The difference is with the latter you are guaranteed each thread accesses entirely different bytes, guaranteeing thread safety purely via cache coherence. Let me know if the fix works in your case as well or if there's still something else we need to nail down.

@nmnobre ，thanks for your explanation! I will try it later.