Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New ConvexGeometry based on QuickHull #10987

Merged
merged 23 commits into from
Mar 13, 2017
Merged

New ConvexGeometry based on QuickHull #10987

merged 23 commits into from
Mar 13, 2017

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Mar 11, 2017

This PR introduces a QuickHull implementation for generating convex hulls. Features:

  • Average-case time complexity of O(nlogn) instead of O(n^2)
  • More robust (solves ConvexHull example is broken #10624)
  • New ConvexGeometry and ConvexBufferGeometry
  • Documentation of all new entities

I've prepared two simple examples (with a 50k vertices model) so you can better compare the execution times (see console):

Old Implementation O(n^2)
New Implementation O(nlogn)

As you can see in the code the QuickHull implementation is not part of ConvexGeometry but a standalone entity. So you can do e.g. new THREE.QuickHull().setFromObject( mesh ) to create the bounding volume for a 3D object as with Box3. ConvexGeometry uses the result of THREE.QuickHull to compute the geometry data. But you can also use it for other purposes like collision detection.

The implementation is mainly based on https://github.com/maurizzzio/quickhull3d so credit goes to @Maurizzzio for his great work!

@mrdoob
Copy link
Owner

mrdoob commented Mar 11, 2017

Great stuff as always!

However, just like with DecalGeometry, I think it would be better to keep it in examples/js/geometries/.

@WestLangley
Copy link
Collaborator

I think this is really great! :)

The only thing that bothers me is the creation of the class you call Vertex. It is really an element of a linked list.

In three.js, the term 'vertex' has special meaning: as vertex of a triangle, or as a point on the surface of a mesh.

I would call it QPoint or something.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 11, 2017

@WestLangley Ok, i'll try to figure out a different name 👍 . QPoint sounds already good. Maybe i will also change VertexList to QPointList.

However, just like with DecalGeometry, I think it would be better to keep it in examples/js/geometries/.

@mrdoob The problem is that the implementation of QuickHull3 is more complex and the module system is a great help here. And unlike DecalGeometry, the code should work uniformly with all geometries you can create with the library. (I know we want to keep the core of the library as small as possible. Just wanted to highlight these facts. I accept of course both solutions.)

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 12, 2017

Okay, i moved to code to examples. I renamed Vertex to VertexNode because it's more consistent to the rest of the naming conventions in QuickHull. Hope, that's okay 👍

@mrdoob
Copy link
Owner

mrdoob commented Mar 13, 2017

Sweet!

@mrdoob mrdoob merged commit b42ce88 into mrdoob:dev Mar 13, 2017
@mrdoob
Copy link
Owner

mrdoob commented Mar 13, 2017

Thanks!

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Mar 13, 2017

Yay! 🎉

I think we can close now the following issues: #10624 and #8416

This was referenced Mar 13, 2017
@WestLangley
Copy link
Collaborator

Thanks @Mugen87 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants