Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Individual mesh picking on consolidated meshes - gltfpack #165

Closed
sweco-sekrsv opened this issue Jul 9, 2020 · 22 comments
Closed

Individual mesh picking on consolidated meshes - gltfpack #165

sweco-sekrsv opened this issue Jul 9, 2020 · 22 comments
Labels

Comments

@sweco-sekrsv
Copy link

I saw the discussion over here:
KhronosGroup/glTF#1699

Have you already added support for this or have plans to add support for custom attributes? To be able to store the original node index integer in a custom attribute to be able to do individual mesh picking and also fetch extras data from the consolidated meshes. I think it would be very useful for many people in the AEC/CAD/BIM industry, making these models alot more accessible in more tools.

@zeux
Copy link
Owner

zeux commented Jul 10, 2020

Support for this hasn't been added. To do this we need to establish a specific naming convention, since gltfpack needs to be able to reason about this attribute in terms of expected format, and figure out if that's something that an external application should feed into gltfpack or gltfpack should synthesize by itself.

One alternate possibility is to use EXT_mesh_gpu_instancing; early experiments on CAD-like models with that were pretty encouraging. gltfpack supports it out of the box in recent builds using -mi command line option. This is nicer because it is a more idiomatic way to represent large collections of objects in glTF while still keeping an ability to distinguish one object from another, although it's not as performant on scenes with many different kinds of objects.

@zeux zeux added the gltfpack label Jul 10, 2020
@sweco-sekrsv
Copy link
Author

sweco-sekrsv commented Jul 10, 2020

Interesting , I'll have a look at the EXT_mesh_gpu_instancing. Do you already have an example in babylon.js using this that you can share?

What defines a mesh being the same(instance) as another in the input gltf file?

ObjectA - a windowframe
Extras data - attribute A
Unique material: Red
ObjectB
Same geometry as ObjectA
Extras data - attribute B
Unique material: Red
ObjectC
Same geometry as ObjectA
No extras data
Unique material: Red
ObjectD
Same geometry as ObjectA
No extras data
Unique material: Red
ObjectE
Same geometry as ObjectA
No extras data
Unique material: Blue

In the above case, would ObjectC and ObjectD become one merged mesh object or will they be instances of each other or instances of all the objects?
Would the output file contain only two materials, red and blue even if there are 5 unique materials in the original gltf file?

Do you also instansiate subelements in a mesh?
For example if we have 1000 windowframes which beforehand have been collapsed to the same meshobject, are you looking at the subelements of the mesh to identify windowframes extract them and make them into instances? Or would that be worse performant? (they would all share the same extras data) . I guess its a balance between filesize and optimization as well.

@zeux
Copy link
Owner

zeux commented Jul 10, 2020

See attached file BabylonJS/Babylon.js#8164 (performance issues have been fixed since)

What defines a mesh being the same(instance) as another in the input gltf file?

Same geometry and material. Currently gltfpack ignores extras for the purpose of instancing, so it's lost on instanced meshes. This is the same for mesh merging as well.

For example if we have 1000 windowframes which beforehand have been collapsed to the same meshobject

If these are primitives of the single mesh object then they will be instanced, if the geometry has been merged into a single primitive then gltfpack won't split it back.

@sweco-sekrsv
Copy link
Author

Thanks, I tried the file you linked to and it loads quickly in babylon.js. Very impressive! I also tried picking in babylon.js but that does not work on meshes using EXT_mesh_gpu_instancing unfortunately. Not sure if it's a bug or by design.

Even if it did work, we still would need to map the node names/ids to the correct extras metadata.(not sure if there were any in the orginal file you converted). After all the optimization of gltfpack is done, would it be possible that gltfpack could also export a file (maybe json) containing all the node id/names linked to the correct extras metadata? It would be nice however if this data could be compressed and contained in the glb file as well. Basically if there are many objects that have extras metadata in the original file I can see the json file grow quite rapidly but at least there is a possibility to load both the glb and the json data separatly and match them together in runtime when needed. I think wallabyway also describes this as a 'source map' side file in the issue i originally linked to in my first post.

