chandlerprall / ThreeCSG

CSG plugin for Three.js
MIT License
453 stars 152 forks source link

Feature/iterative #59

Open avalero opened 5 years ago

avalero commented 5 years ago

The addTriangles recursive function gives a heap size exceeded exceptionwhen computing BSP over meshes with many triangles (max recursivility exceeded).

I have transformed the BSP Tree creation to an iterative function addTrianglesIterative(), so that it does not give that problem.

avalero commented 5 years ago

Same problem with the creation of the BSP occurrs when serializing BSP to buffer. It was a recursive function. Now serialization and deserialization of arraybuffer is iterative (non recursive).

Tests passed.

avalero commented 5 years ago

@chandlerprall after extensive benchmarking iterative algorithm is slowler than recursive. Recursive version (current version) reaches maximum recursivity (both in Chrome and Firefox) as soon as meshes have many triangles.

I have not managed to get the same speed with v1 that I get with master branch version (with iterative algrorithm, choosing triangles[0] as divider and lowing EPSILON to 1e-5)

The WASM version needs to be iterative (for same reason) and again, computation time is greater than master branch algorithm (although faster than js v1)

These are my conclusions. We (bitbloq) will stick to master branch algorithm by now, and keep on working on v1 around november (we have a planned release on october).

chandlerprall commented 5 years ago

Thanks for the feedback, and the PRs! It's really strange that the iterative approach is slower, I'll try to find some time to dig into that.

I believe main difference now between master and v1 is that master tracks polygons while v1 sticks to triangles, which would cause additional computations. My decision to change that aspect may need to be rolled back, if it is the cause of degraded performance.

avalero commented 5 years ago

Hi, I have been testing the iterative methods, and in fact they are not slowler. I got slowler results because of a change I did in my code to adapt to iterative (sorry for the confussion). So it would be safe to accept PR.

In any case, as you said, maybe it's worth going back to triangles, as v1 keeps being slowler than master.

Also, in case you want to stick with recursion, this reading might be interesting (I have not tested it myself) https://medium.com/openmindonline/js-monday-06-adopting-memory-safe-recursion-d26dcee409c9

johncalvinyoung commented 5 years ago

@avalero you working on a WASM port? Is that direct crosscompilation from Typescript, or in C/CPP or Rust or something?

avalero commented 5 years ago

@johncalvinyoung is C++,

It's far from being production ready, but I got it to make some basic CSG operations. Then I got problems with stack (at the end you have a recursive js function). I should port it to iterative and check performance.

My problem was that the cost (in time) of serializing and deserializing the geometries from js -> wasm and from wasm -> js . At then end, the time spent de/serlializing geometries dit not allow to get an overall better computation time (which was my main goal).

You can check the code here https://github.com/avalero/wasmCSG

johncalvinyoung commented 5 years ago

@avalero that's really cool. I took a quick look, and got it to build locally. Been working on another port of CSG stuff to WASM (albeit an existing CPP port of CSG.js) and didn't get correct results, sadly. So I'll take a look at yours.

avalero commented 5 years ago

Well, it would be just great if can make it work. And I am sure @chandlerprall could also help, at the end, it's his algorithm. You can find two branches: develop is based on this master repo. feature/v1 is based on this v1 repo branch.