Fix meshgl merge functions returning input pointer on no-op (#1632)
When Merge() returns false (mesh already manifold), both manifold_meshgl_merge and manifold_meshgl64_merge were destructing the valid copy and returning the input pointer, violating the C API's ownership convention. Always return the copy instead, since Merge() leaves the mesh valid regardless of its return value. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
d93943fc7e
commit
d88e295085
@@ -548,11 +548,8 @@ ManifoldMeshGL* manifold_meshgl_copy(void* mem, ManifoldMeshGL* m) {
|
||||
|
||||
ManifoldMeshGL* manifold_meshgl_merge(void* mem, ManifoldMeshGL* m) {
|
||||
auto duplicate = new (mem) MeshGL(*from_c(m));
|
||||
if (duplicate->Merge()) {
|
||||
return to_c(duplicate);
|
||||
}
|
||||
duplicate->~MeshGL();
|
||||
return m;
|
||||
duplicate->Merge();
|
||||
return to_c(duplicate);
|
||||
}
|
||||
|
||||
ManifoldMeshGL64* manifold_get_meshgl64(void* mem, ManifoldManifold* m) {
|
||||
@@ -573,11 +570,8 @@ ManifoldMeshGL64* manifold_meshgl64_copy(void* mem, ManifoldMeshGL64* m) {
|
||||
|
||||
ManifoldMeshGL64* manifold_meshgl64_merge(void* mem, ManifoldMeshGL64* m) {
|
||||
auto duplicate = new (mem) MeshGL64(*from_c(m));
|
||||
if (duplicate->Merge()) {
|
||||
return to_c(duplicate);
|
||||
}
|
||||
duplicate->~MeshGL64();
|
||||
return m;
|
||||
duplicate->Merge();
|
||||
return to_c(duplicate);
|
||||
}
|
||||
|
||||
size_t manifold_meshgl_num_prop(ManifoldMeshGL* m) {
|
||||
|
||||
@@ -466,3 +466,61 @@ TEST(CBIND, triangulation) {
|
||||
manifold_delete_triangulation(triangulation);
|
||||
free(tri_verts);
|
||||
}
|
||||
|
||||
TEST(CBIND, meshgl_merge_returns_mem) {
|
||||
// Create a cube and get its meshgl (which already has merge vectors).
|
||||
ManifoldManifold* cube =
|
||||
manifold_cube(alloc_manifold_buffer(), 1.0, 1.0, 1.0, 0);
|
||||
ManifoldMeshGL* original = manifold_get_meshgl(alloc_meshgl_buffer(), cube);
|
||||
|
||||
// This mesh is already manifold, so Merge() internally returns false.
|
||||
// The bug: old code returned the input pointer instead of the output buffer.
|
||||
void* mem = alloc_meshgl_buffer();
|
||||
ManifoldMeshGL* merged = manifold_meshgl_merge(mem, original);
|
||||
|
||||
// The returned pointer must always come from mem, never from original.
|
||||
EXPECT_EQ(reinterpret_cast<void*>(merged), mem);
|
||||
EXPECT_NE(merged, original);
|
||||
|
||||
// The merged mesh should still be valid — construct a Manifold from it.
|
||||
ManifoldManifold* result =
|
||||
manifold_of_meshgl(alloc_manifold_buffer(), merged);
|
||||
EXPECT_EQ(manifold_status(result), MANIFOLD_NO_ERROR);
|
||||
EXPECT_NEAR(manifold_volume(result), 1.0, 0.0001);
|
||||
|
||||
manifold_destruct_manifold(result);
|
||||
manifold_destruct_meshgl(merged);
|
||||
manifold_destruct_meshgl(original);
|
||||
manifold_destruct_manifold(cube);
|
||||
free(result);
|
||||
free(mem);
|
||||
free(original);
|
||||
free(cube);
|
||||
}
|
||||
|
||||
TEST(CBIND, meshgl64_merge_returns_mem) {
|
||||
ManifoldManifold* cube =
|
||||
manifold_cube(alloc_manifold_buffer(), 1.0, 1.0, 1.0, 0);
|
||||
ManifoldMeshGL64* original =
|
||||
manifold_get_meshgl64(alloc_meshgl64_buffer(), cube);
|
||||
|
||||
void* mem = alloc_meshgl64_buffer();
|
||||
ManifoldMeshGL64* merged = manifold_meshgl64_merge(mem, original);
|
||||
|
||||
EXPECT_EQ(reinterpret_cast<void*>(merged), mem);
|
||||
EXPECT_NE(merged, original);
|
||||
|
||||
ManifoldManifold* result =
|
||||
manifold_of_meshgl64(alloc_manifold_buffer(), merged);
|
||||
EXPECT_EQ(manifold_status(result), MANIFOLD_NO_ERROR);
|
||||
EXPECT_NEAR(manifold_volume(result), 1.0, 0.0001);
|
||||
|
||||
manifold_destruct_manifold(result);
|
||||
manifold_destruct_meshgl64(merged);
|
||||
manifold_destruct_meshgl64(original);
|
||||
manifold_destruct_manifold(cube);
|
||||
free(result);
|
||||
free(mem);
|
||||
free(original);
|
||||
free(cube);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user