I was going to try the -mi command line option but it's not availeble in the latest release(0.14). Is it possible to get a new release which includes this option? I'm on Windows.

I will also prepare a gltf file which will contain lots of extras for the objects, typical fpr CAD-like files.

@zeux
Copy link
Owner

zeux commented Jul 13, 2020

It would be nice however if this data could be compressed and contained in the glb file as well.

This is possible in principle, although we'd need to define the format that this data is saved in more precisely.

I was going to try the -mi command line option but it's not availeble in the latest release(0.14). Is it possible to get a new release which includes this option? I'm on Windows.

You can get builds from master from GitHub Actions page, e.g. https://github.com/zeux/meshoptimizer/actions/runs/158394020

@sweco-sekrsv
Copy link
Author

Thanks, I did not notice that you had actions for the builds.

It seems that babylon is going to add support for picking on meshes using EXT_mesh_gpu_instancing, see this thread:
https://forum.babylonjs.com/t/picking-instances-using-ext-mesh-gpu-instancing/12399

See attached file for a typical BIM example which contain extras for many nodes. The extras attached to the nodes are in file Schependomlaan_extras.gltf. Schependomlaan.gltf has the same meshes but with no extras data attached to the nodes. As you can see the file size can grow quite much with many extras per node.

If you need more files to test with, just let me know and I'll prepare some more!

Schependomlaan_gltf.zip

@starhopper1
Copy link

I have the same problem, would be awesome if you could save the nodes in the glb, maby as extra option. I have converted the example step files from opencascade to glb, and when i optimize them with you tool the nodes are gone. :/
I know for this little glb its fine if i just load them directly, but we have much bigger glb that are nearly impossible to load.
But once i optimize them with your tool its nearly instant loading!

as1-oc-214_5.zip

@zeux
Copy link
Owner

zeux commented Jul 23, 2020

@starhopper1 Have you tried using -kn flag?

@starhopper1
Copy link

Ah yes, perfekt sorry, did not see that modifier... thank you very much! 👍

@zeux
Copy link
Owner

zeux commented Sep 29, 2020

@sweco-sekrsv If Babylon.JS now supports picking for instances (looks like it was merged about a month ago), does that workflow work for you when using -mi in gltfpack? It feels like this is the right solution here and that this issue is addressed by that, but I want to make sure before closing it.

@sweco-sekrsv
Copy link
Author

The picking does not seem to work yet, at least not with the stadium scene. I'll make a report about that over at babylon.js forum
However even if it was working I cant understand how to able to pick the collapsed meshes to get the orginal ID of the meshes. Maybe I'm missing something?

Look at my attached scene above (Schependomlaan_gltf.zip)
Original gltf has 5693 drawcalls/meshes

After gltfpack its 34 draw calls/meshes

After gltfpack with -mi its also 34 draw calls/meshes. So it does not seem that the -mi option produces any thin instances on this particular scene so even if thin instances picking would work I could not get this scene working.

@zeux
Copy link
Owner

zeux commented Oct 3, 2020

@sweco-sekrsv On that scene visually it looks like there are a lot of repeated elements, but from the scene composition perspective it looks like there's no sharing of geometry - is that intentional? The way -mi works right now is that it only uses instancing for nodes that refer to the same original mesh, and doesn't deduplicate based on mesh contents.

This deduplication is certainly possible to implement, other CAD scenes I've seen generally had meshes that were already deduplicated to a large extent so this hasn't been done yet.

@zeux
Copy link
Owner

zeux commented Dec 21, 2020

I don't think there's actionable items left here so I'm going to close this for now. Discussion is welcome to continue in discussion threads in the relevant section of the repository (I've enabled the new GitHub Discussions feature); my expectation here in general is that EXT_mesh_gpu_instancing is going to be a good way to encode CAD scenes. If per-instance ids / names need to be provided, this would require some sort of standard or semi-standard (via extras?) way to specify those, which would need to be decided as part of glTF ecosystem and then gltfpack can implement that.

