Blocks don't correctly apply their instance transforms when exported

[jrz371]

https://discourse.mcneel.com/t/block-location-was-incorrect-after-convert-to-gltf/147856

[visose]

Hi, this can be fixed by changing this line:

glTF-Bin/glTF-BinExporter/RhinoDocGltfConverter.cs

Line 342 in 462af26

ExplodeRecursive(instanceObject, Rhino.Geometry.Transform.Identity, pieces, transforms);

to

ExplodeRecursive(instanceObject, instanceObject.InstanceXform, pieces, transforms);

But ideally it would be great if it supported instancing, to not duplicate the geometry of the blocks in the glTF file.

[visose]

Instancing support can be added without making any large changes using a dictionary similar to how you reuse materials, with the rhino object's id as the key and the mesh index returned by RhinoMeshGltfConverter as value. This way you can reuse the mesh buffers on different objects.

I added this to my fork here:
https://github.com/visose/glTF-Bin/blob/7afaace0f5cd1ab043fb7ec013a99e25d43984af/glTF-BinExporter/RhinoDocGltfConverter.cs#L112

[jrz371]

This issue has been fixed in the glTF-IO project
https://github.com/mcneel/glTF-IO
That project is a fork of this that will be shipping with Rhino 8. It also has a few other improvements (such as point cloud IO and soon [hopefully] support for Draco compression on the point clouds). I agree that the lack of instancing support is kind of lame. I think it might be a bit more complicated than just storing the object meshes in a dictionary. With nested blocks the transforms will need to be correct, and the nested structure should probably also be represented in the nodes. If you want, it'd be great if you cherry picked your commits and opened a PR there. Then I can base further work on that. Otherwise I'll open an issue in YouTrack and get around to it when I can.

I need to update the README here saying primary development is occurring over on McNeels fork

[visose]

Thanks, looks like you moved there recently, after I forked this repo.

Instancing would indeed need to be more complicated if you want to preserve the nested block structure. In our case we just need the size reduction and performance of reusing buffers for the same objects. So this method is like if you exploded the blocks but then for each unique object you created a block that contains only that object.
In the code you are already iterating over the transforms on the nested blocks, what I've done is rather than transform the meshes, just add the transforms to the Matrix property of the gltf Node object.

Not having all roots at the origin should also help improve the quantization step of compression (as long as the block geometry wasn't accidentally created far away from the origin and the instances are moving it closer).

[visose]

I'll see if I can cherry pick commits, the main problem is that I have made changes you don't want in the project (see #81), so have to be careful not to commit those.

Another thing, not very important but, found the code doesn't follow consistent formatting style (for example, some if statements have a space before the parenthesis and some don't) and the default auto-formatting changes too many lines that wouldn't want to include in a commit.
It would be great if you included an .editorconfig file and did one commit where you formatted the entire project.

[visose]

Btw, I also changed it so that the geometry is always stored in meters, no matter the units used in the Rhino document. This is to follow the gltf spec, but also has other advantages if you can assume the gltf files are always in meters.

[visose]

The repo at the McNeel org doesn't have issues enabled, would be great if it did.

I'd like to comment why the code for glTFLoader has been directly included in the repo. They publish nuget packages so ideally it should be included as a package (not with git submodules either as this project does).

[jrz371]

Yes, the formatting is an issue, not just with this but across the board at McNeel. I've been fixing the indenting as I modify the files but I'm usually pretty hesitant to blow out all the git history in one go.

The meters suggestion is a good point. I should probably add that as a export since some use cases may want something modeled small to appear large.

I enabled issues in glTF-IO repository.

If you open an issue in glTF-IO about the glTFLoader I can explain the choice there. One improvement is to make the glTF-IO have two sets of projects. The first configured for the Rhino build and the second configured so users can build and debug.

[londos]

Instancing support can be added without making any large changes using a dictionary similar to how you reuse materials, with the rhino object's id as the key and the mesh index returned by RhinoMeshGltfConverter as value. This way you can reuse the mesh buffers on different objects.

I added this to my fork here: https://github.com/visose/glTF-Bin/blob/7afaace0f5cd1ab043fb7ec013a99e25d43984af/glTF-BinExporter/RhinoDocGltfConverter.cs#L112

@visose Is your fork still shareable? I was also expecting block instances to come through as node instances of a single mesh.