[3.x] Fix Mesh::get_face_count()
#88198
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AThousandShips
changed the title
Mesh::get_face_count()Mesh::get_face_count()
lawnjelly
force-pushed
the
fix_facecount_bug
branch
from
6ee44da
to
771a19e
Compare
This sounds familiar... yep - that was me! I've changed it to |
kleonc
reviewed
Feb 11, 2024
Ha that's super funny, I'd assumed the pointy finger of blame was Juan! 😆 Hopefully new name is a little more obvious, but it is slightly confusing code there. |
This fixes a minor bug whereby facecount was actually returning the facecount * 3. There were no major problems from this, but it did mean the optional threshold poly count used when merging was out by a factor of 3.
lawnjelly
force-pushed
the
fix_facecount_bug
branch
from
771a19e
to
a32a261
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a minor bug whereby facecount was actually returning the facecount * 3. There were no major problems from this, but it did mean the optional threshold poly count used when merging was out by a factor of 3.
I've also taken the opportunity to rename it to
triangle countrather than face, becausegenerate_triangle_mesh()currently relies on it being triangles.Notes
get_face_count()has only been present in 3.x for a few days since [3.x] Add MergeGroup node to simplify merging Meshes at runtime #61568 so there is little likelihood of any knockon problems fixing this.MergeGroupPR: it was due to some rather dodgy naming cough by the previous author 😁 (that I copy pasted code from) wherefaces_sizewas not the face count, but the number of vertices used by the faces.Mesh::generate_triangle_mesh()to prevent this happening again.Mesh::generate_triangle_mesh()(where it caused no problems due to the aforementioned naming), and optionally when merging if the user had set a threshold poly count for splitting (where the threshold would have been out by 3, which is no big problem).