Comments (7)
Proposed solution:
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.
from gltf-transform.
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...).
from gltf-transform.
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(...)
.
from gltf-transform.
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.
from gltf-transform.
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)
from gltf-transform.
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.
from gltf-transform.
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.
from gltf-transform.
Related Issues (20)
- Cache cleanup HOT 2
- Draco compression may produce an UNSIGNED_SHORT index buffer with vertex count > 65535 HOT 3
- Change default compression with 'optimize' CLI to Meshopt
- `partition` command is slow for large models HOT 6
- Simplify Draco and Meshopt compression implementations
- Prune fails to clean up accessors referenced by non-root properties HOT 2
- VRM gets treated as glTF instead of GLB HOT 11
- Serialization format for transforms HOT 2
- Roadmap for '@gltf-transform/view' package
- Quantization creates invalid skinning weights for CesiumMan.glb HOT 3
- Support transforms reporting a 'skip' result
- Minor glitch in documentation typings HOT 1
- implements meshoptimizer -sv HOT 1
- sharp error HOT 9
- Improving website credits
- CLI 'validate' command should use exit codes and stderr properly
- optimize removes leaf nodes with --prune-leaves false HOT 2
- Support Morph Targets in '@gltf-transform/view' package
- Remove skin bind matrices HOT 2
- CLI optimize command takes very long but reports it ran in < 200ms HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gltf-transform.