@zeux zeux closed this as completed Dec 21, 2020
@wallabyway
Copy link

wallabyway commented Oct 20, 2021

If glTFpack could add an ID to the vertex attribute, before doing the mesh-merge, same for Instanced meshes, based on this extension...
EXT_mesh_features / Vertex ID
ref: KhronosGroup/glTF#2082

feature

...Then, a viewer could still do 'picking' based on the vertexID, and rendering would be fast (since '-cc' option reduces draw calls).

The ID of each node, would come from the node/name. Process the node/name string as either a Number() or create a string murmur3 hash )... but something agreeable.

@zeux
Copy link
Owner

zeux commented Oct 21, 2021

It seems reasonable for me to use this extension optionally to provide the original node names for each individual mesh in the aggregate. Based on my cursory read of this extension I think it would be possible in a uniform way for both mesh merging and instance merging, via some separate option like -mf that would instruct gltfpack to add this metadata to the output.

Feel free to open a separate issue with this feature request. My understanding is that the spec is relatively free form, would be great to note what specific details would need to be exposed - is it just node name or is there anything else.

I'd prefer to wait until the EXT_mesh_features extension is merged (in draft form) to glTF repository to minimize the possibility of churn. It would be ideal if some viewers planned to support it as well, but it's reasonably straightforward and can probably be implemented "blindly" (without having a viewer to validate).

@donmccurdy
Copy link
Contributor

My understanding is that the spec is relatively free form, would be great to note what specific details would need to be exposed - is it just node name or is there anything else.

Expected property definitions for "ID" and "name" properties might look something like the snippet below. The semantic values ("ID" and/or "NAME") would need to be exact and case-sensitive. I think "NAME" seems more appropriate for this case; "ID" is meant for cases where the source data might have had non-numeric IDs and so the application needs more context in addition to the integer "FEATURE_ID" attribute.

"properties" {
  "id": {"componentType": "STRING", "semantic": "ID"},
  "name": {"componentType": "STRING", "semantic": "NAME"},
}             

Some implementation work is in progress, but feedback is certainly welcome on the PR as well!

@zeux
Copy link
Owner

zeux commented Oct 21, 2021

Some implementation work is in progress, but feedback is certainly welcome on the PR as well!

Yeah I'll try to look at the extension spec more closely over the weekend.

@FreakTheMighty
Copy link

@zeux this is the same issues that we were trying to solve with #530 and relates to this #529.

I believe the issue with supporting the general case for custom attributes was that we don't know the semantics, but if we're looking to support an object ID, perhaps that can be a special case.

@zeux
Copy link
Owner

zeux commented Jun 13, 2023

The PRs you link refer to preserving the existing ID information in the vertices, not generating new ones, no?

I agree that object ID could be a special case even in absence of EXT_mesh_features yeah.

@FreakTheMighty
Copy link

I believe the PR is more general, preserving any custom attribute, but we're using _ID specifically to ID meshes after they have been batched.

I hadn't seen the EXT_mesh_features, but I'm open to using it if its easier to integrate into this library than a more generic handling of custom attributes.

@zeux
Copy link
Owner

zeux commented Jun 14, 2023

Got it. I don't think the PR in question can be merged but it would probably make sense to preserve ID attribute specifically. I was originally hoping for a quicker adoption of EXT_mesh_features but it can be treated orthogonally.

Just to double check, in your case you have an attribute called _ID that contains an integer that uniquely identifies the mesh subset, and the only issue is that gltfpack currently strips the attribute out? Which is to say that the attribute doesn't need to be transformed in any way, it just needs to be preserved?

@FreakTheMighty
Copy link

Correct, it would be an integer per vertex, so if a vertex is collapsed, the corresponding ID should be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants