-
Notifications
You must be signed in to change notification settings - Fork 502
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
Comments
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 |
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 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? Do you also instansiate subelements in a mesh? |
See attached file BabylonJS/Babylon.js#8164 (performance issues have been fixed since)
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.
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. |
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. |
This is possible in principle, although we'd need to define the format that this data is saved in more precisely.
You can get builds from master from GitHub Actions page, e.g. https://github.com/zeux/meshoptimizer/actions/runs/158394020 |
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: 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! |
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. :/ |
@starhopper1 Have you tried using -kn flag? |
Ah yes, perfekt sorry, did not see that modifier... thank you very much! 👍 |
@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 |
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 Look at my attached scene above (Schependomlaan_gltf.zip) 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. |
@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 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. |
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. |
If glTFpack could add an ...Then, a viewer could still do 'picking' based on the vertexID, and rendering would be fast (since '-cc' option reduces draw calls).
|
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 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). |
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! |
Yeah I'll try to look at the extension spec more closely over the weekend. |
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. |
I believe the PR is more general, preserving any custom attribute, but we're using I hadn't seen the |
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 |
Correct, it would be an integer per vertex, so if a vertex is collapsed, the corresponding ID should be removed. |
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.
The text was updated successfully, but these errors were encountered: