GLTF: Use const Vector internally instead of TypedArray copies#113172
Merged
akien-mga merged 1 commit intogodotengine:masterfrom Dec 1, 2025
Merged
GLTF: Use const Vector internally instead of TypedArray copies#113172akien-mga merged 1 commit intogodotengine:masterfrom
const Vector internally instead of TypedArray copies#113172akien-mga merged 1 commit intogodotengine:masterfrom
Conversation
aaronfranke
force-pushed
the
gltf-state-vector
branch
from
411a66b to
d2aa36e
Compare
aaronfranke
force-pushed
the
gltf-state-vector
branch
from
d2aa36e to
3c9bbfd
Compare
fire
approved these changes
Nov 26, 2025
aaronfranke
force-pushed
the
gltf-state-vector
branch
from
3c9bbfd to
342088c
Compare
Mickeon
reviewed
Nov 28, 2025
Member
Mickeon
left a comment
Mickeon
left a comment
There was a problem hiding this comment.
Just putting my two cents here since the docs have been marked as changed.
... still fine. I do like the parameters getting renamed.
akien-mga
approved these changes
Dec 1, 2025
github-project-automation
bot
moved this from Ready for review
to Done
in Asset Pipeline Issue Triage
Member
|
Thanks! |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 PR fixes a performance regression introduced in PR #109103. As part of moving the accessor code from GLTFDocument to GLTFAccessor, I replaced the internal
Vectoruses with calls to the exposedTypedArrayversions. This seemed harmless, but in fact, an astonishing 98% of CPU time in the array decode functions is spent converting betweenVectorandTypedArray. It seems that it is absolutely performance critical for us to useVectorinternally.The fix in this PR is simple: Add new functions that let us use
Vectorinternally. We still need to keep theTypedArrayversions around, but discourage their use, therefore I have their internal names suffixed with_bind. At the same time, since this PR changes this code, I figured I may as well make all the gettersconst, which also requires binding compatibility functions. As such, this PR does involve a lot of boilerplate changes, but there isn't much functionally different in here.I have tested this on the TPS demo, which is where @akien-mga noticed the performance regression. I can reproduce the atrocious import times in master after PR #109103, nearly 10 minutes for me to import the TPS demo from scratch, and this PR restores the speed found in 4.5, a bit over 2 minutes of total time, with most of the time no longer spent importing the glTF files.