From 2b468ed57dc5a8b220dd23693597317f882d28d1 Mon Sep 17 00:00:00 2001 From: Don McCurdy Date: Mon, 7 Oct 2024 22:38:01 -0400 Subject: [PATCH] Restore and improve "fix(io): Handle duplicate URIs on read/write (#1511)" Partially restores commit 13d83a75d0231fe159f3c7c1859e98c167d06e7f. --- packages/core/src/io/writer-context.ts | 18 ++++- packages/core/src/io/writer.ts | 10 +-- packages/core/test/io/platform-io.test.ts | 87 +++++++++++++++++++++++ 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/packages/core/src/io/writer-context.ts b/packages/core/src/io/writer-context.ts index 717d15f2..2f63559d 100644 --- a/packages/core/src/io/writer-context.ts +++ b/packages/core/src/io/writer-context.ts @@ -184,10 +184,26 @@ export class WriterContext { } else { const extension = ImageUtils.mimeTypeToExtension(texture.getMimeType()); imageDef.uri = this.imageURIGenerator.createURI(texture, extension); - this.jsonDoc.resources[imageDef.uri] = data; + this.assignResourceURI(imageDef.uri, data); } } + public assignResourceURI(uri: string, data: Uint8Array): void { + const resources = this.jsonDoc.resources; + + // https://github.com/KhronosGroup/glTF/issues/2446 + if (uri in resources && data !== resources[uri]) { + throw new Error(`Resource URI "${uri}" already assigned to different data.`); + } + + if (uri in resources) { + this.logger.warn(`Duplicate resource URI, "${uri}".`); + return; + } + + resources[uri] = data; + } + /** * Returns implicit usage type of the given accessor, related to grouping accessors into * buffer views. Usage is a superset of buffer view target, including ARRAY_BUFFER and diff --git a/packages/core/src/io/writer.ts b/packages/core/src/io/writer.ts index 8b74dbc1..45546943 100644 --- a/packages/core/src/io/writer.ts +++ b/packages/core/src/io/writer.ts @@ -437,9 +437,11 @@ export class GLTFWriter { .filter((extension) => extension.prewriteTypes.includes(PropertyType.BUFFER)) .forEach((extension) => extension.prewrite(context, PropertyType.BUFFER)); - const hasBinaryResources = - root.listAccessors().length > 0 || root.listTextures().length > 0 || context.otherBufferViews.size > 0; - if (hasBinaryResources && root.listBuffers().length === 0) { + const needsBuffer = + root.listAccessors().length > 0 || + context.otherBufferViews.size > 0 || + (root.listTextures().length > 0 && options.format === Format.GLB); + if (needsBuffer && root.listBuffers().length === 0) { throw new Error('Buffer required for Document resources, but none was found.'); } @@ -552,7 +554,7 @@ export class GLTFWriter { // Write buffer views to buffer. bufferDef.byteLength = bufferByteLength; - jsonDoc.resources[uri] = BufferUtils.concat(buffers); + context.assignResourceURI(uri, BufferUtils.concat(buffers)); } json.buffers!.push(bufferDef); diff --git a/packages/core/test/io/platform-io.test.ts b/packages/core/test/io/platform-io.test.ts index 342813c5..145ab4aa 100644 --- a/packages/core/test/io/platform-io.test.ts +++ b/packages/core/test/io/platform-io.test.ts @@ -179,3 +179,90 @@ test('glb with unknown chunk', async (t) => { const node = document.getRoot().listNodes()[0]; t.is(node.getName(), 'RootNode', 'parses nodes'); }); + +test('read duplicate buffer URIs', async (t) => { + const io = await createPlatformIO(); + + // https://github.com/KhronosGroup/glTF/issues/2446 + // https://github.com/donmccurdy/glTF-Transform/issues/1513 + const jsonDocument: JSONDocument = { + json: { + asset: { version: '2.0' }, + buffers: [ + { uri: 'scene.bin', byteLength: 20 }, + { uri: 'scene.bin', byteLength: 20 }, + ], + }, + resources: { 'scene.bin': new Uint8Array(20) }, + }; + + const document = await io.readJSON(jsonDocument); + + const bufferURIs = document + .getRoot() + .listBuffers() + .map((buffer) => buffer.getURI()); + + t.deepEqual(bufferURIs, ['scene.bin', 'scene.bin'], 'removes duplicate buffer URIs'); +}); + +test('read duplicate image URIs', async (t) => { + const io = await createPlatformIO(); + + // https://github.com/KhronosGroup/glTF/issues/2446 + // https://github.com/donmccurdy/glTF-Transform/issues/1513 + const image = new Uint8Array(20); + const jsonDocument: JSONDocument = { + json: { + asset: { version: '2.0' }, + images: [{ uri: 'color.png' }, { uri: 'color.png' }], + }, + resources: { 'color.png': image }, + }; + + const document = await io.readJSON(jsonDocument); + + const imageURIs = document + .getRoot() + .listTextures() + .map((texture) => texture.getURI()); + const images = document + .getRoot() + .listTextures() + .map((texture) => texture.getImage()); + + t.deepEqual(imageURIs, ['color.png', 'color.png'], 'loads duplicate image URIs'); + t.deepEqual(images, [image, image], 'loads duplicate image data'); +}); + +test('write duplicate buffer URIs', async (t) => { + const io = await createPlatformIO(); + + // https://github.com/KhronosGroup/glTF/issues/2446 + const document = new Document(); + const buffer1 = document.createBuffer().setURI('scene.bin'); + const buffer2 = document.createBuffer().setURI('scene.bin'); + document.createAccessor().setBuffer(buffer1).setArray(new Float32Array(10).fill(1)); + document.createAccessor().setBuffer(buffer2).setArray(new Float32Array(10).fill(2)); + + await t.throwsAsync(() => io.writeJSON(document), { message: /Resource URI/i }, 'duplicate buffer URIs'); + + buffer2.setURI('aux.bin'); + + t.truthy(await io.writeJSON(document), 'unique buffer URIs'); +}); + +test('write duplicate image URIs', async (t) => { + const io = await createPlatformIO(); + + // https://github.com/KhronosGroup/glTF/issues/2446 + const document = new Document(); + const image1 = document.createTexture().setImage(new Uint8Array(2)).setURI('color.png'); + document.createTexture().setImage(new Uint8Array(1)).setURI('color.png'); + + await t.throwsAsync(() => io.writeJSON(document), { message: /Resource URI/i }, 'duplicate image URIs'); + + image1.setURI('normal.png'); + + t.truthy(await io.writeJSON(document), 'unique image URIs'); +});