--Renamed GltfMeshData -> GenericMeshData; (#617)

* --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.

* --Address reviewer comments.

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<Magnum::Trade::MeshData> meshData_ =
       Corrade::Containers::NullOpt;

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

src/esp/assets/GltfMeshData.cpp → 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 <Corrade/Containers/Array.h>
 #include <Corrade/Containers/ArrayViewStl.h>
+#include <Corrade/Utility/DebugStl.h>
 #include <Magnum/MeshTools/Compile.h>
 #include <Magnum/MeshTools/Interleave.h>
-
 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<GltfMeshData::RenderingBuffer>();
+  renderingBuffer_ = std::make_unique<GenericMeshData::RenderingBuffer>();
   Magnum::MeshTools::CompileFlags compileFlags{};
   if (needsNormals_ &&
       !meshData_->hasAttribute(Mn::Trade::MeshAttribute::Normal)) {
@@ -36,28 +36,28 @@ void GltfMeshData::uploadBuffersToGPU(bool forceReload) {
   buffersOnGPU_ = true;
 }
 
-Magnum::GL::Mesh* GltfMeshData::getMagnumGLMesh() {
+Magnum::GL::Mesh* GenericMeshData::getMagnumGLMesh() {
   if (renderingBuffer_ == nullptr) {
     return nullptr;
   }
 
   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<Mn::Trade::MeshData> meshData =
-      importer.mesh(meshID);
-  if (meshData)
-    meshData_ = Mn::MeshTools::interleave(*std::move(meshData));
-  else
-    meshData_ = Cr::Containers::NullOpt;
 
-  collisionMeshData_.primitive = Magnum::MeshPrimitive::Triangles;
+  /* TODO: Address that non-triangle meshes will have their collisionMeshData_
+   * incorrectly calculated */
+
+  meshData_ = Mn::MeshTools::interleave(*std::move(meshData_));
+
+  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
@@ -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

src/esp/assets/GltfMeshData.h → 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 <Corrade/Containers/Optional.h>
@@ -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.

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<int>(PrimObjTypes::ICOSPHERE_SOLID)]);
   addPrimAssetTemplateToLibrary(icoSolidAttr);
-  // TODO: enable for pending Magnum implementation of wireframe icosphere
-  // auto icoWFAttr = PhysicsIcospherePrimAttributes::create(
-  //     true,
-  //     PrimitiveNames3D[static_cast<int>(PrimObjTypes::ICOSPHERE_WF)]);
-  // addPrimAssetTemplateToLibrary(icoWFAttr);
+  auto icoWFAttr = PhysicsIcospherePrimAttributes::create(
+      true, PrimitiveNames3D[static_cast<int>(PrimObjTypes::ICOSPHERE_WF)]);
+  addPrimAssetTemplateToLibrary(icoWFAttr);
   auto uvSphereSolidAttr = PhysicsUVSpherePrimAttributes::create(
       false, PrimitiveNames3D[static_cast<int>(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<GltfMeshData*>(meshes_[mesh_i].get());
+        GenericMeshData* gltfMeshData =
+            dynamic_cast<GenericMeshData*>(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<CollisionMeshData> meshGroup;
   for (int mesh_i = start; mesh_i <= end; ++mesh_i) {
-    GltfMeshData* gltfMeshData =
-        dynamic_cast<GltfMeshData*>(meshes_[mesh_i].get());
+    GenericMeshData* gltfMeshData =
+        dynamic_cast<GenericMeshData*>(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<GltfMeshData>(
+    auto gltfMeshData = std::make_unique<GenericMeshData>(
         loadedAssetData.assetInfo.requiresLighting);
     gltfMeshData->setMeshData(importer, iMesh);
 

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"
@@ -102,10 +102,8 @@ 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,
+  ICOSPHERE_WF,
   /**
    * Primitive object corresponding to Magnum::Primitives::uvSphereSolid
    */
@@ -121,11 +119,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

src/esp/core/RigidState.h

@@ -26,5 +26,5 @@ struct RigidState {
 
   ESP_SMART_POINTERS(RigidState)
 };
-};  // namespace core
-};  // namespace esp
+}  // namespace core
+}  // namespace esp