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.

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

GLTF export: Remove snapping and fix validation #89352

Merged
merged 1 commit into from
Mar 10, 2024

Conversation

lyuma
Copy link
Contributor

@lyuma lyuma commented Mar 10, 2024

Fixes almost all validation warnings and errors on gtlf exports.

  • Round min/max correctly in accessors
  • Include correct target in vertex and indices bufferViews
  • Avoid use of Math::snapped
  • Normalize vertex weights.

I do still occasionally encounter a vertex which appears to round differently in min/max calculation and I don't know why:

ACCESSOR_MIN_MISMATCH	Declared minimum value for this component (-8.310235699582336e-8) does not match actual minimum (-8.310236410125071e-8).	/accessors/29/min/2
ACCESSOR_ELEMENT_OUT_OF_MIN_BOUND	Accessor contains 6 element(s) less than declared minimum value -8.310235699582336e-8.	/accessors/29/min/2

It is off by roughly 1.5e-14 which is really small but my guess is it's somehow the adjacent float value from a e-8

@lyuma lyuma requested a review from a team as a code owner March 10, 2024 11:56
Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

Needs to pass cicd but looks good. We discussed the problem on Discord.

Round min/max correctly in accessors
Include correct target in vertex and indices bufferViews
Avoid use of Math::snapped
Normalize vertex weights.
@AThousandShips AThousandShips added this to the 4.3 milestone Mar 10, 2024
@akien-mga akien-mga changed the title gltf export: Remove snapping and fix validation GLTF export: Remove snapping and fix validation Mar 10, 2024
@akien-mga akien-mga merged commit af527e5 into godotengine:master Mar 10, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

4 participants