From 9e8ddf20e54bd5b97535e67186d83008a70ccf3e Mon Sep 17 00:00:00 2001 From: John Turner <7strbass@gmail.com> Date: Thu, 5 Dec 2024 16:35:36 -0500 Subject: [PATCH] --[BE] - Miscellaneous Clean-up and Refactors (#2513) * --loop iterator signed-ness matching * --remove unused values; improve field validation feedback * --cleanup config bindings * --mark unused arguments * --signed-ness; moving a temp prevents copy elision * --only 1 copy of constexpr string; unused args cleanup. * --properly handle the move constructors --- src/esp/assets/ResourceManager.cpp | 27 +++++---- src/esp/bindings/ConfigBindings.cpp | 18 +++--- src/esp/metadata/URDFParser.cpp | 39 ++++++------- .../metadata/attributes/AbstractAttributes.h | 2 +- .../attributes/AttributesEnumMaps.cpp | 31 ++++++----- .../attributes/LightLayoutAttributes.cpp | 2 +- .../attributes/SceneInstanceAttributes.cpp | 14 ++--- .../attributes/SemanticAttributes.cpp | 4 +- .../managers/AbstractAttributesManager.h | 2 +- .../managers/AssetAttributesManager.cpp | 4 +- .../managers/LightLayoutAttributesManager.cpp | 5 +- .../managers/PbrShaderAttributesManager.cpp | 3 +- .../SceneDatasetAttributesManager.cpp | 2 +- .../managers/SceneDatasetAttributesManager.h | 2 +- .../SceneInstanceAttributesManager.cpp | 55 +++++++++++-------- src/esp/sensor/CameraSensor.cpp | 2 +- src/esp/sensor/VisualSensor.cpp | 2 +- src/esp/sim/BatchPlayerImplementation.cpp | 2 +- src/esp/sim/BatchReplayRenderer.cpp | 7 ++- src/esp/sim/ClassicReplayRenderer.cpp | 6 +- 20 files changed, 121 insertions(+), 108 deletions(-) diff --git a/src/esp/assets/ResourceManager.cpp b/src/esp/assets/ResourceManager.cpp index 07eae508d3..521ca1a65a 100644 --- a/src/esp/assets/ResourceManager.cpp +++ b/src/esp/assets/ResourceManager.cpp @@ -1238,7 +1238,8 @@ void ResourceManager::buildPrimitiveAssetData( << "Primitive Asset Added : ID : " << primTemplate->getID() << ": Attributes key : `" << primTemplate->getHandle() << "`| Class : `" << primClassName << "` | Importer Conf has group for this obj type : " - << conf.hasGroup(primClassName); + << conf.hasGroup(primClassName) << " | Asset placed in resourceDict : " + << (inserted.second ? "True" : "False"); } // ResourceManager::buildPrimitiveAssetData @@ -1397,7 +1398,7 @@ bool ResourceManager::loadRenderAssetSemantic(const AssetInfo& info) { // specify colormap to use to build TextureVisualizerShader // If this is true, we want to build a colormap from the vertex colors. - for (int meshIDLocal = 0; meshIDLocal < instanceMeshes.size(); + for (uint32_t meshIDLocal = 0; meshIDLocal < instanceMeshes.size(); ++meshIDLocal) { if (getCreateRenderer()) { instanceMeshes[meshIDLocal]->uploadBuffersToGPU(false); @@ -1546,7 +1547,7 @@ bool ResourceManager::loadRenderAssetGeneral(const AssetInfo& info) { "Error loading general mesh data from file '{}'", filename)); // load file and add it to the dictionary - LoadedAssetData loadedAssetData{info}; + LoadedAssetData loadedAssetData{info, {}}; if (requiresTextures_) { loadTextures(*fileImporter_, loadedAssetData); loadMaterials(*fileImporter_, loadedAssetData); @@ -1826,8 +1827,8 @@ bool ResourceManager::buildTrajectoryVisualization( // if (resourceDict_.count(trajVisName) != 0) { // resourceDict_.erase(trajVisName); // } - auto inserted = - resourceDict_.emplace(trajVisName, std::move(loadedAssetData)); + // auto inserted = + resourceDict_.emplace(trajVisName, std::move(loadedAssetData)); return true; } // ResourceManager::loadTrajectoryVisualization @@ -2046,7 +2047,7 @@ Mn::Trade::MaterialData createUniversalMaterial( arrayAppend(newAttributes, {{MaterialAttribute::AmbientTexture, BCTexture}, {MaterialAttribute::DiffuseTexture, BCTexture}}); - if (metalness >= 0.5) { + if (metalness >= 0.5f) { arrayAppend(newAttributes, {MaterialAttribute::SpecularTexture, BCTexture}); } @@ -2634,7 +2635,7 @@ void ResourceManager::loadMeshes(Importer& importer, nextMeshID_ = meshEnd + 1; loadedAssetData.meshMetaData.setMeshIndices(meshStart, meshEnd); - for (int iMesh = 0; iMesh < importer.meshCount(); ++iMesh) { + for (uint32_t iMesh = 0; iMesh < importer.meshCount(); ++iMesh) { // don't need normals if we aren't using lighting auto gltfMeshData = std::make_unique( !loadedAssetData.assetInfo.forceFlatShading); @@ -2659,7 +2660,7 @@ void ResourceManager::loadSkins(Importer& importer, nextSkinID_ = skinEnd + 1; loadedAssetData.meshMetaData.setSkinIndices(skinStart, skinEnd); - for (int iSkin = 0; iSkin < importer.skin3DCount(); ++iSkin) { + for (uint32_t iSkin = 0; iSkin < importer.skin3DCount(); ++iSkin) { auto skinData = std::make_shared(); Cr::Containers::Optional skin = @@ -2694,10 +2695,10 @@ Mn::Image2D ResourceManager::convertRGBToSemanticId( srcImage.pixels(); Cr::Containers::StridedArrayView2D output = resImage.pixels(); - for (std::size_t y = 0; y != size.y(); ++y) { + for (long y = 0; y != size.y(); ++y) { Cr::Containers::StridedArrayView1D inputRow = input[y]; Cr::Containers::StridedArrayView1D outputRow = output[y]; - for (std::size_t x = 0; x != size.x(); ++x) { + for (long x = 0; x != size.x(); ++x) { const Mn::Color3ub color = inputRow[x]; /* Fugly. Sorry. Needs better API on Magnum side. */ const Mn::UnsignedInt colorInt = geo::getValueAsUInt(color); @@ -2739,7 +2740,8 @@ void ResourceManager::loadTextures(Importer& importer, static_cast(i); } - for (int iTexture = 0; iTexture < importer.textureCount(); ++iTexture) { + for (uint32_t iTexture = 0; iTexture < importer.textureCount(); + ++iTexture) { auto currentTextureID = textureStart + iTexture; auto txtrIter = textures_.emplace(currentTextureID, std::make_shared()); @@ -2775,7 +2777,8 @@ void ResourceManager::loadTextures(Importer& importer, // Whether semantic RGB or not } } else { - for (int iTexture = 0; iTexture < importer.textureCount(); ++iTexture) { + for (uint32_t iTexture = 0; iTexture < importer.textureCount(); + ++iTexture) { auto currentTextureID = textureStart + iTexture; auto txtrIter = textures_.emplace(currentTextureID, std::make_shared()); diff --git a/src/esp/bindings/ConfigBindings.cpp b/src/esp/bindings/ConfigBindings.cpp index dbf7b1efa2..c8236fc86f 100644 --- a/src/esp/bindings/ConfigBindings.cpp +++ b/src/esp/bindings/ConfigBindings.cpp @@ -20,10 +20,12 @@ py::object getObjectForConfigValue(const ConfigValue& value) { return py::cast(value.get()); case ConfigValType::Integer: return py::cast(value.get()); + case ConfigValType::MagnumRad: + return py::cast(static_cast(value.get())); + case ConfigValType::MagnumDeg: + return py::cast(static_cast(value.get())); case ConfigValType::Double: return py::cast(value.get()); - case ConfigValType::String: - return py::cast(value.get()); case ConfigValType::MagnumVec2: return py::cast(value.get()); case ConfigValType::MagnumVec2i: @@ -38,10 +40,8 @@ py::object getObjectForConfigValue(const ConfigValue& value) { return py::cast(value.get()); case ConfigValType::MagnumQuat: return py::cast(value.get()); - case ConfigValType::MagnumRad: - return py::cast(static_cast(value.get())); - case ConfigValType::MagnumDeg: - return py::cast(static_cast(value.get())); + case ConfigValType::String: + return py::cast(value.get()); } return py::cast(nullptr); } @@ -83,8 +83,9 @@ void initConfigBindings(py::module& m) { .value("Unknown", ConfigValType::Unknown) .value("Boolean", ConfigValType::Boolean) .value("Integer", ConfigValType::Integer) + .value("MagnumRad", ConfigValType::MagnumRad) + .value("MagnumDeg", ConfigValType::MagnumDeg) .value("Float", ConfigValType::Double) - .value("String", ConfigValType::String) .value("MagnumVec2", ConfigValType::MagnumVec2) .value("MagnumVec2i", ConfigValType::MagnumVec2i) .value("MagnumVec3", ConfigValType::MagnumVec3) @@ -92,8 +93,7 @@ void initConfigBindings(py::module& m) { .value("MagnumQuat", ConfigValType::MagnumQuat) .value("MagnumMat3", ConfigValType::MagnumMat3) .value("MagnumMat4", ConfigValType::MagnumMat4) - .value("MagnumRad", ConfigValType::MagnumRad) - .value("MagnumDeg", ConfigValType::MagnumDeg); + .value("String", ConfigValType::String); auto pyConfiguration = py::class_(m, "Configuration"); diff --git a/src/esp/metadata/URDFParser.cpp b/src/esp/metadata/URDFParser.cpp index 46f17de59b..6b04867e19 100644 --- a/src/esp/metadata/URDFParser.cpp +++ b/src/esp/metadata/URDFParser.cpp @@ -453,24 +453,25 @@ bool Parser::parseLink(const std::shared_ptr& model, ESP_VERY_VERBOSE() << link.m_name; return false; } - link.m_inertia.m_mass *= model->getGlobalScaling(); + link.m_inertia.m_mass *= static_cast(model->getGlobalScaling()); } else { if ((strlen(linkName) == 5) && (strncmp(linkName, "world", 5)) == 0) { - link.m_inertia.m_mass = 0.f; + link.m_inertia.m_mass = 0.0; link.m_inertia.m_linkLocalFrame = Mn::Matrix4(); // Identity - link.m_inertia.m_ixx = 0.f; - link.m_inertia.m_iyy = 0.f; - link.m_inertia.m_izz = 0.f; + link.m_inertia.m_ixx = 0.0; + link.m_inertia.m_iyy = 0.0; + link.m_inertia.m_izz = 0.0; } else { ESP_VERY_VERBOSE() << "W - No inertial data for link:" << link.m_name << ", using mass=1, localinertiadiagonal = 1,1,1, identity local " "inertial frame"; - link.m_inertia.m_mass = 1.f * model->getMassScaling(); + link.m_inertia.m_mass = + 1.0 * static_cast(model->getMassScaling()); link.m_inertia.m_linkLocalFrame = Mn::Matrix4(); // Identity - link.m_inertia.m_ixx = 1.f; - link.m_inertia.m_iyy = 1.f; - link.m_inertia.m_izz = 1.f; + link.m_inertia.m_ixx = 1.0; + link.m_inertia.m_iyy = 1.0; + link.m_inertia.m_izz = 1.0; } } @@ -625,9 +626,9 @@ bool Parser::parseTransform(Mn::Matrix4& tr, const XMLElement* xml) const { if (rpy_str != nullptr) { Mn::Vector3 rpy; if (parseVector3(rpy, std::string(rpy_str))) { - double roll = rpy[0]; - double pitch = rpy[1]; - double yaw = rpy[2]; + double roll = static_cast(rpy[0]); + double pitch = static_cast(rpy[1]); + double yaw = static_cast(rpy[2]); double phi = roll / 2.0; double the = pitch / 2.0; @@ -766,7 +767,7 @@ bool Parser::parseGeometry(Geometry& geom, const XMLElement* g) { bool Parser::parseInertia(Inertia& inertia, const XMLElement* config) { inertia.m_linkLocalFrame = Mn::Matrix4(); // Identity - inertia.m_mass = 0.f; + inertia.m_mass = 0.0; // Origin const XMLElement* o = config->FirstChildElement("origin"); @@ -913,12 +914,12 @@ bool Parser::initTreeAndRoot(const std::shared_ptr& model) const { bool Parser::parseJointLimits(Joint& joint, const tinyxml2::XMLElement* config) const { - joint.m_lowerLimit = 0.f; - joint.m_upperLimit = -1.f; - joint.m_effortLimit = 0.f; - joint.m_velocityLimit = 0.f; - joint.m_jointDamping = 0.f; - joint.m_jointFriction = 0.f; + joint.m_lowerLimit = 0.0; + joint.m_upperLimit = -1.0; + joint.m_effortLimit = 0.0; + joint.m_velocityLimit = 0.0; + joint.m_jointDamping = 0.0; + joint.m_jointFriction = 0.0; const char* lower_str = config->Attribute("lower"); if (lower_str) { diff --git a/src/esp/metadata/attributes/AbstractAttributes.h b/src/esp/metadata/attributes/AbstractAttributes.h index ba414933d8..939460f4da 100644 --- a/src/esp/metadata/attributes/AbstractAttributes.h +++ b/src/esp/metadata/attributes/AbstractAttributes.h @@ -19,7 +19,7 @@ namespace metadata { * attributes should replace this tag with the base filename (minus all paths * and extensions) */ -constexpr char CONFIG_NAME_AS_ASSET_FILENAME[] = +static constexpr char CONFIG_NAME_AS_ASSET_FILENAME[] = "%%CONFIG_NAME_AS_ASSET_FILENAME%%"; namespace attributes { diff --git a/src/esp/metadata/attributes/AttributesEnumMaps.cpp b/src/esp/metadata/attributes/AttributesEnumMaps.cpp index 8b97905aa6..fce088beae 100644 --- a/src/esp/metadata/attributes/AttributesEnumMaps.cpp +++ b/src/esp/metadata/attributes/AttributesEnumMaps.cpp @@ -344,23 +344,26 @@ const std::map * the string key it maps to in the FisheyeSensorModelTypeMap */ std::string getFisheyeSensorModelTypeName( - esp::sensor::FisheyeSensorModelType fisheyeSensorModelTypeEnum) { + CORRADE_UNUSED esp::sensor::FisheyeSensorModelType + fisheyeSensorModelTypeEnum) { // this verifies that enum value being checked is supported by string-keyed // map. The values below should be the minimum and maximum enums supported by // FisheyeSensorModelTypeMap - if ((fisheyeSensorModelTypeEnum <= - esp::sensor::FisheyeSensorModelType::DoubleSphere) || - (fisheyeSensorModelTypeEnum >= - esp::sensor::FisheyeSensorModelType::EndFisheyeSensorModelType)) { - // default to double_sphere for illegal values - return "double_sphere"; - } - // Must always be valid value - for (const auto& it : FisheyeSensorModelTypeMap) { - if (it.second == fisheyeSensorModelTypeEnum) { - return it.first; - } - } + + // TODO expand to handle more types of FisheyeSensors + // if ((fisheyeSensorModelTypeEnum <= + // esp::sensor::FisheyeSensorModelType::DoubleSphere) || + // (fisheyeSensorModelTypeEnum >= + // esp::sensor::FisheyeSensorModelType::EndFisheyeSensorModelType)) { + // // default to double_sphere for illegal values + // return "double_sphere"; + // } + // // Must always be valid value + // for (const auto& it : FisheyeSensorModelTypeMap) { + // if (it.second == fisheyeSensorModelTypeEnum) { + // return it.first; + // } + // } return "double_sphere"; } // getFisheyeSensorModelTypeName diff --git a/src/esp/metadata/attributes/LightLayoutAttributes.cpp b/src/esp/metadata/attributes/LightLayoutAttributes.cpp index 83a0fc7a45..eabfec5aae 100644 --- a/src/esp/metadata/attributes/LightLayoutAttributes.cpp +++ b/src/esp/metadata/attributes/LightLayoutAttributes.cpp @@ -58,7 +58,7 @@ LightLayoutAttributes::LightLayoutAttributes(const LightLayoutAttributes& otr) } LightLayoutAttributes::LightLayoutAttributes( LightLayoutAttributes&& otr) noexcept - : AbstractAttributes(std::move(static_cast(otr))), + : AbstractAttributes(std::move(static_cast(otr))), availableLightIDs_(std::move(otr.availableLightIDs_)) { lightInstConfig_ = editSubconfig("lights"); } diff --git a/src/esp/metadata/attributes/SceneInstanceAttributes.cpp b/src/esp/metadata/attributes/SceneInstanceAttributes.cpp index 254dded883..a2e21f8580 100644 --- a/src/esp/metadata/attributes/SceneInstanceAttributes.cpp +++ b/src/esp/metadata/attributes/SceneInstanceAttributes.cpp @@ -229,17 +229,13 @@ SceneAOInstanceAttributes::SceneAOInstanceAttributes( std::string SceneAOInstanceAttributes::getSceneObjInstanceInfoHeaderInternal() const { std::string infoHdr{"Base Type,Inertia Source,Link Order,Render Mode,"}; - int iter = 0; const auto initJointPose = getInitJointPose(); - for (const auto& it : initJointPose) { - Cr::Utility::formatInto(infoHdr, infoHdr.size(), "Init Pose {},", - std::to_string(iter++)); + for (uint32_t iter = 0; iter < initJointPose.size(); ++iter) { + Cr::Utility::formatInto(infoHdr, infoHdr.size(), "Init Pose {},", iter); } - iter = 0; const auto initJointVel = getInitJointVelocities(); - for (const auto& it : initJointVel) { - Cr::Utility::formatInto(infoHdr, infoHdr.size(), "Init Vel {},", - std::to_string(iter++)); + for (uint32_t iter = 0; iter < initJointPose.size(); ++iter) { + Cr::Utility::formatInto(infoHdr, infoHdr.size(), "Init Vel {},", iter); } return infoHdr; } // SceneAOInstanceAttributes::getSceneObjInstanceInfoHeaderInternal @@ -319,7 +315,7 @@ SceneInstanceAttributes::SceneInstanceAttributes( } SceneInstanceAttributes::SceneInstanceAttributes( SceneInstanceAttributes&& otr) noexcept - : AbstractAttributes(std::move(static_cast(otr))), + : AbstractAttributes(std::move(static_cast(otr))), availableObjInstIDs_(std::move(otr.availableObjInstIDs_)), availableArtObjInstIDs_(std::move(otr.availableArtObjInstIDs_)) { // get refs to internal subconfigs for object and ao instances diff --git a/src/esp/metadata/attributes/SemanticAttributes.cpp b/src/esp/metadata/attributes/SemanticAttributes.cpp index be6d02c304..098b24da64 100644 --- a/src/esp/metadata/attributes/SemanticAttributes.cpp +++ b/src/esp/metadata/attributes/SemanticAttributes.cpp @@ -22,7 +22,7 @@ std::string SemanticVolumeAttributes::getObjectInfoHeaderInternal() const { std::string res = "Name,Label,Floor Height,Extrusion Height,Min Bounds,Max Bounds,"; const auto& polyLoop = getPolyLoop(); - for (int iter = 0; iter < polyLoop.size(); ++iter) { + for (uint32_t iter = 0; iter < polyLoop.size(); ++iter) { Cr::Utility::formatInto(res, res.size(), "Poly Vert {} XYZ,", std::to_string(iter)); } @@ -90,7 +90,7 @@ SemanticAttributes::SemanticAttributes(const SemanticAttributes& otr) } // SemanticAttributes copy ctor SemanticAttributes::SemanticAttributes(SemanticAttributes&& otr) noexcept - : AbstractAttributes(std::move(static_cast(otr))), + : AbstractAttributes(std::move(static_cast(otr))), availableRegionInstIDs_(std::move(otr.availableRegionInstIDs_)) { // get refs to internal subconfigs for semantic region attributes // original data were moved over so should retain full derived class diff --git a/src/esp/metadata/managers/AbstractAttributesManager.h b/src/esp/metadata/managers/AbstractAttributesManager.h index 2ecd33b76f..f941704487 100644 --- a/src/esp/metadata/managers/AbstractAttributesManager.h +++ b/src/esp/metadata/managers/AbstractAttributesManager.h @@ -413,7 +413,7 @@ AbstractAttributesManager::loadAllFileBasedTemplates( std::string dir = CrPath::split(paths[0]).first(); ESP_DEBUG() << "Loading" << paths.size() << "" << this->objectType_ << "templates found in" << dir; - for (int i = 0; i < paths.size(); ++i) { + for (uint32_t i = 0; i < paths.size(); ++i) { auto attributesFilename = paths[i]; ESP_VERY_VERBOSE() << "Load" << this->objectType_ << "template:" << CrPath::split(attributesFilename).second(); diff --git a/src/esp/metadata/managers/AssetAttributesManager.cpp b/src/esp/metadata/managers/AssetAttributesManager.cpp index 0302915ef4..4ebfc7e3ed 100644 --- a/src/esp/metadata/managers/AssetAttributesManager.cpp +++ b/src/esp/metadata/managers/AssetAttributesManager.cpp @@ -250,8 +250,8 @@ AbstractPrimitiveAttributes::ptr AssetAttributesManager::buildObjectFromJSONDoc( } // AssetAttributesManager::buildObjectFromJSONDoc void AssetAttributesManager::setValsFromJSONDoc( - AttribsPtr attribs, - const io::JsonGenericValue& jsonConfig) { + CORRADE_UNUSED AttribsPtr attribs, + CORRADE_UNUSED const io::JsonGenericValue& jsonConfig) { // TODO support loading values from JSON docs // check for user defined attributes // this->parseUserDefinedJsonVals(attribs, jsonConfig); diff --git a/src/esp/metadata/managers/LightLayoutAttributesManager.cpp b/src/esp/metadata/managers/LightLayoutAttributesManager.cpp index 0240274d5f..cbf6d9d9fe 100644 --- a/src/esp/metadata/managers/LightLayoutAttributesManager.cpp +++ b/src/esp/metadata/managers/LightLayoutAttributesManager.cpp @@ -325,8 +325,9 @@ gfx::LightSetup LightLayoutAttributesManager::createLightSetupFromAttributes( lightVector = {lightAttr->getPosition(), 1.0f}; } } // switch on type - res.push_back( - {.vector = lightVector, .color = color, .model = posModelEnum}); + + res.push_back({lightVector, color, posModelEnum}); + //{.vector = lightVector, .color = color, .model = posModelEnum}); } // for each light instance described } // if >0 light instances described } // lightLayoutAttributes of requested name exists diff --git a/src/esp/metadata/managers/PbrShaderAttributesManager.cpp b/src/esp/metadata/managers/PbrShaderAttributesManager.cpp index 8bbcf939ec..6da1e01a19 100644 --- a/src/esp/metadata/managers/PbrShaderAttributesManager.cpp +++ b/src/esp/metadata/managers/PbrShaderAttributesManager.cpp @@ -231,7 +231,8 @@ PbrShaderAttributesManager::preRegisterObjectFinalize( } // PbrShaderAttributesManager::preRegisterObjectFinalize void PbrShaderAttributesManager::finalizeAttrPathsBeforeRegister( - const attributes::PbrShaderAttributes::ptr& attributes) const { + CORRADE_UNUSED const attributes::PbrShaderAttributes::ptr& attributes) + const { // TODO Verify getIBLBrdfLUTAssetHandle and getIBLEnvMapAssetHandle exist as // either file-based assets or resources and build paths to be relative if // file-based diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp index 5d2932804c..602341e5fa 100644 --- a/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp +++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.cpp @@ -151,7 +151,7 @@ void SceneDatasetAttributesManager::readDatasetJSONCell( // Get the count of the number of expected members found. If more members // than this exist in the cell, then we will read for extra key-value // mappings. - int numMembersFound = 0; + uint32_t numMembersFound = 0; // process JSON jCell here - this cell potentially holds : // 1. "default_attributes" : a single attributes default of the // specified type. diff --git a/src/esp/metadata/managers/SceneDatasetAttributesManager.h b/src/esp/metadata/managers/SceneDatasetAttributesManager.h index d95ca497de..a2a558bc0f 100644 --- a/src/esp/metadata/managers/SceneDatasetAttributesManager.h +++ b/src/esp/metadata/managers/SceneDatasetAttributesManager.h @@ -105,7 +105,7 @@ class SceneDatasetAttributesManager * @param attributes The attributes to be filtered. */ void finalizeAttrPathsBeforeRegister( - const attributes::SceneDatasetAttributes::ptr& attributes) + CORRADE_UNUSED const attributes::SceneDatasetAttributes::ptr& attributes) const override {} protected: diff --git a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp index 0494f951eb..c7e5c759e1 100644 --- a/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp +++ b/src/esp/metadata/managers/SceneInstanceAttributesManager.cpp @@ -196,31 +196,38 @@ void SceneInstanceAttributesManager::setValsFromJSONDoc( io::JsonGenericValue::ConstMemberIterator pbrShaderRegionJSONIter = jsonConfig.FindMember("pbr_shader_region_configs"); - if ((pbrShaderRegionJSONIter != jsonConfig.MemberEnd()) && - (pbrShaderRegionJSONIter->value.IsObject())) { - const auto& articulatedObjArray = artObjJSONIter->value; - - // pbr_shader_region_configs tag exists, and should be an object, holding - // unique region names and the handle to the PbrShaderAttributes to use for - // that region. - - // Tag should have the format of an array of key-value - // pairs, where the key is some region identifier and the value is a - // string representing the PbrShaderAttributes to use, as specified in the - // PbrShaderAttributesManager. - const auto& pbrShaderRegionHandles = pbrShaderRegionJSONIter->value; - int count = 0; - // iterate through objects - for (rapidjson::Value::ConstMemberIterator it = - pbrShaderRegionHandles.MemberBegin(); - it != pbrShaderRegionHandles.MemberEnd(); ++it) { - // create attributes and set its name to be the tag in the JSON for the - // individual light - const std::string region = it->name.GetString(); - const std::string pbrHandle = it->value.GetString(); - attribs->addRegionPbrShaderAttributesHandle(region, pbrHandle); + if (pbrShaderRegionJSONIter != jsonConfig.MemberEnd()) { + if (pbrShaderRegionJSONIter->value.IsObject()) { + // pbr_shader_region_configs tag exists, and should be an object, holding + // unique region names and the handle to the PbrShaderAttributes to use + // for that region. + + // Tag should have the format of an array of key-value + // pairs, where the key is some region identifier and the value is a + // string representing the PbrShaderAttributes to use, as specified in the + // PbrShaderAttributesManager. + const auto& pbrShaderRegionHandles = pbrShaderRegionJSONIter->value; + // iterate through objects + for (rapidjson::Value::ConstMemberIterator it = + pbrShaderRegionHandles.MemberBegin(); + it != pbrShaderRegionHandles.MemberEnd(); ++it) { + // create attributes and set its name to be the tag in the JSON for the + // individual light + const std::string region = it->name.GetString(); + const std::string pbrHandle = it->value.GetString(); + attribs->addRegionPbrShaderAttributesHandle(region, pbrHandle); + } + } else { + // Non-object (i.e. array) quantities are not supported for + // pbrShaderRegionConfigs + ESP_ERROR(Mn::Debug::Flag::NoSpace) + << "PBR/IBL Shader configurations were specified for Scene " + "Instance `" + << attribsDispName + << "` but they were specified incorrectly. Please make sure the " + "value with tag `pbr_shader_region_configs` is of proper format " + "(a JSON Object, not array)"; } - } else { // No pbr_shader_region_configs tag exists in scene instance. Not // necessarily a bad thing, not all datasets have specified PBR/IBL diff --git a/src/esp/sensor/CameraSensor.cpp b/src/esp/sensor/CameraSensor.cpp index cd5c7d6889..9e1f53bd2d 100644 --- a/src/esp/sensor/CameraSensor.cpp +++ b/src/esp/sensor/CameraSensor.cpp @@ -29,7 +29,7 @@ void CameraSensorSpec::sanityCheck() const { "SensorSubType " "Pinhole or Orthographic", ); CORRADE_ASSERT( - orthoScale > 0, + orthoScale > 0.0f, "CameraSensorSpec::sanityCheck(): orthoScale must be greater than 0", ); } diff --git a/src/esp/sensor/VisualSensor.cpp b/src/esp/sensor/VisualSensor.cpp index 4af59f4d42..7538f548d4 100644 --- a/src/esp/sensor/VisualSensor.cpp +++ b/src/esp/sensor/VisualSensor.cpp @@ -49,7 +49,7 @@ void VisualSensorSpec::sanityCheck() const { "VisualSensorSpec::sanityCheck(): the value of the channels which is" << channels << "is illegal", ); CORRADE_ASSERT( - near > 0.0 && far > near, + near > 0.0f && far > near, "VisualSensorSpec::sanityCheck(): the near or far plane is illegal.", ); } diff --git a/src/esp/sim/BatchPlayerImplementation.cpp b/src/esp/sim/BatchPlayerImplementation.cpp index bf4c230fba..699466925f 100644 --- a/src/esp/sim/BatchPlayerImplementation.cpp +++ b/src/esp/sim/BatchPlayerImplementation.cpp @@ -131,7 +131,7 @@ void BatchPlayerImplementation::changeLightSetup( } renderer_.clearLights(sceneId_); - for (std::size_t i = 0; i != lights.size(); ++i) { + for (std::size_t i = 0; i < lights.size(); ++i) { const gfx::LightInfo& light = lights[i]; CORRADE_INTERNAL_ASSERT(light.model == gfx::LightPositionModel::Global); diff --git a/src/esp/sim/BatchReplayRenderer.cpp b/src/esp/sim/BatchReplayRenderer.cpp index ced37c3d7b..f03c691556 100644 --- a/src/esp/sim/BatchReplayRenderer.cpp +++ b/src/esp/sim/BatchReplayRenderer.cpp @@ -47,7 +47,7 @@ BatchReplayRenderer::BatchReplayRenderer( theOnlySensorName_ = sensor.uuid; theOnlySensorProjection_ = sensor.projectionMatrix(); - for (Mn::UnsignedInt i = 0; i != cfg.numEnvironments; ++i) { + for (int i = 0; i < cfg.numEnvironments; ++i) { arrayAppend( envs_, EnvironmentRecord{ std::make_shared(*renderer_, i)}); @@ -63,7 +63,7 @@ void BatchReplayRenderer::doClose() { } void BatchReplayRenderer::doCloseImpl() { - for (int i = 0; i < envs_.size(); ++i) { + for (uint32_t i = 0; i < envs_.size(); ++i) { envs_[i].player_.close(); } envs_ = {}; @@ -125,7 +125,8 @@ void BatchReplayRenderer::doRender( // todo: integrate debugLineRender_->flushLines CORRADE_INTERNAL_ASSERT(!debugLineRender_); - for (int envIndex = 0; envIndex != envs_.size(); ++envIndex) { + for (int envIndex = 0; envIndex < static_cast(envs_.size()); + ++envIndex) { const auto rectangle = Mn::Range2Di::fromSize( renderer_->tileSize() * Mn::Vector2i{envIndex % renderer_->tileCount().x(), diff --git a/src/esp/sim/ClassicReplayRenderer.cpp b/src/esp/sim/ClassicReplayRenderer.cpp index 7a54186eb9..2ab43beffa 100644 --- a/src/esp/sim/ClassicReplayRenderer.cpp +++ b/src/esp/sim/ClassicReplayRenderer.cpp @@ -59,7 +59,7 @@ ClassicReplayRenderer::ClassicReplayRenderer( "A rig instance with the specified ID already exists."); gfx::Rig rig{}; - for (int i = 0; i < boneNames.size(); ++i) { + for (uint32_t i = 0; i < boneNames.size(); ++i) { const std::string& boneName = boneNames[i]; rig.boneNames[boneName] = i; @@ -82,7 +82,7 @@ ClassicReplayRenderer::ClassicReplayRenderer( const std::vector& pose) override { auto& rig = self_.resourceManager_->getRigManager().getRigInstance(rigId); const size_t boneCount = rig.bones.size(); - for (int i = 0; i < boneCount; ++i) { + for (uint32_t i = 0; i < boneCount; ++i) { const auto& transform = pose[i]; rig.bones[i] ->setTranslation(transform.translation) @@ -181,7 +181,7 @@ void ClassicReplayRenderer::doClose() { } void ClassicReplayRenderer::doCloseImpl() { - for (int envIdx = 0; envIdx < envs_.size(); ++envIdx) { + for (uint32_t envIdx = 0; envIdx < envs_.size(); ++envIdx) { envs_[envIdx].player_.close(); auto& sensorMap = envs_[envIdx].sensorMap_; for (auto& sensorPair : sensorMap) {