Skip to content

Commit

Permalink
Restore and improve "fix(io): Handle duplicate URIs on read/write (#1511
Browse files Browse the repository at this point in the history
)"

Partially restores commit 13d83a7.
  • Loading branch information
donmccurdy committed Oct 8, 2024
1 parent 95c2eb6 commit 2b468ed
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 5 deletions.
18 changes: 17 additions & 1 deletion packages/core/src/io/writer-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/io/writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
}

Expand Down Expand Up @@ -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);
Expand Down
87 changes: 87 additions & 0 deletions packages/core/test/io/platform-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});

0 comments on commit 2b468ed

Please sign in to comment.