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

Mesh::deduplicate_vertices #16016

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Mesh::deduplicate_vertices #16016

wants to merge 1 commit into from
+405 −1

Conversation

stepancheg
Copy link
Contributor

Objective

Current Mesh is not easy to create programmatically: a developer has either:

  • generate duplicate vertices
  • or do non-trivial logic of dealing with indices

I raised this issue in #10928, which was rejected. This is an alternative approach.

Solution

Currently there's Mesh::duplicate_vertices operation which flattens vertices by removing indices.

This PR proposes inverse operation: Mesh::deduplicate_vertices: if index is not set, deduplicate vertex data and create an index.

So for quick experiments, when startup time may be not as important as amount of data, mesh can be generated without dealing with indices, and then Mesh::deduplicate_vertices will generate an index.

Testing

Unit test added.

@stepancheg stepancheg force-pushed the deduplicate branch 3 times, most recently from 0aa1a90 to d631732 Compare October 19, 2024 23:14
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen X-Uncontroversial This work is generally agreed upon S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Math Fundamental domain-agnostic mathematical operations labels Oct 20, 2024
@stepancheg stepancheg changed the title Mesh::deduplicate Mesh::deduplicate_vertices Oct 20, 2024
@525c1e21-bd67-4735-ac99-b4b0e5262290
Copy link
Contributor

LGTM on mac

@BenjaminBrienen BenjaminBrienen added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 31, 2024
@stepancheg
Copy link
Contributor Author

Rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
A-Math Fundamental domain-agnostic mathematical operations 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-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Uncontroversial This work is generally agreed upon
Projects
Rendering
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stepancheg @525c1e21-bd67-4735-ac99-b4b0e5262290 @alice-i-cecile @BenjaminBrienen