fix(io): Handle duplicate URIs on read/write

[donmccurdy]

• Fixes Object transforms look off after using Instance #1509
• Related Multiple buffers, duplicate URIs? KhronosGroup/glTF#2446

Changes:

• While reading, distinct resources (buffers, images) with the same URIs remain distinct, but the URI is removed from all but the first, such that a new URI will be generated for the remainder when writing. A warning is logged.
• While writing, a warning is logged if two resources share the same URI. If the two resources are not identical, an error is thrown.

To explicitly consolidate duplicate resources, the dedup() or unpartition() functions may be used.

[javagl]

A related question came up for me in JglTF recently. In that context, I did a few experiments with glTF-Transform, and referred to that in a comment at javagl/JglTF#115 (comment)

The test that I did there, and where one could argue about whether it is a "bug" or only a "maybe somewhat surprising behavior" of glTF-Transform is this:

import fs from "fs";
import { Document } from "@gltf-transform/core";
import { NodeIO } from "@gltf-transform/core";

async function run() {
  const io = new NodeIO();
  const document = new Document();
  document.createBuffer();

  const image0 = fs.readFileSync("image0.png");
  const texture0 = document.createTexture();
  texture0.setMimeType("image/png");
  texture0.setImage(image0);

  const image1 = fs.readFileSync("image1.png");
  const texture1 = document.createTexture();
  texture1.setMimeType("image/png");
  texture1.setImage(image1);

  //texture0.setURI("test.png");
  texture1.setURI("texture_1.png");

  const output = await io.writeJSON(document);
  console.log(JSON.stringify(output.json, null, 2));
  console.log(output.resources);
}

run();

It only sets the URI of the second texture, via texture1.setURI("texture_1.png");, and this causes the first texture to be overwritten, because it happens to receive the auto-generated URI "texture_1.png" as well....

---

Crying wolf: The part in the comment above that says

... the URI is removed from all but the first,

sounds like it could cause unexpected behavior, but I haven't looked into the details yet.

[donmccurdy]

Reverting in 84e77f5, due to #1513.

The immediate the problem seems to be the "import" side, though I think you raise some good points about export. The export side might be better served if unique URL generation were aware of every other resource URL in the document, and not just the unnamed resources.

[javagl]

every other resource URL

That generalization is also a valid point. I could imagine that some of the functions in JglTF will produce unexpected results when someone creates a buffer and a texture that both have a URI like "nastyCase.xyz": It will not considered to be duplicate in either of the functions that handle duplicates, but something might be overwritten.
(Additionally, there may be extensions that may contain URIs... ).

Given these complications, I think that it may indeed be warranted to sort out some of these things in the context of KhronosGroup/glTF#2446 , and maybe add 'Implementation Notes' about this sort of duplication.

The export side might be better served if unique URL generation ,,,,

... iff this should be done. The question is whether it should realy be impossible to create a glTF like the one that you posted in the issue....

[donmccurdy]

Updates to this PR coming in:

• fix(io): Handle duplicate URIs on read/write (v2) #1522

I may consider making this handling more strict in v5 or another major release, whether it's considered invalid by the spec or not, but still TBD.

It only sets the URI of the second texture, via texture1.setURI("texture_1.png");, and this causes the first texture to be overwritten, because it happens to receive the auto-generated URI "texture_1.png" as well....

Currently happening here:

glTF-Transform/packages/core/src/io/writer-context.ts

Lines 233 to 252 in 93ca869

	export class UniqueURIGenerator<T extends Texture | Buffer> {
	private counter = {} as Record<string, number>;
	constructor(
	private readonly multiple: boolean,
	private readonly basename: (t: T) => string,
	) {}
	public createURI(object: T, extension: string): string {
	if (object.getURI()) {
	return object.getURI();
	} else if (!this.multiple) {
	return `${this.basename(object)}.${extension}`;
	} else {
	const basename = this.basename(object);
	this.counter[basename] = this.counter[basename] || 1;
	return `${basename}_${this.counter[basename]++}.${extension}`;
	}
	}
	}

The result won't always be texture_1, it would be <slot>_1.png if the texture were assigned to a material, but still conflicts are possible if the input scene has a mix of external and embedded (no URI) textures, or if textures are added through the scripting API without assigning unique URIs. Improvements here are welcome but not currently planned.