Unexpected handling of `generator` during `merge`

[javagl]

This might not be considered to be an "issue", but ... I stumbled over this, and find it at least unexpected: When calling merge, then the asset.generator of the target document is apparently just overwritten with the one from the merged-in document.

To Reproduce

Run this:

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

async function run() {
  const documentA = new Document();
  const documentB = new Document();
  const documentC = new Document();
  documentA.getRoot().getAsset().generator = "Generator A";
  documentB.getRoot().getAsset().generator = "Generator B";
  documentC.getRoot().getAsset().generator = "Generator C";
  documentA.merge(documentB);
  documentA.merge(documentC);
  console.log(documentA.getRoot().getAsset().generator);
  console.log(documentB.getRoot().getAsset().generator);
  console.log(documentC.getRoot().getAsset().generator);
}

run();

The output will be

Generator C
Generator B
Generator C

Expected behavior

That's the tough one 🙂 The generator is basically a "free-text" field without any specified meaning. But roughly speaking, I'd expect that ~"no information is lost". A simple and straightforward solution could be just just append the merged-in generator to the one that is already present, yielding

Generator A, Generator B, Generator C
Generator B
Generator C

in the above example. But... with the obvious caveat that the output should't be

Generator X, Generator X, Generator X, Generator X, Generator X, Generator X, Generator X, Generator X

when merge-assembling one target document from 8 partial input documents.

Regardless of what the solution could or will be, I thought it might be worth bringing this up...

Versions:

• 3.10.1
• Environment: All

[donmccurdy]

I'm not aware of any precedent for comma-separated generator strings, and I'm hesitant to invent one... I think this is kind of what XMP's 'ingredient' concept was made for, but it's complicated and subjective enough (discussed in #1367) that I don't think glTF Transform should try to do that in merge() automatically.

Is this a problem for your use case? My hunch is that your application knows better than glTF Transform can know, whether one generator string should win, or whether concatenating them is meaningful... It might be best to assign the generator string directly in the script above.

In terms of the default behavior — I'm tempted to say that the original generator string on the document should be kept, and not modified in a merge, which feels more semantically defensible than last-document-in-wins.

One could also argue that the generator should be "glTF Transform". ;)

[javagl]

The comma-separated list was just an overly suggestive attempt to "retain all information" (with the obvious caveats).

I'd also agree that keeping the generator of the target document when merging in others could make more sense. (And that's what I assumed to happen ... but... fixed it...). Justifications for that could be...

• the order in which documents are merged into a target should not matter
• the document that others are merged in is usually generated by glTF-Transform (if it was created from scratch)

But I don't have a strong opinion about the solution. Knowing what the behavior is allows users to set the generator as they see fit, and I just wanted to bring it up. (So the issue could be closed if there's no strong reason to change anything).

[donmccurdy]

Proposed solution:

• https://github.com/donmccurdy/glTF-Transform/pull/1375

tl;dr - a.merge(b) is replaced by mergeDocuments(a, b). The generator string of the target document will be kept when merging, along with other root-level properties. For more granular control, the new copyToDocument(target, source, sourceProperties) function may be used instead.