-
Notifications
You must be signed in to change notification settings - Fork 327
Description
I noticed a bug in our project code yesterday that used vertex indices to get metadata using UCesiumFeatureIdSetBlueprintLibrary::GetFeatureIDForVertex
: we were using vertex indices from the Unreal mesh, but those don't always match the original glTF model vertex indices, because in some cases there is vertex duplication in CesiumGltfComponent.cpp
's loadPrimitive
:
bool duplicateVertices = needToGenerateFlatNormals || needToGenerateTangents; |
(Note: visiting Unreal mesh vertices was a bad idea as the buffers have no reason to remain in main memory once the render thread has copied them to graphics memory)
Me and @Julot94 checked that possible index mistmatch was not a problem for cesium-unreal blueprint library's functions doing picking, as those are based on Unreal mesh face indices, which match glTF's, and obtain glTF vertex indices directly from there.
But all variants of GetFeatureIDForVertex
as well as UCesiumFeatureIdAttributeBlueprintLibrary::GetFeatureID
are blueprint-accessible, and it made us wonder whether this is really a good idea? Is there a way a user of these functions would be able to supply a glTF vertex index, and not merely an Unreal one, leading to "random" bugs?
So I see several possibilities:
- deprecate or remove these functions from blueprint accessibility, keeping only the equivalent code for their internal usage sites,
- declare that they take Unreal vertex indices as input, and support the vertex duplication case somehow
- or I may simply be wrong in assuming that these functions can take Unreal vertex indices, and there is nothing to do ^^