Skip to content

Fix NullReferenceException for morph target accessors without BufferView#900

Draft
SmittyWerbenJJ wants to merge 2 commits intoKhronosGroup:mainfrom
SmittyWerbenJJ:NullException-Fix-2022.3
Draft

Fix NullReferenceException for morph target accessors without BufferView#900
SmittyWerbenJJ wants to merge 2 commits intoKhronosGroup:mainfrom
SmittyWerbenJJ:NullException-Fix-2022.3

Conversation

@SmittyWerbenJJ
Copy link

Importing a glTF file with morph targets exported from Blender (confirmed also
reproducible with files exported from Godot) throws a NullReferenceException:

Asset import failed, "Assets/.../Daisy_Import_File.glb" > NullReferenceException: 
Object reference not set to an instance of an object
UnityGLTF.GLTFSceneImporter.ConstructMeshTargetsPrepareBuffers (...)
  at ImporterMeshes.cs:786

The crash is on this line in ConstructMeshTargetsPrepareBuffers:

bufferIdPair = targetAttribute.Value.Value.BufferView.Value.Buffer;

Root Cause

The glTF 2.0 spec explicitly defines accessor.bufferView as optional.

Section 5.1.1 of the glTF 2.0 spec (accessor.bufferView):

"When undefined, the accessor MUST be initialized with zeros; sparse property or extensions MAY override zeros with actual values."

Section 3.6.2.3 (Sparse Accessors) reinforces this:

"When accessor.bufferView is undefined, the sparse accessor is initialized as an array of zeros of size (size of the accessor element) * (accessor.count) bytes."

This means bufferView AND sparse can both legally be null simultaneously. It simply means the accessor is all zeros.

For morph targets specifically, Section 3.7.2.2 explicitly calls this out as an intended authoring pattern (emphasis mine):

"Attributes present in the base mesh primitive but not included in a given morph target MUST retain their original values for the morph target."

Implementation Note: This allows skipping zero-filled accessors and implies that different morph targets may contain different sets of attributes.

The existing code already handles the null bufferView case in the sparse != null
branch (added for Draco support), but the sparse == null branch was missing the
same guard and assumed bufferView would always be present.

Fix

Added a null-check to the non-sparse branch, mirroring the existing Draco guard

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

I forgot to upload the right version ... Actually checking for null BufferView
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.

2 participants