tileset.modelMatrix not working for .i3dm with ModelExperimental enabled

[j9liu]

Found this while testing for #10250. With ExperimentalFeatures.enableModelExperimental = true, the instanced model doesn't show up at the correct position, instead remaining on the ground. The PR itself did not introduce it; I tested on main and the same behavior occurs.

Here's a sandcastle to demonstrate.

[ptrgags]

Not sure if it's the same problem, but I noticed in this sandcastle that the i3dm models seem to be oriented incorrectly:

The trees are knocked down:

I need to check if the bridge is an i3dm, but it also seems to be oriented incorrectly:

[ptrgags]

Some more details:

• I did a git bisect, it pointed me to this commit. It seems like when we refactored the model matrices to support model size, we didn't take .i3dm into account
• Looking back at the instancing code, I'm finding it rather confusing. i3dm handles world space and the RTC transform differently than EXT_mesh_gpu_instancing so there's a separate shader code path (see LegacyInstancingStageVS.glsl). I wonder how much overlap there is. If needed, we could split InstancingPipelineStage into two if there's not much overlap.
• I notice that the old Instanced3DModel3DTileContent computes a model matrix with the RTC offset baked in before passing it to ModelInstancingCollection. The instancing stage doesn't include this offset in the matrix passed to the shader
• I also notice that InstancingPipelineStage has a u_instance_nodeTransform matrix which includes handling the axis correction. I wonder if this matrix isn't quite correct with the other matrix changes

[ptrgags]

Interesting, if I set u_instance_nodeTransform() to return Matrix4.IDENTITY things look correct.

[ptrgags]

Not quite... it doesn't seem to make a difference for the cubes example. Sounds like this will require some more digging to better understand the matrices in the instancing code.

[lilleyse]

These are the expected orders of transformations

i3dm: computed tile transform * RTC center * instance transform * model matrix * y-up-to-z-up * glTF node hierarchy * vertex
gltf: computed tile transform * model matrix * y-up-to-z-up * glTF node hierarchy * instance transform * vertex

(Computed tile transform includes the tileset's model matrix and the tile transforms)

[ptrgags]

I'm starting to understand this in bits and pieces.

I've been taking a closer look at the above equations and trying to translate them to the current code's variables. Let me try to organize my thoughts so far:

Tile = "computed tile transform" = tile.computedTransform
Rtc = "RTC Center" = components.transform (poorly named! this is only ever used for RTC)
Instance = "instance transform" = TRS from instancing parameters = getInstancingTransform() in shader stage
Model = "model matrix" = sceneGraph.modelMatrix
Zup = "y-up-to-z-up" = applied via ModelExperimentalUtility.correctModelMatrix()
Node = "gltfNode hierarchy" = runtimeNode.transformToRoot * runtimeNode.transform

Let me re-write the desired transforms a little bit:

let Local = Model * Zup * Node // local transform for this node

i3dmModel = Tile * Rtc * Instance * Local
gltfModel = Tile * Rtc=Identity * Local * Instance

Introducing Identity to take the place of the Rtc transform, then it's more obvious that the only difference is where to apply the Instance transform.

That said, there's some gotchas here:

• in the glTF case, Tile * Rtc * Local can be used directly as drawCommand.modelMatrix which is used in czm_modelView However, in the i3dm case, you have to keep Local separate since Instance comes from an attribute and must be handled in the shader. This is why the old code had a legacy instancing stage.
• I'm noticing that for 3D Tiles we pass Tile to model.modelMatrix, which means Tile = Model. It might be necessary to make more of a distinction in the code.

Some other potential problems in the current code:

• I think due to the confusing name of components.transform, I thought this was applied like Model * components.transform, but really it should go on the left.
• runtimeNode.transform's meaning changed recently, so now u_instance_nodeTransform = runtimeNode.transform * Zup is only using a single node's matrix, not the whole chain to the root node. Plus it's applying Zup incorrectly according to the previous comment.

I have to think a bit about the cleanest way to fix these problems.

[ptrgags]

Hm, looking at the old code for 3D Tiles 1.0 formats, it was a bit different:

• for b3dm, the content passes in Model = Tile * Rtc.
• for i3dm, ModelInstanceCollection.modelMatrix = Tile and then for a specific instance, Model = Rtc * Instance. So I think the equations become:

let Components = components.transform
let Model = Tile * Components
let Local = Zup * Node // local transform for this node

i3dmModel = Model * Instance * Local
gltfModel = Model * Local * Instance

Furthermore, I found at least one case where components.transform is used for more than the RTC transform (see PntsLoader.js) so I'll start with just documenting the property better.

[ptrgags]

Update: I tried an experiment:

uniformMap.u_instance_nodeTransform = function () {
  return Matrix4.multiply(
    renderResources.runtimeNode.transformToRoot, 
    renderResources.runtimeNode.axisCorrectedTransform, 
    new Matrix4());
};

and that seems to fix things. What was happening is the glTF root node had a z-up-to-y-up transform that was getting ignored, so the axis correction was not being canceled out properly.

This still doesn't address the tileset.modelMatrix problem with the cubes example. That's probably something else... I'm surprised, since that should be folded into the computed tile transform.

[sanjeetsuhag]

Regarding the tileset.modelMatrix issue, my understanding of the non-I3DM path is that when we change the model matrix of the model (by multiplying the tileset/tile.modelMatrix), the change gets applied to the model matrix that is attached to the DrawCommand. That is then set to the czm_model/czm_modelView in UniformState and accessed in the shader. Since the legacy instancing path uses, u_instance_modifiedModelView which is based on sceneGraph.components.transform - if that remains unchanged, then any change to the model matrix will not have an effect. So, perhaps the thing to try would be to make sure the world space transformation of the model is being propagated to u_instance_modifiedModelView.