search

CesiumGS / cesium-unreal

Bringing the 3D geospatial ecosystem to Unreal Engine
https://cesium.com/platform/cesium-for-unreal/
Apache License 2.0
921 stars 295 forks source link

Add option to enforce BuildChaosMeshTriangles for degenerate triangles #1456

Closed arbertrary closed 2 months ago

arbertrary commented 3 months ago

Currently, Cesium discards degenerate triangles in the function BuildChaosMeshTriangles in CesiumGltfComponent.cpp. This causes collision to not work for the discarded triangles.

  for (int32 i = 0; i < triangleCount; ++i) {
    const int32 index0 = 3 * i;
    int32 vIndex0 = indices[index0 + 1];
    int32 vIndex1 = indices[index0];
    int32 vIndex2 = indices[index0 + 2];

    if (!isTriangleDegenerate(
            vertices.X(vIndex0),
            vertices.X(vIndex1),
            vertices.X(vIndex2))) {
      triangles.Add(Chaos::TVector<int32, 3>(vIndex0, vIndex1, vIndex2));
      faceRemap.Add(i);
    }
  }

https://github.com/arbertrary/cesium-unreal/blob/b6b26aa823bdcbcad41cd44c3d7ab2aa86953a74/Source/CesiumRuntime/Private/CesiumGltfComponent.cpp#L3821C1-L3834C4

As described and discussed with @kring in this thread on the Cesium community forums, there might be cases where users would want to enforce that these degenerate triangles are not being discarded.

https://community.cesium.com/t/line-trace-passing-through-cesium3dtileset-on-high-lod/27485

This pull request is a naive approach to adding a boolean option to the Cesium3DTileset actor which a user can activate to enforce this behavior.

Most likely this is not a sensible solution to that issue (dragging that boolean through ~5 different functions seems a bit excessive) but it might serve as a starting point.

kring commented 3 months ago

Thanks for the PR @arbertrary! It's unfortunate that we need this, but I think it's a good stopgap workaround until we can investigate more thoroughly what's going on here. I'm going to make a few changes to avoid threading the bool through and then I'll merge this.

kring commented 3 months ago

Hi @arbertrary I spent some time digging into this problem today, and I think I've found a good solution: https://github.com/CesiumGS/cesium-unreal/pull/1465 So we're going to hold off on adding this new option for tomorrow's release, and hopefully ship a complete fix next release.

arbertrary commented 2 months ago

This pull request is now obsolete due to #1465