diff --git a/packages/core/src/io/reader-context.ts b/packages/core/src/io/reader-context.ts index 9b56d4af..460934b8 100644 --- a/packages/core/src/io/reader-context.ts +++ b/packages/core/src/io/reader-context.ts @@ -1,5 +1,4 @@ import type { JSONDocument } from '../json-document.js'; -import type { Document } from '../document.js'; import type { Accessor, Animation, @@ -14,7 +13,6 @@ 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 @@ -24,11 +22,6 @@ import type { ILogger } from '../utils/logger.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[] = []; @@ -43,13 +36,7 @@ export class ReaderContext { public animations: Animation[] = []; public scenes: Scene[] = []; - private _usedURIs = new Set(); - - constructor(document: Document, jsonDoc: JSONDocument) { - this.document = document; - this.jsonDoc = jsonDoc; - this.logger = document.getLogger(); - } + constructor(public readonly jsonDoc: JSONDocument) {} public setTextureInfo(textureInfo: TextureInfo, textureInfoDef: GLTF.ITextureInfo): void { this.textureInfos.set(textureInfo, textureInfoDef); @@ -80,15 +67,4 @@ 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 b45e0264..3fdd4ccb 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(document, jsonDoc); + const context = new ReaderContext(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(context.requestURI(bufferDef.uri)); + buffer.setURI(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(context.requestURI(imageDef.uri)); + texture.setURI(imageDef.uri); } } diff --git a/packages/core/src/io/writer-context.ts b/packages/core/src/io/writer-context.ts index 2f63559d..717d15f2 100644 --- a/packages/core/src/io/writer-context.ts +++ b/packages/core/src/io/writer-context.ts @@ -184,26 +184,10 @@ export class WriterContext { } else { const extension = ImageUtils.mimeTypeToExtension(texture.getMimeType()); imageDef.uri = this.imageURIGenerator.createURI(texture, extension); - this.assignResourceURI(imageDef.uri, data); + this.jsonDoc.resources[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 45546943..8b74dbc1 100644 --- a/packages/core/src/io/writer.ts +++ b/packages/core/src/io/writer.ts @@ -437,11 +437,9 @@ export class GLTFWriter { .filter((extension) => extension.prewriteTypes.includes(PropertyType.BUFFER)) .forEach((extension) => extension.prewrite(context, PropertyType.BUFFER)); - const needsBuffer = - root.listAccessors().length > 0 || - context.otherBufferViews.size > 0 || - (root.listTextures().length > 0 && options.format === Format.GLB); - if (needsBuffer && root.listBuffers().length === 0) { + const hasBinaryResources = + root.listAccessors().length > 0 || root.listTextures().length > 0 || context.otherBufferViews.size > 0; + if (hasBinaryResources && root.listBuffers().length === 0) { throw new Error('Buffer required for Document resources, but none was found.'); } @@ -554,7 +552,7 @@ export class GLTFWriter { // Write buffer views to buffer. bufferDef.byteLength = bufferByteLength; - context.assignResourceURI(uri, BufferUtils.concat(buffers)); + jsonDoc.resources[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 a2807c9a..342813c5 100644 --- a/packages/core/test/io/platform-io.test.ts +++ b/packages/core/test/io/platform-io.test.ts @@ -179,88 +179,3 @@ 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'); -});