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

Getting different results after upgrading from v0.0.10 to v0.0.11 #173

Closed danielesteban closed 1 year ago

danielesteban commented 1 year ago

I having some different results with the same operations after upgrading to v0.0.11 this morning.

I tried to reduce it into the smallest example to reproduce what I mean. Here you can test by swapping the three-bvh-csg imports at the top: https://glitch.com/edit/#!/flawless-adventurous-server?path=script.js

Here's with v0.0.10: image

Here's with v0.0.11: image

gkjohnson commented 1 year ago

Hello! Thanks for he report - I'm unable to open the glitch for some reason (it just says error starting the error). Can you use something like jsfiddle, instead?

Also if you can help find which commit caused the error that would help narrow down how to fix the issue.

danielesteban commented 1 year ago

Sure. There you go: https://jsfiddle.net/zr3jq5fL

I dunno an straightforward way to know which commit did it. Is there a list of builds by commit somewhere online i can swap to test them? Or is it the only choice to download the repo, go commit by commit building and testing it out?

gkjohnson commented 1 year ago

Thanks!

Or is it the only choice to download the repo, go commit by commit building and testing it out?

Yes right now you'd have the pull the repo and do something like a git bisect from v0.0.10 to v0.0.11. If not I'll be able to get to it - but I appreciate the help!

danielesteban commented 1 year ago

Aight.. I think I tracked it down.

With the previous https://github.com/gkjohnson/three-bvh-csg/commit/f50feeacfab86bfc9f138c6d48c9c7e59e577b64: image

But from this one onwards: https://github.com/gkjohnson/three-bvh-csg/commit/5009a216b9e1c8b2bc4268e84e17ae66cc82982c image

danielesteban commented 1 year ago

I uploaded the repo I used to test this to: https://github.com/danielesteban/csgtest

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

Nice. That was quick! Just to confirm that this one did the trick: https://github.com/gkjohnson/three-bvh-csg/commit/44025fe99ca703a12f84a08d268da3d9a2320510 image Should I close the issue or do you want me to wait until it hits an npm release?

gkjohnson commented 1 year ago

Thanks! Got it fixed in #176 and just published a new version.

gkjohnson commented 1 year ago

Sorry missed your last message -

Just to confirm that this one did the trick: 44025fe99ca703a12f84a08d268da3d9a2320510

Yeah this is the commit. It was using the same ray direction for every raycast to test containment previously which could cause problems the triangle was coplanar.

danielesteban commented 1 year ago

No worries.

I'm sorry but I found another bug/issue.. not sure if related but it happens rigth after: https://github.com/gkjohnson/three-bvh-csg/commit/5009a216b9e1c8b2bc4268e84e17ae66cc82982c

I just added a cone at the bottom of the base. Here's with v0.0.12: image

Here's with https://github.com/gkjohnson/three-bvh-csg/commit/f50feeacfab86bfc9f138c6d48c9c7e59e577b64: image

I updated the example repo at: https://github.com/danielesteban/csgtest And the jsfiddle: https://jsfiddle.net/9kLhuqn1/3/

gkjohnson commented 1 year ago

Thanks - can you make another issue? I'll have to take a look later.