Skip to content

[UUM-133527] - Improve floating point precision when Probuilderizing small objects#658

Open
thomas-tu wants to merge 1 commit intomasterfrom
bugfix/small-scale-precision
Open

[UUM-133527] - Improve floating point precision when Probuilderizing small objects#658
thomas-tu wants to merge 1 commit intomasterfrom
bugfix/small-scale-precision

Conversation

@thomas-tu
Copy link
Collaborator

DO NOT FORGET TO INCLUDE A CHANGELOG ENTRY

Purpose of this PR

[Brief summary of the changes in this PR.]

Links

Jira: https://jira.unity3d.com/browse/UUM-133527

Comments to Reviewers

[List known issues, planned work, provide any extra context for your code.]

…o closed vertices were getting removed because of lack of precision
Copy link

@u-pr u-pr bot left a comment

Choose a reason for hiding this comment

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

May require changes
The review identified a critical issue regarding the increase of FltCompareResolution. This change is likely to cause integer overflows and floating-point stability issues.

🤖 Helpful? 👍/👎

static class VectorHash
{
public const float FltCompareResolution = 1000f;
public const float FltCompareResolution = 1000000f;
Copy link

Choose a reason for hiding this comment

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

high
Have you considered the impact of this new resolution on float precision limits and potential integer overflows?

Increasing FltCompareResolution to 1000000f introduces two major issues:

  1. Integer Overflow: Conversions in VectorHash and related structs (like IntVec3) multiply vertex coordinates by this resolution and convert them using System.Convert.ToInt32(). With a multiplier of 1,000,000, any coordinate with a magnitude greater than ~2147.48 will exceed int.MaxValue, triggering a runtime OverflowException and crashing the mesh generation process.
  2. Hash Stability: The purpose of this resolution is to mask floating-point imprecision so that spatially identical vertices hash to the same value. A 32-bit float only provides ~7 significant decimal digits. A scale of 1,000,000 demands exactness up to $10^{-6}$. For any coordinate $\ge 10$, this exceeds standard float precision limits. Minor math inaccuracies will cause virtually identical vertices to resolve to different integers, which will break vertex welding, duplicate detection, and spatial sorting.

I highly recommend reverting this to 1000f (which provides a reliable $0.001$ precision tolerance and safely supports coordinates up to ~2,000,000) to maintain tool stability.

🤖 Helpful? 👍/👎

@codecov-github-com
Copy link

codecov-github-com bot commented Mar 6, 2026

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff           @@
##           master     #658   +/-   ##
=======================================
  Coverage   36.05%   36.06%           
=======================================
  Files         277      277           
  Lines       34909    34910    +1     
=======================================
+ Hits        12588    12589    +1     
  Misses      22321    22321           
Flag Coverage Δ
probuilder_MacOS_6000.0 34.58% <ø> (ø)
probuilder_MacOS_6000.3 34.58% <ø> (ø)
probuilder_MacOS_6000.4 34.57% <ø> (ø)
probuilder_MacOS_6000.5 34.58% <ø> (+<0.01%) ⬆️
probuilder_MacOS_6000.6 34.57% <ø> (?)

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

Files with missing lines Coverage Δ
Runtime/Core/VectorHash.cs 100.00% <ø> (ø)
🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant