diff --git a/packages/core/src/io/reader-context.ts b/packages/core/src/io/reader-context.ts index 460934b8..9b56d4af 100644 --- a/packages/core/src/io/reader-context.ts +++ b/packages/core/src/io/reader-context.ts @@ -1,4 +1,5 @@ import type { JSONDocument } from '../json-document.js'; +import type { Document } from '../document.js'; import type { Accessor, Animation, @@ -13,6 +14,7 @@ import type { TextureInfo, } from '../properties/index.js'; import type { GLTF } from '../types/gltf.js'; +import type { ILogger } from '../utils/logger.js'; /** * Model class providing glTF Transform objects representing each definition in the glTF file, used @@ -22,6 +24,11 @@ import type { GLTF } from '../types/gltf.js'; * @hidden */ export class ReaderContext { + public readonly document: Document; + // TODO(v5): Rename to jsonDocument; + public readonly jsonDoc: JSONDocument; + private readonly logger: ILogger; + public buffers: Buffer[] = []; public bufferViews: Uint8Array[] = []; public bufferViewBuffers: Buffer[] = []; @@ -36,7 +43,13 @@ export class ReaderContext { public animations: Animation[] = []; public scenes: Scene[] = []; - constructor(public readonly jsonDoc: JSONDocument) {} + private _usedURIs = new Set(); + + constructor(document: Document, jsonDoc: JSONDocument) { + this.document = document; + this.jsonDoc = jsonDoc; + this.logger = document.getLogger(); + } public setTextureInfo(textureInfo: TextureInfo, textureInfoDef: GLTF.ITextureInfo): void { this.textureInfos.set(textureInfo, textureInfoDef); @@ -67,4 +80,15 @@ export class ReaderContext { textureInfo.setWrapT(samplerDef.wrapT); } } + + public requestURI(uri: string): string { + // https://github.com/KhronosGroup/glTF/issues/2446 + if (this._usedURIs.has(uri)) { + this.logger.warn(`Duplicate resource URI, "${uri}".`); + return ''; + } + + this._usedURIs.add(uri); + return uri; + } } diff --git a/packages/core/src/io/reader.ts b/packages/core/src/io/reader.ts index 3fdd4ccb..b45e0264 100644 --- a/packages/core/src/io/reader.ts +++ b/packages/core/src/io/reader.ts @@ -40,7 +40,7 @@ export class GLTFReader { /* Reader context. */ - const context = new ReaderContext(jsonDoc); + const context = new ReaderContext(document, jsonDoc); /** Asset. */ @@ -98,7 +98,7 @@ export class GLTFReader { if (bufferDef.extras) buffer.setExtras(bufferDef.extras); if (bufferDef.uri && bufferDef.uri.indexOf('__') !== 0) { - buffer.setURI(bufferDef.uri); + buffer.setURI(context.requestURI(bufferDef.uri)); } return buffer; @@ -172,7 +172,7 @@ export class GLTFReader { } else if (imageDef.uri !== undefined) { texture.setImage(jsonDoc.resources[imageDef.uri]); if (imageDef.uri.indexOf('__') !== 0) { - texture.setURI(imageDef.uri); + texture.setURI(context.requestURI(imageDef.uri)); } } 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..a2807c9a 100644 --- a/packages/core/test/io/platform-io.test.ts +++ b/packages/core/test/io/platform-io.test.ts @@ -179,3 +179,88 @@ 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 + 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', ''], 'removes duplicate buffer URIs'); +}); + +test('read duplicate image URIs', async (t) => { + const io = await createPlatformIO(); + + // https://github.com/KhronosGroup/glTF/issues/2446 + 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', ''], 'removes 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'); +});