CesiumGS / cesium-unity

Bringing the 3D geospatial ecosystem to Unity
https://cesium.com/platform/cesium-for-unity/
Apache License 2.0
358 stars 83 forks source link

Update feature ID texture for `KHR_texture_transform` (plus bugfix!) #428

Closed j9liu closed 7 months ago

j9liu commented 8 months ago

Depends on https://github.com/CesiumGS/cesium-native/pull/827.

Unity counterpart of https://github.com/CesiumGS/cesium-unreal/pull/1360. This incorporates the previous cesium-native changes so that KHR_texture_transform can be used while picking feature ID textures.

Test Picking

  1. Create a dummy tileset containing the feature ID texture transform model here.
  2. In the 05_CesiumMetadata sample level, add a new blank tileset. Add the aforementioned tileset via URL.
  3. Set the ECEF coordinates of the CesiumGeoreference to (0, 0, 0). You should be able to double-click the tileset to zoom to it, but if that doesn't work properly, enable Show Tiles In Hierarchy and double-click the child game object that contains the mesh.
  4. Reset the DynamicCamera transform. So position should be (0, 0, 0). Rotation should also be (0, 0, 0). (I've noticed the Unity editor complains about NaNs here, maybe I should write an issue about that...)
  5. Offset the DynamicCamera from (0, 0, 0), along the Z axis. You want to position it so it can turn and view the feature ID texture at runtime.
  6. Modify CesiumSamplesMetadataPicking.cs to Debug.Log the feature ID resulting from GetFeatureIdFromHit. Also, be sure that you remove the check for modelMetadata != null b/c this tileset doesn't use metadata.
  7. Press Play. If all works out, you'll be able to move the camera (with some effort) to click on the feature ID texture. The darkest square returns a value of 54, and the brightest square corresponds to 135.

When a value is repeatedly logged, Unity will consolidate the notifications to a single entry in the console. So sometimes you won't see a feature ID at the bottom of the console if you re-pick it, because it's being counted under an earlier entry. Just clear the console so you can see it pop up again.

Bonus Bugfix

While implementing this, I discovered a bug with feature ID picking that resulted from an inconsistency between Unity and the glTF model. In particular, the RaycastHit.barycentricCoordinate returned by Unity were completely different (in magnitude) from the barycentric coordinates returned in Unreal Engine. For the darkest square in the texture, the barycentric coordinates would largest in the 2nd vertex for Unity, whereas in Unreal the 3rd vertex had the biggest coordinate. I'm not 100% sure why this happens -- maybe it has to do with the difference in the way Unity handles mesh data (e.g. clockwise vs. counter-clockwise winding order).

In any case, I couldn't find a failproof way to swizzle the coordinates while maintaining correctness for every case, so I just implemented the barycentric coordinate logic in cesium-native, and use that in Unity. Now, feature ID picking should be accurate.

j9liu commented 8 months ago

Hm I forgot to mention that I was experiencing a Reinterop error that I manually fixed. It was in Matrix4x4.cpp, the MultiplyPoint3x4 method has a &(*this) line that should actually be *this.

kring commented 7 months ago

@j9liu I pushed a fix for the Reinterop problem.

kring commented 7 months ago

I was able to follow your procedure above, and it worked as described. I verified the expected feature IDs against the model. And I looked through the code, and it all makes sense to me. So it looks great, merging!