Add spliceNode function to reparent an entire scene or subtree

[rotu]

This is inspired by #1530 (comment). In particular, instead of a transformScene function, this creates a new Node to which you can apply transforms.

[rotu]

I'm not sure about the API. In particular, the const newNode = new Node(parent.getGraph()). I'm not sure if it's kosher for a function to create a Node object outside of document.createNode(). The alternative is to require the caller to pass in the new parent, e.g.

const newParent = document.createNode().setRotation([1,0,0,0]);
spliceNode(oldParent, newParent);

but then it's not clear what to do if newParent already has a parent. Or worse, if newParent is already a child of oldParent.

Also not totally happy with the name of this function.

[donmccurdy]

It's OK for the function to create a node, I think, as long as the naming is consistent with that expectation. Calling the Node constructor (or other Property subclass constructors) directly will cause issues though, I would copy the pattern here:

glTF-Transform/packages/functions/src/list-texture-slots.ts

Lines 13 to 16 in 1693ffd

	export function listTextureSlots(texture: Texture): string[] {
	const document = Document.fromGraph(texture.getGraph())!;
	const root = document.getRoot();
	const slots = texture

I'd like to make that more obvious at some point, but in the meantime this avoids requiring users to pass in a document to functions unnecessarily.

That said, having the user pass in both nodes would sound OK to me too.

[donmccurdy]

Thinking more about my recommendation from #1530 (comment) here, and unfortunately it's not as simple as I'd been hoping.

1. As mentioned, we will get validation warnings if we put a parent above a skinned mesh. Technically it's valid, these are just 'hints' or 'warnings' at the validator level, because the transform won't do anything to the skinned mesh directly, transforms affect only the joints.
2. However, it's possible that the skinned mesh's joints are descendants of the node. In that case, skipping the skinned mesh will cause the transform to not affect the joints as expected, and thus not to affect the skinned mesh.

Probably (2) is the more critical error, as it will change the rendered result.

We could search the child's subtree for any joints, and skip the skinned mesh only if no joints are found. But this might be a more complex and less predictable operation than users are expecting, calling a method like 'spliceNode'. If we skip that special case, then the method body becomes something more like:

const node = Document.fromGraph(parent.getGraph()).createNode();
for (const child of parent.listChildren()) {
  node.addChild(child);
}
parent.addChild(newNode);

[rotu]

It's OK for the function to create a node, I think, as long as the naming is consistent with that expectation. Calling the Node constructor (or other Property subclass constructors) directly will cause issues though, I would copy the pattern here:

glTF-Transform/packages/functions/src/list-texture-slots.ts

Lines 13 to 16 in 1693ffd

	export function listTextureSlots(texture: Texture): string[] {
	const document = Document.fromGraph(texture.getGraph())!;
	const root = document.getRoot();
	const slots = texture

I'd like to make that more obvious at some point, but in the meantime this avoids requiring users to pass in a document to functions unnecessarily.

Here are some passing notes about how I was confused. I'm sure more hacking on this will clear things up:

• I didn't use fromGraph because I inferred from the name (incorrectly, but in analog with Array.from(...)) that it creates a new Document. It also has the problem that it can return null and it's not clear what to do if the node is merely part of a document fragment.
• getGraph is fuzzy about whether the returned object is necessarily the same for all GraphNodes, or if it's just the reachable subgraph (subtree for Node, since the node hierarchy is acyclic)
• I'm super uncertain about the distinction between a Graph and a Document and what's needed in order to maintain class invariants. I called the constructor that way because that's what the createNode function seems to do.