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.

Sign up for GitHub

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

Jump to bottom

METIS-based meshlet generation #16947

Merged
merged 18 commits into from
Jan 5, 2025
Merged

METIS-based meshlet generation #16947

merged 18 commits into from
Jan 5, 2025

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Dec 23, 2024

Objective

Improve DAG building for virtual geometry

Solution

  • Use METIS to group triangles into meshlets which lets us minimize locked vertices which improves simplification, instead of using meshopt which prioritizes culling efficiency. Also some other minor tweaks.
  • Currently most meshlets have 126 triangles, and not 128. Fixing this might involve calling METIS recursively ourselves to manually bisect the graph, not sure. Not going to attempt to fix this in this PR.

Testing

  • Did you test these changes? If so, how?
    • Tested on bunny.glb and cliff.glb
  • Are there any parts that need more testing?
    • No
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    • Download the new bunny asset, run the meshlet example.

Showcase

New
image

Old
image

@JMS55 JMS55 added the A-Rendering Drawing game state to the screen label Dec 23, 2024
@JMS55 JMS55 requested a review from atlv24 December 23, 2024 23:17
@BenjaminBrienen BenjaminBrienen added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Dec 24, 2024
@alice-i-cecile alice-i-cecile added the C-Feature A new feature, making something new possible label Dec 24, 2024
@BenjaminBrienen BenjaminBrienen added the D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes label Dec 24, 2024
@JMS55 @IceSentry
Fix typo
eb67de7 
Co-authored-by: IceSentry <[email protected]>
atlv24
atlv24 approved these changes Jan 4, 2025
View reviewed changes
crates/bevy_pbr/src/meshlet/from_mesh.rs
options[metis::option::UFactor::INDEX] = 1; // Important that there's very little imbalance between partitions

let mut meshlet_per_triangle = vec![0; triangle_count];
let partition_count = triangle_count.div_ceil(126); // Need to undershoot to prevent METIS from going over 128 triangles per meshlet
Copy link
Contributor

Choose a reason for hiding this comment

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

by how much does it overshoot the partitions it makes if you set this to 128?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1-2 triangles, which then leads to tiny meshlets with 1 triangle. Need to undershoot unfortunately :(

@JMS55 JMS55 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 4, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 5, 2025
Merged via the queue into bevyengine:main with commit fe58993 Jan 5, 2025
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atlv24 atlv24 atlv24 approved these changes

@IceSentry IceSentry IceSentry approved these changes

@alice-i-cecile alice-i-cecile Awaiting requested review from alice-i-cecile

@Jondolf Jondolf Awaiting requested review from Jondolf

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Rendering
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JMS55 @IceSentry @atlv24 @alice-i-cecile @BenjaminBrienen