From 8db3e82057e99f2ea12d0fd2f4b17077bde6ec82 Mon Sep 17 00:00:00 2001 From: Tim Moore Date: Mon, 16 Dec 2024 17:25:28 +0100 Subject: [PATCH] Incorporate code review suggestions Highlights: Use the default move constructor instead of defining it. Simplify FCesiumPrimitiveFeatures constructor for instances by passing in the Gltf::ExtensionExtInstanceFeatures object. --- .../Private/CesiumFeatureIdSet.cpp | 10 +--------- .../Private/CesiumGltfComponent.cpp | 4 ++-- .../Private/CesiumPrimitiveFeatures.cpp | 15 ++++----------- Source/CesiumRuntime/Private/LoadGltfResult.h | 18 +++--------------- .../Public/CesiumPrimitiveFeatures.h | 9 ++++++--- 5 files changed, 16 insertions(+), 40 deletions(-) diff --git a/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp b/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp index 0a7d19edc..36a45ce70 100644 --- a/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp +++ b/Source/CesiumRuntime/Private/CesiumFeatureIdSet.cpp @@ -78,7 +78,7 @@ FCesiumFeatureIdSet::FCesiumFeatureIdSet( // For backwards compatibility with GetFeatureTableName. const CesiumGltf::ExtensionModelExtStructuralMetadata* pMetadata = InModel.getExtension(); - if (pMetadata && _propertyTableIndex >= 0) { + if (pMetadata && this->_propertyTableIndex >= 0) { size_t index = static_cast(_propertyTableIndex); if (index < pMetadata->propertyTables.size()) { const CesiumGltf::PropertyTable& propertyTable = @@ -174,14 +174,6 @@ int64 UCesiumFeatureIdSetBlueprintLibrary::GetFeatureIDForVertex( return -1; } -namespace { -struct FeatureIdFromAccessorView : public CesiumGltf::FeatureIdFromAccessor { - FeatureIdFromAccessorView(int64_t index_) { index = index_; } - - int64_t operator()(std::monostate) { return -1; } -}; -} // namespace - int64 UCesiumFeatureIdSetBlueprintLibrary::GetFeatureIDForInstance( UPARAM(ref) const FCesiumFeatureIdSet& FeatureIDSet, int64 InstanceIndex) { diff --git a/Source/CesiumRuntime/Private/CesiumGltfComponent.cpp b/Source/CesiumRuntime/Private/CesiumGltfComponent.cpp index 401986cbc..e2c2624c3 100644 --- a/Source/CesiumRuntime/Private/CesiumGltfComponent.cpp +++ b/Source/CesiumRuntime/Private/CesiumGltfComponent.cpp @@ -2039,7 +2039,7 @@ static void loadInstancingData( } if (pInstanceFeatures) { result.pInstanceFeatures = - MakeShared(model, node); + MakeShared(model, node, *pInstanceFeatures); } } @@ -2892,7 +2892,7 @@ static void loadPrimitiveGameThreadPart( bool createNavCollision, ACesium3DTileset* pTilesetActor, const std::vector& instanceTransforms, - TSharedPtr pInstanceFeatures) { + const TSharedPtr& pInstanceFeatures) { TRACE_CPUPROFILER_EVENT_SCOPE(Cesium::LoadPrimitive) #if DEBUG_GLTF_ASSET_NAMES diff --git a/Source/CesiumRuntime/Private/CesiumPrimitiveFeatures.cpp b/Source/CesiumRuntime/Private/CesiumPrimitiveFeatures.cpp index 5fd05b645..0abb7a933 100644 --- a/Source/CesiumRuntime/Private/CesiumPrimitiveFeatures.cpp +++ b/Source/CesiumRuntime/Private/CesiumPrimitiveFeatures.cpp @@ -30,21 +30,14 @@ FCesiumPrimitiveFeatures::FCesiumPrimitiveFeatures( FCesiumPrimitiveFeatures::FCesiumPrimitiveFeatures( const CesiumGltf::Model& Model, - const CesiumGltf::Node& Node) + const CesiumGltf::Node& Node, + const CesiumGltf::ExtensionExtInstanceFeatures& InstanceFeatures) : _vertexCount(0), _primitiveMode(-1) { if (Node.mesh < 0 || Node.mesh >= Model.meshes.size()) { return; } - const auto* pInstancing = - Node.getExtension(); - if (!pInstancing) { - return; - } - if (const auto* pInstanceFeatures = - Node.getExtension()) { - for (const auto& featureId : pInstanceFeatures->featureIds) { - _featureIdSets.Emplace(Model, Node, featureId); - } + for (const auto& featureId : InstanceFeatures.featureIds) { + this->_featureIdSets.Emplace(Model, Node, featureId); } } diff --git a/Source/CesiumRuntime/Private/LoadGltfResult.h b/Source/CesiumRuntime/Private/LoadGltfResult.h index 5b25b1820..6d01fe2db 100644 --- a/Source/CesiumRuntime/Private/LoadGltfResult.h +++ b/Source/CesiumRuntime/Private/LoadGltfResult.h @@ -162,15 +162,8 @@ struct LoadMeshResult { LoadMeshResult() {} LoadMeshResult(const LoadMeshResult&) = delete; - - LoadMeshResult(LoadMeshResult&& other) { - primitiveResults.swap(other.primitiveResults); - } - - LoadMeshResult& operator=(LoadMeshResult&& other) { - primitiveResults.swap(other.primitiveResults); - return *this; - } + LoadMeshResult(LoadMeshResult&& other) = default; + LoadMeshResult& operator=(LoadMeshResult&& other) = default; std::vector primitiveResults{}; }; @@ -182,12 +175,7 @@ struct LoadNodeResult { LoadNodeResult() {} LoadNodeResult(const LoadNodeResult&) = delete; - - LoadNodeResult(LoadNodeResult&& other) - : pInstanceFeatures(std::move(other.pInstanceFeatures)) { - InstanceTransforms.swap(other.InstanceTransforms); - meshResult.swap(other.meshResult); - } + LoadNodeResult(LoadNodeResult&& other) = default; std::optional meshResult = std::nullopt; /** diff --git a/Source/CesiumRuntime/Public/CesiumPrimitiveFeatures.h b/Source/CesiumRuntime/Public/CesiumPrimitiveFeatures.h index ef5e1325f..ed4be1560 100644 --- a/Source/CesiumRuntime/Public/CesiumPrimitiveFeatures.h +++ b/Source/CesiumRuntime/Public/CesiumPrimitiveFeatures.h @@ -11,6 +11,7 @@ namespace CesiumGltf { struct ExtensionExtMeshFeatures; +struct ExtensionExtInstanceFeatures; } // namespace CesiumGltf /** @@ -36,8 +37,7 @@ struct CESIUMRUNTIME_API FCesiumPrimitiveFeatures { * @param Model The model that contains the EXT_mesh_features extension * @param Primitive The mesh primitive that stores EXT_mesh_features * extension - * @param Features The EXT_mesh_features of the glTF mesh primitive. - * primitive + * @param Features The EXT_mesh_features of the glTF mesh primitive */ FCesiumPrimitiveFeatures( const CesiumGltf::Model& Model, @@ -50,10 +50,13 @@ struct CESIUMRUNTIME_API FCesiumPrimitiveFeatures { * @param Model The model that contains the EXT_instance_features extension * @param Node The node that stores EXT_instance_features * extension + * @param InstanceFeatures The EXT_Instance_features of the glTF mesh + * primitive */ FCesiumPrimitiveFeatures( const CesiumGltf::Model& Model, - const CesiumGltf::Node& Node); + const CesiumGltf::Node& Node, + const CesiumGltf::ExtensionExtInstanceFeatures& InstanceFeatures); private: TArray _featureIdSets;