Skip to content

fix issue where probuilder with quad topology instead of triangle would trigger out of bounds exception when being exported #619

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

Merged
merged 4 commits into from
Aug 18, 2025

Conversation

varinotmUnity
Copy link
Contributor

Purpose of this PR

The degenerate triangle removal code introduced in v6.0.6 assumed all submeshes used triangular topology,
but ProBuilder meshes can contain mixed topologies (triangles, quads, etc.). The code incremented through indices
by 3 and accessed indexes[tri + 2], causing IndexOutOfRangeException when processing non-triangular submeshes
like quads with indices arrays not divisible by 3. This specifically affected shapes like Door that use quad topology.

Fix: Added a topology check if (submeshes[i].m_Topology == MeshTopology.Triangles) before applying the
degenerate triangle removal logic. This ensures the triangle-specific degenerate detection only runs on actual
triangular submeshes, preventing the index bounds violation on quad/other topologies.

Preserves Original Fix: The degenerate triangle removal functionality remains completely intact for triangular
submeshes - it still detects and removes zero-area triangles using cross-product magnitude checks. Non-triangular
submeshes bypass this processing entirely, which is appropriate since the concept of "degenerate triangles" doesn't
apply to quads or other topologies. The fix maintains backward compatibility while eliminating the crash.

Result: Door shapes and other mixed-topology meshes can now be exported without IndexOutOfRangeException,
while triangular meshes continue to benefit from degenerate triangle cleanup and improved rendering quality.

Links

https://jira.unity3d.com/browse/PBLD-251

Comments to Reviewers

I did not check for degenerate quads in this PR, because I could not repro the issue the user was having with degenerate triangles (making the bloom flash weirdly)

…ld trigger out of bounds exception when being exported
@TimAidleyAtUnity
Copy link
Collaborator

Thank you for fixing the bug I introduced!

@codecov-github-com
Copy link

codecov-github-com bot commented Aug 13, 2025

Codecov Report

Attention: Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
Runtime/Core/ProBuilderMeshFunction.cs 94.11% 1 Missing ⚠️
@@           Coverage Diff           @@
##           master     #619   +/-   ##
=======================================
  Coverage   33.86%   33.86%           
=======================================
  Files         277      277           
  Lines       34797    34802    +5     
=======================================
+ Hits        11783    11787    +4     
- Misses      23014    23015    +1     
Flag Coverage Δ
mac_trunk 33.65% <94.11%> (+<0.01%) ⬆️
win_trunk 33.54% <94.11%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
Runtime/Core/ProBuilderMeshFunction.cs 71.29% <94.11%> (+0.08%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

numBadIndices += 3;
}
else
if (indexes.Length % 3 == 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: couldn't you merge both of these checks in to one?

if (submeshes[i].m_Topology == MeshTopology.Triangles && indexes.Length % 3 == 0)

@varinotmUnity varinotmUnity self-assigned this Aug 14, 2025
@varinotmUnity varinotmUnity added the bug Something isn't working label Aug 14, 2025
Copy link
Collaborator

@zatzuri-unity zatzuri-unity left a comment

Choose a reason for hiding this comment

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

Tested the repro steps for this issue as well as the original repro steps from the degenerate triangles issues. I've also tested exporting different meshes

@varinotmUnity varinotmUnity merged commit 9bc1a05 into master Aug 18, 2025
11 checks passed
@varinotmUnity varinotmUnity deleted the bugfix/pbld-251-export-mesh-error-fix branch August 18, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants