From 4f5e3ea8614780ce480808372871fa9865741e32 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Wed, 27 Nov 2024 18:11:00 +0100 Subject: [PATCH] Stylistic changes after code review --- .../Private/CesiumFeatureIdAttribute.cpp | 1 - .../Private/CesiumFeatureIdSet.cpp | 27 ++++++++++--------- Source/CesiumRuntime/Private/LoadGltfResult.h | 4 ++- .../Public/CesiumFeatureIdAttribute.h | 5 ++-- .../CesiumRuntime/Public/CesiumFeatureIdSet.h | 13 +++++---- .../Public/CesiumInstanceFeatures.h | 17 ++++++------ 6 files changed, 37 insertions(+), 30 deletions(-) diff --git a/Source/CesiumRuntime/Private/CesiumFeatureIdAttribute.cpp b/Source/CesiumRuntime/Private/CesiumFeatureIdAttribute.cpp index 2f607e69b..24fc1ebf2 100644 --- a/Source/CesiumRuntime/Private/CesiumFeatureIdAttribute.cpp +++ b/Source/CesiumRuntime/Private/CesiumFeatureIdAttribute.cpp @@ -2,7 +2,6 @@ #include "CesiumFeatureIdAttribute.h" #include -#include #include FCesiumFeatureIdAttribute::FCesiumFeatureIdAttribute( diff --git a/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp b/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp index 9f0416861..16b393923 100644 --- a/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp +++ b/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp @@ -82,7 +82,8 @@ FCesiumFeatureIdSet::FCesiumFeatureIdSet( if (pMetadata && _propertyTableIndex >= 0) { size_t index = static_cast(_propertyTableIndex); if (index < pMetadata->propertyTables.size()) { - const CesiumGltf::PropertyTable& propertyTable = pMetadata->propertyTables[index]; + const CesiumGltf::PropertyTable& propertyTable = + pMetadata->propertyTables[index]; std::string name = propertyTable.name.value_or(""); propertyTableName = FString(name.c_str()); } @@ -217,6 +218,18 @@ int64 UCesiumFeatureIdSetBlueprintLibrary::GetFeatureIDForInstance( int64 UCesiumFeatureIdSetBlueprintLibrary::GetFeatureIDFromHit( UPARAM(ref) const FCesiumFeatureIdSet& FeatureIDSet, const FHitResult& Hit) { + // FeatureIDs from instanced geometry take precedence. + const auto* pInstancedComponent = + Cast(Hit.Component); + if (IsValid(pInstancedComponent)) { + const FCesiumInstanceFeatures& instanceFeatures = + UCesiumInstanceFeaturesBlueprintLibrary::GetInstanceFeatures( + pInstancedComponent); + return UCesiumInstanceFeaturesBlueprintLibrary::GetFeatureIDFromInstance( + instanceFeatures, + Hit.Item); + } + if (FeatureIDSet._featureIDSetType == ECesiumFeatureIdSetType::Texture) { FCesiumFeatureIdTexture texture = std::get(FeatureIDSet._featureID); @@ -225,18 +238,6 @@ int64 UCesiumFeatureIdSetBlueprintLibrary::GetFeatureIDFromHit( Hit); } - { - const auto* pInstancedComponent = - Cast(Hit.Component); - if (IsValid(pInstancedComponent)) { - const FCesiumInstanceFeatures& instanceFeatures = - UCesiumInstanceFeaturesBlueprintLibrary::GetInstanceFeatures( - pInstancedComponent); - return UCesiumInstanceFeaturesBlueprintLibrary::GetFeatureIDFromInstance( - instanceFeatures, - Hit.Item); - } - } // Find the first vertex of the face. const UCesiumGltfPrimitiveComponent* pGltfComponent = Cast(Hit.Component); diff --git a/Source/CesiumRuntime/Private/LoadGltfResult.h b/Source/CesiumRuntime/Private/LoadGltfResult.h index ed14b73f6..b341b8098 100644 --- a/Source/CesiumRuntime/Private/LoadGltfResult.h +++ b/Source/CesiumRuntime/Private/LoadGltfResult.h @@ -196,7 +196,9 @@ struct LoadNodeResult { */ std::vector InstanceTransforms; /** - * Instance features + * Features from EXT_instance_features. A pointer is used for shared ownership + * because there may be multiple primitives in the same mesh belonging to a + * single instance. */ TSharedPtr pInstanceFeatures = nullptr; }; diff --git a/Source/CesiumRuntime/Public/CesiumFeatureIdAttribute.h b/Source/CesiumRuntime/Public/CesiumFeatureIdAttribute.h index 554e0cf15..88b440751 100644 --- a/Source/CesiumRuntime/Public/CesiumFeatureIdAttribute.h +++ b/Source/CesiumRuntime/Public/CesiumFeatureIdAttribute.h @@ -63,10 +63,11 @@ struct CESIUMRUNTIME_API FCesiumFeatureIdAttribute { const FString& PropertyTableName); /** - * @brief Constructs a feature ID attribute instance from instance metadata. + * @brief Constructs a feature ID attribute instance from + * EXT_instance_features data. * * @param Model The model. - * @param Node The "mesh node" containing the feature ID attribute. + * @param Node The node containing the feature ID attribute. * @param FeatureIDAttribute The attribute index specified by the FeatureId. * @param PropertyTableName The name of the property table this attribute * corresponds to, if one exists, for backwards compatibility. diff --git a/Source/CesiumRuntime/Public/CesiumFeatureIdSet.h b/Source/CesiumRuntime/Public/CesiumFeatureIdSet.h index 9586e9a07..3fc4a51bd 100644 --- a/Source/CesiumRuntime/Public/CesiumFeatureIdSet.h +++ b/Source/CesiumRuntime/Public/CesiumFeatureIdSet.h @@ -30,8 +30,9 @@ enum class ECesiumFeatureIdSetType : uint8 { /** * @brief A blueprint-accessible wrapper for a feature ID set from a glTF * primitive. A feature ID can be defined as a per-vertex attribute, as a - * feature texture, or implicitly via vertex ID. These can be used with the - * corresponding {@link FCesiumPropertyTable} to access per-vertex metadata. + * feature texture, implicitly via vertex ID, or associated with glTF + * instances. These can be used with the corresponding {@link + * FCesiumPropertyTable} to access the metadata. */ USTRUCT(BlueprintType) struct CESIUMRUNTIME_API FCesiumFeatureIdSet { @@ -189,12 +190,14 @@ class CESIUMRUNTIME_API UCesiumFeatureIdSetBlueprintLibrary int64 VertexIndex); /** - * Gets the feature ID associated with a given instance. The feature ID can be - * used with a FCesiumPropertyTable to retrieve the corresponding metadata. + * Gets the feature ID associated with a given instance in glTF models using + * the EXT_mesh_gpu_instancing and EXT_instance_features extensions. The + * feature ID can be used with a FCesiumPropertyTable to retrieve the + * corresponding metadata. * * This returns -1 if the given instance is out-of-bounds, if the feature ID * set is not for instances, or if the feature ID set is invalid (e.g., it - * contains an invalid feature ID texture). + * contains an invalid feature ID attribute). */ UFUNCTION( BlueprintCallable, diff --git a/Source/CesiumRuntime/Public/CesiumInstanceFeatures.h b/Source/CesiumRuntime/Public/CesiumInstanceFeatures.h index 353251f54..df07dd0c8 100644 --- a/Source/CesiumRuntime/Public/CesiumInstanceFeatures.h +++ b/Source/CesiumRuntime/Public/CesiumInstanceFeatures.h @@ -23,15 +23,15 @@ struct CESIUMRUNTIME_API FCesiumInstanceFeatures { public: /** - * Constructs an empty primitive features instance. + * Constructs an empty instance features object. */ FCesiumInstanceFeatures() : _instanceCount(0) {} /** - * Constructs an instance feature. + * Constructs an instance feature object. * * @param Model The model that contains the EXT_instance_features extension - * @param Node The node that stores instance data + * @param Node The node that stores EXT_instance_features * extension */ FCesiumInstanceFeatures( @@ -52,8 +52,9 @@ class CESIUMRUNTIME_API UCesiumInstanceFeaturesBlueprintLibrary public: /** - * Gets the instance features of a glTF primitive component. If component is - * not a Cesium glTF primitive component, the returned features are empty. + * Gets the instance features of an instanced glTF primitive component. If + * component is not an instanced Cesium glTF component, the returned features + * are empty. */ UFUNCTION( BlueprintCallable, @@ -63,8 +64,8 @@ class CESIUMRUNTIME_API UCesiumInstanceFeaturesBlueprintLibrary GetInstanceFeatures(const UPrimitiveComponent* component); /** - * Gets all the feature ID sets that are associated with the - * primitive. + * Gets all the feature ID sets that are associated with the features + * associated with an instanced primitive. */ UFUNCTION( BlueprintCallable, @@ -86,7 +87,7 @@ class CESIUMRUNTIME_API UCesiumInstanceFeaturesBlueprintLibrary ECesiumFeatureIdSetType Type); /** - * Gets the feature ID associated with an instance number + * Gets the feature ID associated with an instance at an index. * */ UFUNCTION(