gkjohnson / three-bvh-csg

A flexible, memory compact, fast and dynamic CSG implementation on top of three-mesh-bvh
MIT License
628 stars 49 forks source link

Unexpected behaviour when upgrading from v0.0.10 to v0.0.12 #177

Closed danielesteban closed 1 year ago

danielesteban commented 1 year ago

I beleive this was introduced also on: https://github.com/gkjohnson/three-bvh-csg/commit/5009a216b9e1c8b2bc4268e84e17ae66cc82982c

Here's with v0.0.12: 276931270-33aaab9e-16de-46f0-b9f3-46367bd5eb37

Here's with https://github.com/gkjohnson/three-bvh-csg/commit/f50feeacfab86bfc9f138c6d48c9c7e59e577b64: 276931470-8f39e09b-4ac5-435d-b03c-07a416228c61

jsfiddle: https://jsfiddle.net/9kLhuqn1/3/

test repo:

git clone --recursive https://github.com/danielesteban/csgtest.git
cd csgtest
pnpm install
pnpm start
# Open: http://localhost:8080/

# If you do this it works as intended
cd three-bvh-csg/
git checkout f50feeacfab86bfc9f138c6d48c9c7e59e577b64
danielesteban commented 1 year ago

Ah! And if you're curious this is what I changed from the #173 example that got fixed. https://github.com/danielesteban/csgtest/commit/36d597c08453bb48ad05c0f74f4bb7d5c7b59b15#diff-27653c212e1cfe533e4eb2f7d0d3f89604c9de48a09583b4cbbbcbd08a07da79 Just added a cone at the bottom of the base and moved the camera to better illustrate the issue.

danielesteban commented 1 year ago

Been doing some digging and made an even simpler example. This one is just an addition between a box and a cone: https://jsfiddle.net/z4cnpsra/38/

Here's with v0.0.10: image

Here's with v0.0.12: image

In the new version it creates sort of like an interior face when merging the box and the cone. I dunno if any of this is related. but seems it could be the root of the issue I posted on the previous example.

gkjohnson commented 1 year ago

Should be fixed now in #178. Can you give it a check before I merge and publish? A simple way is to just use the commit link index.js from that PR via githack to test the functionality:

https://raw.githack.com/gkjohnson/three-bvh-csg/d2f7d1221476e22b8769402b8d7342b593d0c72b/src/index.js
image

There was another longer standing bug that this exposed relating to coplanar faces, as well. Thanks again for submitting minimal repros and hunting down the commits!

danielesteban commented 1 year ago

Just tested all the minimal examples with the #178 patch and everything works as intended as far as I can tell.

Also tested the more complex ones that originated the issue and everything is also working as intended: screenshot-(33)

You're so welcome. I'm glad I was able to help. And thank you for this amazing library! I just couldn't have done all this geometry without it.

gkjohnson commented 1 year ago

Awesome! Just published a new version