Merging documents with extensions

[javagl]

Describe the bug

This issue has two levels of abstraction:

• How to merge two documents with KHR_xmp_json_ld
• How to implement extensions in a way that allows them to survive a merge operation in general

The second one is related to #958, and very specifically to some difficulties when trying to merge documents that both contain an instance of the EXT_structural_metadata implementation. I spent a few hours debug-stepping through various parts of the code, but no avail.

Eventually, I started to compare the behavior to another extension that defines ROOT-level elements - namely, KHR_xmp_json_ld. This leads to the first point: It looks like KHR_xmp_json_ld can not be merged properly...

To Reproduce

The following program creates two documents, each containing the KHR_xmp_json_ld extension, with one packet. It then merges these documents. The resulting document is written properly. But the "root type" of the extension is Packet - so it's not possible to access both packets.

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

import { ALL_EXTENSIONS } from '@gltf-transform/extensions';
import { KHRXMP } from '@gltf-transform/extensions';
import { Packet } from '@gltf-transform/extensions';

function createTestDocument(testName: string) {
	const document = new Document();
	const xmpExtension = document.createExtension(KHRXMP);
	const packet = xmpExtension.createPacket();
	packet.setContext({
		dc: 'http://purl.org/dc/elements/1.1/',
	});
	packet.setProperty('dc:Creator', { '@list': [testName] });
	const root = document.getRoot();
	root.setExtension('KHR_xmp_json_ld', packet);
	return document;
}

async function run() {
	const documentA = createTestDocument('creatorA');
	const documentB = createTestDocument('creatorB');
	const document = new Document();

	document.merge(documentA);
	document.merge(documentB);

	const root = document.getRoot();
	const packet = root.getExtension<Packet>('KHR_xmp_json_ld');
	//console.log("Result:\n", result);
	const creator = packet?.getProperty('dc:Creator');
	console.log('creator', creator);

	const io = new NodeIO();
	io.registerExtensions([...ALL_EXTENSIONS]);
	const jsonDocument = await io.writeJSON(document);
	console.log(JSON.stringify(jsonDocument.json, null, 2));
}

run();

Expected behavior

The general question is: When there is an extension that defines top-level PropertyType.ROOT elements: Which precautions have to be taken or which "hooks" into the code can be employed to properly handle these during a merge operation?

For KHR_xmp_json_ld, one path might be to have to be some (trivial) class PacketList extends ExtensionProperty that just is an array of Packet objects and serves as the top-level type for the extension. (Further details and an example are actually given in the extension specs). Then, the question will be: At which point of the merge process will the contents of these PacketList-objects be merged?

(For the more general case, I hope that any draft/sketch/idea of how this could be handled for KHR_xmp_json_ld could help me to figure out a solution for EXT_structural_metadata as well 🤞 )

Versions:

• Version: 3.10.1
• Environment: All

[donmccurdy]

KHR_xmp_json_ld does not really allow multiple packets associated with the top-level asset – assigning the packet to the root in glTF Transform is equivalent to setting:

"asset": {
    "extensions": {
      "KHR_xmp_json_ld": {
        "packet": 0
      }
    },
 ...

... and the root-level list of packets is still generated automatically, but there can only be one packet on the asset. Moreover, there is a special process by which one should technically create a new packet with 'ingredients' based on other packets:

https://developer.adobe.com/xmp/docs/XMPNamespaces/xmpMM/

See xmpMM:Ingredients. Or you could attach packets to specific nodes or materials sourced from each of the two Documents, and not have a top-level packet at all.

It is ... complicated. And there's no obvious right answer for what 'merge' is to XMP, so I've left that in userland.

---

None of the above helps you with EXT_structural_metadata, of course. I wouldn't be opposed to adding new extension hooks to handle special logic in a merge, but, I wonder if that's going to require defining opinionated ideas of what a 'merge' is, unnecessarily?

Would something like #924 be better here? I think that might allow you to exercise more control over what is actually happening rather than just calling document.merge(...).

[javagl]

The reference to KHR_xmp_json_ld just fell out of my attempt to figure out what's currently wrong with EXT_structural_metadata. As such, it might have become a 'red herring': Maybe there are aspects of KHR_xmp_json_ld that are too specific to be discussed as instances of the general question.

(For example, I now saw that the "root-level" packets list is also only generated by the writer, and ... the packet reference is snuck into the asset when the parent is ROOT...)

---

Would something like #924 be better here?

I think that the goal would not be to transfer properties. The source should be unaffected by whatever is happening there. But maybe I'm overlooking a connection on a lower level of the implementation.

I wouldn't be opposed to adding new extension hooks to handle special logic in a merge

Preferably, that should not be necessary. The EXT_structural_metadata implementation is huge and complex. And it would probably be better to create some DUMMY_example_extension implementation that illustrates to problem that I'm currently seeing in EXT_structural_metadata:

• There are two documents
• Each document has a root.
• Each root contains a single StructuralMetadata object.
• These two documents are supposed to be merged.
• There is no way to define which StructuralMetadata object is supposed to be contained in the merged document
  • (And it cannot just be either of them - it has to be a single, merged object)

It might be that the ExtStructuralMetadata/StructuralMetadata implementation structure is flawed in a non-obvious way. (Although it worked pretty well until the merge came up...). I think that the parentTypes: [PropertyType.ROOT] in the StructuralMetadata class is playing a role in the issue that I'm observing. There is no other extension implementation where an ExtensionProperty has the ROOT as the parent (except for KHR_xmp_json_ld, but this has a special handling).

The general problem of having to do a manual merge operation for a specific ExtensionProperty might also be something that is not supposed to be covered by merge. So I'll also explore the path of calling merge, throwing away any resulting StructuralMetadata, and re-creating and assigning the merged one. But this wouldn't solve the issue on the level of the implementation itself, and users of the extension implementation would likely expect that.

[javagl]

Sorry for the "walls of text" and the (probably just distracting) reference to KHR_xmp_json_ld.

The core of this issue could be boiled down to:

• There is an extension that defines an ExampleExtensionProperty
• Two documents that contain this extension should be merged
• The result could contain a single ExampleExtensionProperty
• There is no way to say how to create this single ExampleExtensionProperty from the two inputs

As such, this is not a "bug". It is, at most, a limitation in corner cases, where some special handling is required for merging the data of two 'instances' of extensions into a single object.

Depending on your preference, this issue (with the distracting first part) could be closed, and the summary that I gave above could be moved into a new issue.

In any case, it's not something where a quick and good solution could be pulled out of a hat (and I'll try to find any solution for the specific case of EXT_structural_metadata independently)

