From 43ee7f02f808a6d9a1becf9c4fae1c0fe068c9b2 Mon Sep 17 00:00:00 2001 From: John Turner <7strbass@gmail.com> Date: Mon, 4 May 2020 12:43:14 -0400 Subject: [PATCH 1/2] --Renamed GlftMeshData -> GenericMeshData; GlftMeshData class functionality applies to other types of files, as well as Magnum primtives, and so is more appropriately considered generic mesh data. --- src/esp/assets/BaseMesh.h | 8 ++--- src/esp/assets/CMakeLists.txt | 4 +-- .../{GltfMeshData.cpp => GenericMeshData.cpp} | 36 +++++++++++-------- .../{GltfMeshData.h => GenericMeshData.h} | 26 +++++++++----- src/esp/assets/ResourceManager.cpp | 20 +++++------ src/esp/assets/ResourceManager.h | 13 ++++--- src/esp/core/RigidState.h | 4 +-- 7 files changed, 63 insertions(+), 48 deletions(-) rename src/esp/assets/{GltfMeshData.cpp => GenericMeshData.cpp} (66%) rename src/esp/assets/{GltfMeshData.h => GenericMeshData.h} (74%) diff --git a/src/esp/assets/BaseMesh.h b/src/esp/assets/BaseMesh.h index ad6351a88c..82d6dfd8ac 100644 --- a/src/esp/assets/BaseMesh.h +++ b/src/esp/assets/BaseMesh.h @@ -55,10 +55,10 @@ enum SupportedMeshType { PTEX_MESH = 1, /** - * Meshes loaded from gltf format (i.e. .glb file). Object is likely - * type @ref GltfMeshData. + * Meshes loaded from gltf format (i.e. .glb file), or instances of Magnum + * Primitives. Object is likely type @ref GenericMeshData. */ - GLTF_MESH = 2, + GENERIC_MESH = 2, /** * Number of enumerated supported types. @@ -167,7 +167,7 @@ class BaseMesh { /** * @brief Optional storage container for mesh render data. * - * See @ref GltfMeshData::setMeshData. + * See @ref GenericMeshData::setMeshData. */ Corrade::Containers::Optional meshData_ = Corrade::Containers::NullOpt; diff --git a/src/esp/assets/CMakeLists.txt b/src/esp/assets/CMakeLists.txt index 64e8f7983c..7428990cd0 100644 --- a/src/esp/assets/CMakeLists.txt +++ b/src/esp/assets/CMakeLists.txt @@ -8,8 +8,8 @@ set(assets_SOURCES CollisionMeshData.h GenericInstanceMeshData.cpp GenericInstanceMeshData.h - GltfMeshData.cpp - GltfMeshData.h + GenericMeshData.cpp + GenericMeshData.h MeshData.h MeshMetaData.h Mp3dInstanceMeshData.cpp diff --git a/src/esp/assets/GltfMeshData.cpp b/src/esp/assets/GenericMeshData.cpp similarity index 66% rename from src/esp/assets/GltfMeshData.cpp rename to src/esp/assets/GenericMeshData.cpp index 1f8423916c..b6dc0734c2 100644 --- a/src/esp/assets/GltfMeshData.cpp +++ b/src/esp/assets/GenericMeshData.cpp @@ -2,20 +2,20 @@ // This source code is licensed under the MIT license found in the // LICENSE file in the root directory of this source tree. -#include "GltfMeshData.h" +#include "GenericMeshData.h" #include #include +#include #include #include - namespace Cr = Corrade; namespace Mn = Magnum; namespace esp { namespace assets { -void GltfMeshData::uploadBuffersToGPU(bool forceReload) { +void GenericMeshData::uploadBuffersToGPU(bool forceReload) { if (forceReload) { buffersOnGPU_ = false; } @@ -24,7 +24,7 @@ void GltfMeshData::uploadBuffersToGPU(bool forceReload) { } renderingBuffer_.reset(); - renderingBuffer_ = std::make_unique(); + renderingBuffer_ = std::make_unique(); Magnum::MeshTools::CompileFlags compileFlags{}; if (needsNormals_ && !meshData_->hasAttribute(Mn::Trade::MeshAttribute::Normal)) { @@ -36,7 +36,7 @@ void GltfMeshData::uploadBuffersToGPU(bool forceReload) { buffersOnGPU_ = true; } -Magnum::GL::Mesh* GltfMeshData::getMagnumGLMesh() { +Magnum::GL::Mesh* GenericMeshData::getMagnumGLMesh() { if (renderingBuffer_ == nullptr) { return nullptr; } @@ -44,18 +44,18 @@ Magnum::GL::Mesh* GltfMeshData::getMagnumGLMesh() { return &(renderingBuffer_->mesh); } -void GltfMeshData::setMeshData(Magnum::Trade::AbstractImporter& importer, - int meshID) { - ASSERT(0 <= meshID && meshID < importer.meshCount()); +void GenericMeshData::setMeshData(Magnum::Trade::AbstractImporter& importer, + int meshID) { + /* Guarantee mesh instance success */ + CORRADE_INTERNAL_ASSERT_OUTPUT(meshData_ = importer.mesh(meshID)); /* Interleave the mesh, if not already. This makes the GPU happier (better cache locality for vertex fetching) and is a no-op if the source data is already interleaved, so doesn't hurt to have it there always. */ - Cr::Containers::Optional meshData = - importer.mesh(meshID); - if (meshData) - meshData_ = Mn::MeshTools::interleave(*std::move(meshData)); - else - meshData_ = Cr::Containers::NullOpt; + + CORRADE_ASSERT( + meshData_->primitive() == Mn::MeshPrimitive::Triangles, + "Cannot instance collisionMeshData_; Triangle Mesh expected.", ); + meshData_ = Mn::MeshTools::interleave(*std::move(meshData_)); collisionMeshData_.primitive = Magnum::MeshPrimitive::Triangles; @@ -74,5 +74,13 @@ void GltfMeshData::setMeshData(Magnum::Trade::AbstractImporter& importer, collisionMeshData_.indices = indexData_ = meshData_->indicesAsArray(); } +void GenericMeshData::setMeshData(Magnum::Trade::AbstractImporter& importer, + const std::string& meshName) { + // make sure name is appropriate for importer + int meshID = importer.meshForName(meshName); + CORRADE_ASSERT(meshID != -1, "Unknown meshName : " << meshName, ); + setMeshData(importer, meshID); +} + } // namespace assets } // namespace esp diff --git a/src/esp/assets/GltfMeshData.h b/src/esp/assets/GenericMeshData.h similarity index 74% rename from src/esp/assets/GltfMeshData.h rename to src/esp/assets/GenericMeshData.h index f1f69edd15..cbd0c2cd6c 100644 --- a/src/esp/assets/GltfMeshData.h +++ b/src/esp/assets/GenericMeshData.h @@ -5,8 +5,8 @@ #pragma once /** @file - * @brief Class @ref esp::assets::GltfMeshData, Class @ref - * esp::assets::GltfMeshData::RenderingBuffer + * @brief Class @ref esp::assets::GenericMeshData, Class @ref + * esp::assets::GenericMeshData::RenderingBuffer */ #include @@ -23,7 +23,7 @@ namespace assets { * @brief Mesh data storage and loading for gltf format assets. See @ref * ResourceManager::loadGeneralMeshData. */ -class GltfMeshData : public BaseMesh { +class GenericMeshData : public BaseMesh { public: /** * @brief Stores render data for the mesh necessary for gltf format. @@ -35,13 +35,14 @@ class GltfMeshData : public BaseMesh { Magnum::GL::Mesh mesh; }; - /** @brief Constructor. Sets @ref SupportedMeshType::GLTF_MESH to identify the - * asset type.*/ - GltfMeshData(bool needsNormals = true) - : BaseMesh(SupportedMeshType::GLTF_MESH), needsNormals_{needsNormals} {}; + /** @brief Constructor. Sets @ref SupportedMeshType::GENERIC_MESH to identify + * the asset type.*/ + GenericMeshData(bool needsNormals = true) + : BaseMesh(SupportedMeshType::GENERIC_MESH), + needsNormals_{needsNormals} {}; /** @brief Destructor */ - virtual ~GltfMeshData(){}; + virtual ~GenericMeshData(){}; /** * @brief Compile the @ref renderingBuffer_ if first upload or forceReload is @@ -59,6 +60,15 @@ class GltfMeshData : public BaseMesh { * asset. */ void setMeshData(Magnum::Trade::AbstractImporter& importer, int meshID); + /** + * @brief Load mesh data from a pre-parsed importer for a specific mesh + * component. Sets the @ref collisionMeshData_ references. + * @param importer The importer pre-loaded with asset data from file. + * @param meshName The string identifier of a specific mesh - i.e. with + * PrimitiveImporter to denote which Primitive to instantiate. + */ + void setMeshData(Magnum::Trade::AbstractImporter& importer, + const std::string& meshName); /** * @brief Returns a pointer to the compiled render data storage structure. diff --git a/src/esp/assets/ResourceManager.cpp b/src/esp/assets/ResourceManager.cpp index cf74114626..b1ee4cd2d5 100644 --- a/src/esp/assets/ResourceManager.cpp +++ b/src/esp/assets/ResourceManager.cpp @@ -52,7 +52,7 @@ #include "CollisionMeshData.h" #include "GenericInstanceMeshData.h" -#include "GltfMeshData.h" +#include "GenericMeshData.h" #include "MeshData.h" #ifdef ESP_BUILD_PTEX_SUPPORT @@ -107,11 +107,9 @@ void ResourceManager::initDefaultPrimAttributes() { auto icoSolidAttr = PhysicsIcospherePrimAttributes::create( false, PrimitiveNames3D[static_cast(PrimObjTypes::ICOSPHERE_SOLID)]); addPrimAssetTemplateToLibrary(icoSolidAttr); - // TODO: enable for pending Magnum implementation of wireframe icosphere - // auto icoWFAttr = PhysicsIcospherePrimAttributes::create( - // true, - // PrimitiveNames3D[static_cast(PrimObjTypes::ICOSPHERE_WF)]); - // addPrimAssetTemplateToLibrary(icoWFAttr); + auto icoWFAttr = PhysicsIcospherePrimAttributes::create( + true, PrimitiveNames3D[static_cast(PrimObjTypes::ICOSPHERE_WF)]); + addPrimAssetTemplateToLibrary(icoWFAttr); auto uvSphereSolidAttr = PhysicsUVSpherePrimAttributes::create( false, PrimitiveNames3D[static_cast(PrimObjTypes::UVSPHERE_SOLID)]); addPrimAssetTemplateToLibrary(uvSphereSolidAttr); @@ -397,8 +395,8 @@ bool ResourceManager::loadPhysicsScene( // GLB Mesh else if (info.type == AssetType::MP3D_MESH || info.type == AssetType::UNKNOWN) { - GltfMeshData* gltfMeshData = - dynamic_cast(meshes_[mesh_i].get()); + GenericMeshData* gltfMeshData = + dynamic_cast(meshes_[mesh_i].get()); if (gltfMeshData == nullptr) { Corrade::Utility::Debug() << "AssetInfo::AssetType type error: unsupported physical type, " @@ -714,8 +712,8 @@ int ResourceManager::loadObjectTemplate( //! Gather mesh components for meshGroup data std::vector meshGroup; for (int mesh_i = start; mesh_i <= end; ++mesh_i) { - GltfMeshData* gltfMeshData = - dynamic_cast(meshes_[mesh_i].get()); + GenericMeshData* gltfMeshData = + dynamic_cast(meshes_[mesh_i].get()); CollisionMeshData& meshData = gltfMeshData->getCollisionMeshData(); meshGroup.push_back(meshData); } @@ -1636,7 +1634,7 @@ void ResourceManager::loadMeshes(Importer& importer, for (int iMesh = 0; iMesh < importer.meshCount(); ++iMesh) { // don't need normals if we aren't using lighting - auto gltfMeshData = std::make_unique( + auto gltfMeshData = std::make_unique( loadedAssetData.assetInfo.requiresLighting); gltfMeshData->setMeshData(importer, iMesh); diff --git a/src/esp/assets/ResourceManager.h b/src/esp/assets/ResourceManager.h index ba6b9702d4..64a1e95471 100644 --- a/src/esp/assets/ResourceManager.h +++ b/src/esp/assets/ResourceManager.h @@ -25,7 +25,7 @@ #include "Attributes.h" #include "BaseMesh.h" #include "CollisionMeshData.h" -#include "GltfMeshData.h" +#include "GenericMeshData.h" #include "MeshData.h" #include "MeshMetaData.h" #include "esp/gfx/DrawableGroup.h" @@ -105,7 +105,7 @@ enum class PrimObjTypes : uint32_t { * DOES NOT EXIST * TODO: can/should this be added? */ - // ICOSPHERE_WF, + ICOSPHERE_WF, /** * Primitive object corresponding to Magnum::Primitives::uvSphereSolid */ @@ -121,11 +121,10 @@ enum class PrimObjTypes : uint32_t { }; constexpr const char* PrimitiveNames3D[]{ - "capsule3DSolid", "capsule3DWireframe", "coneSolid", "coneWireframe", - "cubeSolid", "cubeWireframe", "cylinderSolid", "cylinderWireframe", - "icosphereSolid", - //"icosphereWireframe", - "uvSphereSolid", "uvSphereWireframe"}; + "capsule3DSolid", "capsule3DWireframe", "coneSolid", + "coneWireframe", "cubeSolid", "cubeWireframe", + "cylinderSolid", "cylinderWireframe", "icosphereSolid", + "icosphereWireframe", "uvSphereSolid", "uvSphereWireframe"}; /** * @brief Singleton class responsible for diff --git a/src/esp/core/RigidState.h b/src/esp/core/RigidState.h index 3f4bca7a75..aa4d67a91d 100644 --- a/src/esp/core/RigidState.h +++ b/src/esp/core/RigidState.h @@ -26,5 +26,5 @@ struct RigidState { ESP_SMART_POINTERS(RigidState) }; -}; // namespace core -}; // namespace esp +} // namespace core +} // namespace esp From 34a5a6bef69b814ae7478acf491049c073218350 Mon Sep 17 00:00:00 2001 From: John Turner <7strbass@gmail.com> Date: Mon, 4 May 2020 13:45:58 -0400 Subject: [PATCH 2/2] --Address reviewer comments. --- src/esp/assets/GenericMeshData.cpp | 10 +++++----- src/esp/assets/ResourceManager.h | 2 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/esp/assets/GenericMeshData.cpp b/src/esp/assets/GenericMeshData.cpp index b6dc0734c2..63980781c2 100644 --- a/src/esp/assets/GenericMeshData.cpp +++ b/src/esp/assets/GenericMeshData.cpp @@ -52,12 +52,12 @@ void GenericMeshData::setMeshData(Magnum::Trade::AbstractImporter& importer, cache locality for vertex fetching) and is a no-op if the source data is already interleaved, so doesn't hurt to have it there always. */ - CORRADE_ASSERT( - meshData_->primitive() == Mn::MeshPrimitive::Triangles, - "Cannot instance collisionMeshData_; Triangle Mesh expected.", ); + /* TODO: Address that non-triangle meshes will have their collisionMeshData_ + * incorrectly calculated */ + meshData_ = Mn::MeshTools::interleave(*std::move(meshData_)); - collisionMeshData_.primitive = Magnum::MeshPrimitive::Triangles; + collisionMeshData_.primitive = meshData_->primitive(); /* For collision data we need positions as Vector3 in a contiguous array. There's little chance the data are stored like that in MeshData, so unpack @@ -78,7 +78,7 @@ void GenericMeshData::setMeshData(Magnum::Trade::AbstractImporter& importer, const std::string& meshName) { // make sure name is appropriate for importer int meshID = importer.meshForName(meshName); - CORRADE_ASSERT(meshID != -1, "Unknown meshName : " << meshName, ); + CORRADE_ASSERT(meshID != -1, "Unknown meshName :" << meshName, ); setMeshData(importer, meshID); } diff --git a/src/esp/assets/ResourceManager.h b/src/esp/assets/ResourceManager.h index 64a1e95471..e4f23389c8 100644 --- a/src/esp/assets/ResourceManager.h +++ b/src/esp/assets/ResourceManager.h @@ -102,8 +102,6 @@ enum class PrimObjTypes : uint32_t { ICOSPHERE_SOLID, /** * Primitive object corresponding to Magnum::Primitives::icosphereWireframe - * DOES NOT EXIST - * TODO: can/should this be added? */ ICOSPHERE_WF, /**