From d459d1a5683d3b91f3f465fbd4c5eadf1e6b3fe8 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 2 Oct 2024 11:08:06 -0700 Subject: [PATCH 1/2] simplify: Rework vertex_lock implementation for better wedge behavior Previously, we would only read vertex_lock data for the primary vertex (the vertex with the smallest index among those with the same position). When using sparse mode, this would be the first vertex referenced by the index buffer due to how the sparse remap works. If the input locks were inconsistent between all copies, there could be a situation where a vertex with the wrong index was asked to be locked, and the vertex would still be not-locked and could move. When *not* using sparse mode, this was particularly problematic as the flags would be read from the vertex that might not be referenced by the index buffer at all. We now propagate the locked status into the primary vertex in a separate pass; this requires re-locking the wedges in a separate pass, but all of this is very quick especially when using sparse mode. --- src/simplifier.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/simplifier.cpp b/src/simplifier.cpp index 91be38b62..1fcf2f56b 100644 --- a/src/simplifier.cpp +++ b/src/simplifier.cpp @@ -369,12 +369,7 @@ static void classifyVertices(unsigned char* result, unsigned int* loop, unsigned { if (remap[i] == i) { - if (vertex_lock && vertex_lock[sparse_remap ? sparse_remap[i] : i]) - { - // vertex is explicitly locked - result[i] = Kind_Locked; - } - else if (wedge[i] == i) + if (wedge[i] == i) { // no attribute seam, need to check if it's manifold unsigned int openi = openinc[i], openo = openout[i]; @@ -438,6 +433,18 @@ static void classifyVertices(unsigned char* result, unsigned int* loop, unsigned } } + if (vertex_lock) + { + // vertex_lock may lock any wedge, not just the primary vertex, so we need to lock the primary vertex and relock any wedges + for (size_t i = 0; i < vertex_count; ++i) + if (vertex_lock[sparse_remap ? sparse_remap[i] : i]) + result[remap[i]] = Kind_Locked; + + for (size_t i = 0; i < vertex_count; ++i) + if (result[remap[i]] == Kind_Locked) + result[i] = Kind_Locked; + } + if (options & meshopt_SimplifyLockBorder) for (size_t i = 0; i < vertex_count; ++i) if (result[i] == Kind_Border) From 7e0fad10ed93098a47edf9b83b619c7829b01be3 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 2 Oct 2024 11:11:33 -0700 Subject: [PATCH 2/2] demo: Add a test for interaction between lock flags & seams This test would fail before, specifically lock2 would not lock the spine because it was using the side of the spine with larger vertex indices; after the previous fix lock2 and lock3 are equivalent. --- demo/tests.cpp | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/demo/tests.cpp b/demo/tests.cpp index ac4848489..e87a5a349 100644 --- a/demo/tests.cpp +++ b/demo/tests.cpp @@ -1221,6 +1221,78 @@ static void simplifyLockFlags() assert(memcmp(ib, expected, sizeof(expected)) == 0); } +static void simplifyLockFlagsSeam() +{ + float vb[] = { + 0, 0, 0, + 0, 1, 0, + 0, 1, 0, + 0, 2, 0, + 1, 0, 0, + 1, 1, 0, + 1, 1, 0, + 1, 2, 0, + 2, 0, 0, + 2, 1, 0, + 2, 1, 0, + 2, 2, 0, // clang-format :-/ + }; + + unsigned char lock0[12] = { + 1, 0, 0, 1, + 0, 0, 0, 0, + 1, 0, 0, 1, // clang-format :-/ + }; + + unsigned char lock1[12] = { + 1, 0, 0, 1, + 1, 0, 0, 1, + 1, 0, 0, 1, // clang-format :-/ + }; + + unsigned char lock2[12] = { + 1, 0, 1, 1, + 1, 0, 1, 1, + 1, 0, 1, 1, // clang-format :-/ + }; + + unsigned char lock3[12] = { + 1, 1, 0, 1, + 1, 1, 0, 1, + 1, 1, 0, 1, // clang-format :-/ + }; + + // 0 1-2 3 + // 4 5-6 7 + // 8 9-10 11 + + unsigned int ib[] = { + 0, 1, 4, + 4, 1, 5, + 4, 5, 8, + 8, 5, 9, + 2, 3, 6, + 6, 3, 7, + 6, 7, 10, + 10, 7, 11, // clang-format :-/ + }; + + unsigned int res[24]; + // with no locks, we should be able to collapse the entire mesh (vertices 1-2 and 9-10 are locked but others can move towards them) + assert(meshopt_simplifyWithAttributes(res, ib, 24, vb, 12, 12, NULL, 0, NULL, 0, NULL, 0, 1.f, 0) == 0); + + // with corners locked, we should get two quads + assert(meshopt_simplifyWithAttributes(res, ib, 24, vb, 12, 12, NULL, 0, NULL, 0, lock0, 0, 1.f, 0) == 12); + + // with both sides locked, we can only collapse the seam spine + assert(meshopt_simplifyWithAttributes(res, ib, 24, vb, 12, 12, NULL, 0, NULL, 0, lock1, 0, 1.f, 0) == 18); + + // with seam spine locked, we can collapse nothing; note that we intentionally test two different lock configurations + // they each lock only one side of the seam spine, which should be equivalent + assert(meshopt_simplifyWithAttributes(res, ib, 24, vb, 12, 12, NULL, 0, NULL, 0, lock2, 0, 1.f, 0) == 24); + assert(meshopt_simplifyWithAttributes(res, ib, 24, vb, 12, 12, NULL, 0, NULL, 0, lock3, 0, 1.f, 0) == 24); +} + static void simplifySparse() { float vb[] = { @@ -1774,6 +1846,7 @@ void runTests() simplifyAttr(/* skip_g= */ false); simplifyAttr(/* skip_g= */ true); simplifyLockFlags(); + simplifyLockFlagsSeam(); simplifySparse(); simplifyErrorAbsolute(); simplifySeam();