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

I3dm to glb upgrade - tree not visible #153

Open
bertt opened this issue Oct 23, 2024 · 7 comments
Open

I3dm to glb upgrade - tree not visible #153

bertt opened this issue Oct 23, 2024 · 7 comments

Comments

@bertt
Copy link

bertt commented Oct 23, 2024

Hi,

I'm trying to upgrade a 3D Tileset with 1 I3dm (1 instance on Dam square Amsterdam) to 3D Tiles 1.1 GLB.

Upgrade command used:

npx  3d-tiles-tools upgrade -i tileset.json -o ../glb -f --targetVersion 1.1

Version 3d-tiles-tools (npx 3d-tiles-tools --version): unknown (but installed with npm install 3d-tiles-tools)

With a simple box it works ok:

But with a tree model (https://bertt.github.io/cesium_issues/instance_upgrade/tree/tree.glb) the tree doesn't show up:

Only difference is in the used model. Any ideas how to show the tree in glb format after upgrade?

@bertt
Copy link
Author

bertt commented Oct 23, 2024

Notice: When inspecting the resulting tree glb, there are 2 trees instead of 1...

image

@javagl
Copy link
Contributor

javagl commented Oct 23, 2024

Just a quick note: It looks like the I3DM (which works) also contains two trees:

Cesium I3DM to GLB

(nearly at the same position, though).

I'll have a closer look and probably start by logging the positions/TRS that come from the I3DM and go into the GLB

@bertt
Copy link
Author

bertt commented Oct 23, 2024

ah yes well spotted, I've updated the tree samples (result tree glb has now 1 tree, but it doesn't show up in Cesium)

@javagl
Copy link
Contributor

javagl commented Oct 23, 2024

Found'em 😑

Cesium I3DM to GLB there they are!

Note that the trees (at the cursor) are floating somewhere below the ground, and it's difficult to navigate to make them visible (the bounding box has to remain visible as well, otherwise, they'll be clipped away...)

Logging did first not bring toooo many insights: The main difference between the trees is a slight difference in the rotation. Their world positions, as they are coming from the I3DM, are basically equal:

worldPositions [
  [ 3887940.5, 332858.03, 5028256 ],
  [ 3887940.5, 332858.03, 5028256 ]
]

What may be relevant here: These positions do include the RTC_CENTER from the I3DM:

  "RTC_CENTER" : [
    3887940.5,
    332858.03,
    5028256
  ]

And for the reasons laid out in Precisions, Precisions: Using such large values for rendering is difficult. Specifically: Writing such values into the TRANSLATION accessor for EXT_mesh_gpu_instancing does not make sense. Doing so would cause distinct instances to "collapse" to the same position, and/or it would cause the well-known "jittering"...

(This is the reason why the RTC_CENTER was introduced to begin with...)

What the I3DM-to-GLB upgrade is doing in an attempt to handle that: Is tries to generically "relativize" the positions/translations. Specifically, referring to this part of the code:

  • It is computing the worldPositions of the instances (these positions do include the RTC_CENTER offset)
  • It is computing the positionsCenter (just the center of gravity of these)
  • It is computing the translations to be relative to this positionsCenter, and using them to build the EXT_mesh_gpu_instancing extension
  • Eventually, the positionCenter is set as the translation of the root node

(This is actually similar to what CesiumJS is doing for I3DM internally)

Now there are basically two options:

  1. It could be a precision issue.
    This seems unlikely. Any precision error should be (unrecognizably) small, given that all the computations are happening with 64bit float.
  2. Something could be wrong with the computation itself
    This also seems unlikely. The computation is relatively straightforward. The specs do include the InstancedRTC spec from CesiumJS, and this is converted and renderered properly, despite containing the RTC_CENTER. The translation of the root note that is eventually written into the glTF is "translation" : [ 3887940.5, 5028256, -332858.03 ], which is exactly the former RTC_CENTER.

So.... right now, I have no idea why the position of the model may be so wrong... 😕

@javagl
Copy link
Contributor

javagl commented Oct 24, 2024

(Some sort of rubber-duck-debugging here - but also the keep track of some thoughs:)

The TRS properties for the EXT_mesh_gpu_instancing extension are computed as follows:

  • compute a matrix from the TRS propeties of the I3DM (!)
  • convert this matrix into the glTF coordinate system,
  • decompose that back into TRS - and the result of this decomposition will be used to fill the accessors

This inevietably will introduce small errors. I wondered whether even small rounding errors could be emphasized by the node translation that corresponds to the RTC_CENTER. But even when filling the accessors with all identity values, the position seems waaay off. It looks like the root node translation is "wrong", even though it matches the RTC_CENTER, down to the last digit...

@bertt
Copy link
Author

bertt commented Oct 29, 2024

one thing that's special about this tree model: it has a rotation (1/4 pi) in the root node. But I don't see it handled in applyRtcCenter (there a new node is created, the new node gets the translation and the old node is added as a child to the new node).
Well it doesn't explain to me the big translation, but might be a pointer.

@javagl
Copy link
Contributor

javagl commented Oct 30, 2024

Yeah, on the one hand, that tree model is haunting me, due to all the issues (this one, disappearing I3DM parts...), and I think that it "originated" from me casually throwing that into https://github.com/CesiumGS/3d-tiles-samples/tree/main/1.1/MetadataGranularities so I somehow "regret" that. But ... I think that it is good to have a model that "breaks" things, to point out flaws, and increase the robustness.

The issues (both the disappearing I3DM parts, as well as this one) are likely, somehow rooted in the specific node transforms that the model is using. For the disappearing I3DM parts, this is undoubtedly the case (we already figured that out, and drafted solutions). But for the case of the upgrade, any node transform should already be removed. Specifically: The part at

// Flatten all nodes in the glTF asset. This will collapse
should "bake" all transforms into the mesh, and the resulting glTF should not have any node transforms any more. (Maybe some precision issue is happening there...? I have not yet investigated this much further...)

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

No branches or pull requests

2 participants