CesiumGS / gltf-pipeline

Content pipeline tools for optimizing glTF assets. :globe_with_meridians:
Apache License 2.0
1.91k stars 251 forks source link

Delete uri property when parsing binary_glTF buffer (glTF 1.0) #578

Closed lilleyse closed 3 years ago

lilleyse commented 3 years ago

This PR deletes the uri property of the binary_glTF buffer, which is meant to be ignored because the binary chunk should be used instead. Without this fix the glTF 1.0 to 2.0 update step may produce an invalid buffer.

From the KHR_binary_glTF glTF 1.0 extension:

...a runtime can ignore the uri property since the buffer refers to the Binary glTF body section, not an external resource. When a runtime encounters this buffer, it should use the Binary glTF body as the buffer.

Here's what a glTF like this looks like. A fake uri is used because buffer.schema.json requires that uri exists. The uri would be deleted after this change.

  "buffers": {
    "binary_glTF": {
      "type": "arraybuffer",
      "byteLength": 13790,
      "uri": "data:,"
    }
  },
ptrgags commented 3 years ago

@lilleyse what would be the best way to test this? and is there a reason why there aren't any unit tests in this PR?

lilleyse commented 3 years ago

@ptrgags thanks for the reminder, I meant to write a test but forgot

lilleyse commented 3 years ago

@ptrgags ready

ptrgags commented 3 years ago

Looks good, thanks @lilleyse!