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

Clean convex hull decomposition code #50404

Merged

Conversation

pouleyKetchoupp
Copy link
Contributor

Following-up from #50262 (comment) (CC @lawnjelly)

Remove unnecessary conversion between triangle data and vertex data whenever possible.

After testing performance, it doesn't seem to make much difference because the time spent in preparing and finalizing data is negligible compared to the process in VHACD, but the code generally seems cleaner this way.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall having lines of code deleted while doing the same thing is good.

May want someone else to review the details, but it looks good.

The code code never handled quads previously. Trying to think of why it was more complicated.

@lawnjelly
Copy link
Member

In general it looks great. The only suggestion I would make is, that now we are returning the raw vertices, (which in most cases is what we want), but for debugging sometimes you will want to draw these as triangles, and we have lost the connectivity information.

So I would include an optional parameter to return a triangle index list for each hull. Or we could simply return a Geometry::MeshData for each hull, with the vertices filled out, and optionally the faces (whatever is available from VHACD function).

@pouleyKetchoupp pouleyKetchoupp force-pushed the clean-convex-hull-decomposition branch from c18d892 to e2dc89d Compare July 13, 2021 18:14
@pouleyKetchoupp
Copy link
Contributor Author

@lawnjelly I've added an extra parameter to optionally get face information in an efficient way. It's not linked to any script API, but least it's ready in case someone wants to use it for debug purpose.

@reduz
Copy link
Member

reduz commented Aug 30, 2021

conflicts must be fixed

Remove unnecessary conversion between triangle data and vertex data
whenever possible.
@pouleyKetchoupp pouleyKetchoupp merged commit f9c6dc1 into godotengine:master Sep 15, 2021
@pouleyKetchoupp pouleyKetchoupp deleted the clean-convex-hull-decomposition branch September 15, 2021 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants