stla / cgalMeshes

R6 based utilities for 3D meshes.
15 stars 2 forks source link

Memory corruption in union() #9

Closed dwoll closed 1 year ago

dwoll commented 1 year ago

Hi, using the union() / intersection() methods, I think I've encountered a memory corruption issue.

library(cgalMeshes)
mesh1 <- cgalMesh$new("1_HERZ_ES.ply")
mesh2 <- cgalMesh$new("1_HERZ_HRO.ply")
# meshes are closed, volume ok
mesh1$volume()
# [1] 737570.2
mesh2$volume()
# [1] 829835.3

Calling union() however changes both meshes - they are not closed anymore, and the intersection is empty.

mu <- mesh1$union(mesh2)
mesh1$volume()
# Error: The mesh is not closed.
mesh2$volume()
# Error: The mesh is not closed.
mi <- mesh1$intersection(mesh2)
# mi is empty mesh

Either related to the memory issue, or a separate issue: calling orientToBoundVolume() on the empty intersection mesh causes R to crash:

mi$orientToBoundVolume()
# crash

Many thanks for your advice!

1_HERZ_ES.txt 1_HERZ_HRO.txt

stla commented 1 year ago

Hello,

Thanks. There's no corruption :-) This is normal: when you compute the union, the two meshes are modified, the new meshes are disjoint and they form the union. See this example:

library(cgalMeshes)
mesh1 <- cgalMesh$new(sphereMesh())
mesh2 <- cgalMesh$new(sphereMesh(x = 1.5))
umesh <- mesh1$union(mesh2)
rmesh1 <- mesh1$getMesh()
rmesh2 <- mesh2$getMesh()
rgl::shade3d(rmesh1, color = "red", alpha = 0.2)
rgl::shade3d(rmesh2, color = "blue", alpha = 0.2)

For the crash, I will add a check of emptiness. Thanks!

dwoll commented 1 year ago

Ah, thanks! I totally misunderstood the documentation. I thought union() and intersection() did not modify the original mesh, but just returned a new mesh. Is there an efficient way to copy the original mesh to avoid modification? Many thanks in advance.

stla commented 1 year ago

Yes, mesh$copy().

dwoll commented 1 year ago

Indeed, sorry for not looking that up. I confirm that using copies solves the issue. While I admit that I was a bit dense here, it might be worth putting extra warnings in the documentation about the fact that these (and other) functions are mutating state. R is predominantly pass-by-value using functions without side effects. Working with pass-by-reference and side effects takes some real getting used to for people who are not used to C/C++ etc. Thank you very much!

stla commented 1 year ago

Good point. Thanks for the advice. But the documentation of a R6 class does not allow to include extra sections, I don't see how I could emphasize the warnings, except by adding WARNING in the description.