[donmccurdy]

Would something like #924 be better here?

I think that the goal would not be to transfer properties. The source should be unaffected by whatever is happening there. But maybe I'm overlooking a connection on a lower level of the implementation.

Ignoring the move vs. copy distinction, what I mean is a more granular way to get specific contents of Document A into Document B. Currently the only method is b.merge(a), which provides no specificity or control. Compare to a hypothetical function like:

const meshesA = documentA.getRoot().listMeshes().slice(0, 5);
const meshesB = documentB.cloneExternal(meshesA);

There is no way to say how to create this single ExampleExtensionProperty from the two inputs

I think that's the crux of the problem, yes. For a.merge(b), the result should have a union of all resource lists (root.meshes, root.nodes, root.scenes, ...) from the two documents. And the library should probably have a consistent opinion about whether the generator string and root-level extensions from Document a or Document b should win. But I think that defining a framework for consolidating two unknown root-level extensions into one may turn out to be more complicated than the merge() function itself, so perhaps that should remain out of scope.

I am tempted to remove the merge() method from the Document class, and put it into @gltf-transform/functions instead, as something like mergeDocuments(a, b). It could then be tree-shaken. And it might be helpful to communicate that it is only one possible implementation, and not the only possible way to combine two documents.

[javagl]

I now see the connection to the 'transferring properties' issue. And yes, I'm currently thinking that it will be necessary to define "copy-constructors" or clone functions for the classes that are part of the extension. This should allow something like merged.setExtension(..., manualMerge(extensionA, extensionB)) that is not part of the core merge functionality.

It would probably be possible to implement such a cloneExternal function pretty generically (and I fact, I think that Property.copy is already covering most of that). But in the limit case, it will raise the same question as the current merge, namely: If there has to be a special treatment for certain objects/types, then there has to be a way to 'intercept' the generic copy/clone process. (The copy function already receives that PropertyResolver<Property>, but my gut feeling is that this might not be enough for a fully generic and configurable cloning function)

The current merge already covers 99% of what is 'commonly needed'. The special case of having to create a single 'object' from two input objects (that may even be hidden deeply within the data structures that an extension defines) cannot be solved trivially. And any approach be carefully thought through, with the bigger picture in mind. (For example: If there is a function for 'transferring properties', then any generic, configurable 'merge' function would likely leverage that existing functionality).

All these points are reasons to consider this as 'out of scope' for now.

[donmccurdy]

Proposed solution:

• feat(functions): Add mergeDocuments, cloneDocument, copyToDocument, moveToDocument #1375

tl;dr - Root-level properties, including the generator string and root-level extensions, are not transferred when merging two documents. For more granular control, the new copyToDocument(target, source, sourceProperties) function may be used instead. A custom resolver may be passed to copyToDocument to override (in a limited way) how it resolves certain property types. The implementations will move to @gltf-transform/functions, which should make it easier to fork these functions in userland when needed.

[javagl]

Just a short "Ack 👍 " here:

I started implementing the merge functionality for EXT_structural_metadata, using the newly introduced functions (very drafty right now - just using a local copy of copyToDocument, until it is part of a release). It seems like it should work as expected, and allow assembling the merged extension document pretty easily. (I.e. any "complexity" now lies in determining how to merge two metadata schema objects and such...).