From bff9231c3b2a587050ae9061386cb803576b495c Mon Sep 17 00:00:00 2001 From: Alexei Kotov Date: Thu, 14 Sep 2023 13:07:20 +0300 Subject: [PATCH] Refactor NiGeometry/BSTriShape Don't pass invalid geometry data links to the loaders --- apps/openmw_test_suite/nif/node.hpp | 4 +- .../nifloader/testbulletnifloader.cpp | 44 +++---------------- components/nif/node.cpp | 44 +++++++++++++++---- components/nif/node.hpp | 20 ++++----- components/nifbullet/bulletnifloader.cpp | 16 +++---- components/nifosg/nifloader.cpp | 34 ++++++-------- 6 files changed, 73 insertions(+), 89 deletions(-) diff --git a/apps/openmw_test_suite/nif/node.hpp b/apps/openmw_test_suite/nif/node.hpp index e05a056b79..76cc6ac687 100644 --- a/apps/openmw_test_suite/nif/node.hpp +++ b/apps/openmw_test_suite/nif/node.hpp @@ -33,8 +33,8 @@ namespace Nif::Testing inline void init(NiGeometry& value) { init(static_cast(value)); - value.data = NiGeometryDataPtr(nullptr); - value.skin = NiSkinInstancePtr(nullptr); + value.mData = NiGeometryDataPtr(nullptr); + value.mSkin = NiSkinInstancePtr(nullptr); } inline void init(NiTriShape& value) diff --git a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp index 2c2b60db36..8d456fd7ed 100644 --- a/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp +++ b/apps/openmw_test_suite/nifloader/testbulletnifloader.cpp @@ -326,20 +326,20 @@ namespace mNiTriShapeData.mVertices = { osg::Vec3f(0, 0, 0), osg::Vec3f(1, 0, 0), osg::Vec3f(1, 1, 0) }; mNiTriShapeData.mNumTriangles = 1; mNiTriShapeData.mTriangles = { 0, 1, 2 }; - mNiTriShape.data = Nif::NiGeometryDataPtr(&mNiTriShapeData); + mNiTriShape.mData = Nif::NiGeometryDataPtr(&mNiTriShapeData); mNiTriShapeData2.recType = Nif::RC_NiTriShapeData; mNiTriShapeData2.mVertices = { osg::Vec3f(0, 0, 1), osg::Vec3f(1, 0, 1), osg::Vec3f(1, 1, 1) }; mNiTriShapeData2.mNumTriangles = 1; mNiTriShapeData2.mTriangles = { 0, 1, 2 }; - mNiTriShape2.data = Nif::NiGeometryDataPtr(&mNiTriShapeData2); + mNiTriShape2.mData = Nif::NiGeometryDataPtr(&mNiTriShapeData2); mNiTriStripsData.recType = Nif::RC_NiTriStripsData; mNiTriStripsData.mVertices = { osg::Vec3f(0, 0, 0), osg::Vec3f(1, 0, 0), osg::Vec3f(1, 1, 0), osg::Vec3f(0, 1, 0) }; mNiTriStripsData.mNumTriangles = 2; mNiTriStripsData.mStrips = { { 0, 1, 2, 3 } }; - mNiTriStrips.data = Nif::NiGeometryDataPtr(&mNiTriStripsData); + mNiTriStrips.mData = Nif::NiGeometryDataPtr(&mNiTriStripsData); } }; @@ -703,7 +703,7 @@ namespace TEST_F(TestBulletNifLoader, for_tri_shape_child_node_and_filename_starting_with_x_and_not_empty_skin_should_return_static_shape) { - mNiTriShape.skin = Nif::NiSkinInstancePtr(&mNiSkinInstance); + mNiTriShape.mSkin = Nif::NiSkinInstancePtr(&mNiSkinInstance); mNiTriShape.mParents.push_back(&mNiNode); mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; @@ -943,7 +943,7 @@ namespace TEST_F(TestBulletNifLoader, for_tri_shape_child_node_with_empty_data_should_return_shape_with_null_collision_shape) { - mNiTriShape.data = Nif::NiGeometryDataPtr(nullptr); + mNiTriShape.mData = Nif::NiGeometryDataPtr(nullptr); mNiTriShape.mParents.push_back(&mNiNode); mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; @@ -961,7 +961,7 @@ namespace TEST_F(TestBulletNifLoader, for_tri_shape_child_node_with_empty_data_triangles_should_return_shape_with_null_collision_shape) { - auto data = static_cast(mNiTriShape.data.getPtr()); + auto data = static_cast(mNiTriShape.mData.getPtr()); data->mTriangles.clear(); mNiTriShape.mParents.push_back(&mNiNode); mNiNode.mChildren = Nif::NiAVObjectList{ Nif::NiAVObjectPtr(&mNiTriShape) }; @@ -1095,7 +1095,7 @@ namespace init(niTriShape); init(emptyCollisionNode); - niTriShape.data = Nif::NiGeometryDataPtr(&mNiTriShapeData); + niTriShape.mData = Nif::NiGeometryDataPtr(&mNiTriShapeData); niTriShape.mParents.push_back(&mNiNode); emptyCollisionNode.recType = Nif::RC_RootCollisionNode; @@ -1192,21 +1192,6 @@ namespace EXPECT_EQ(*result, expected); } - TEST_F(TestBulletNifLoader, should_ignore_tri_shape_data_with_mismatching_data_rec_type) - { - mNiTriShape.data = Nif::NiGeometryDataPtr(&mNiTriStripsData); - - Nif::NIFFile file("test.nif"); - file.mRoots.push_back(&mNiTriShape); - file.mHash = mHash; - - const auto result = mLoader.load(file); - - const Resource::BulletShape expected; - - EXPECT_EQ(*result, expected); - } - TEST_F(TestBulletNifLoader, for_tri_strips_root_node_should_return_static_shape) { Nif::NIFFile file("test.nif"); @@ -1227,21 +1212,6 @@ namespace EXPECT_EQ(*result, expected); } - TEST_F(TestBulletNifLoader, should_ignore_tri_strips_data_with_mismatching_data_rec_type) - { - mNiTriStrips.data = Nif::NiGeometryDataPtr(&mNiTriShapeData); - - Nif::NIFFile file("test.nif"); - file.mRoots.push_back(&mNiTriStrips); - file.mHash = mHash; - - const auto result = mLoader.load(file); - - const Resource::BulletShape expected; - - EXPECT_EQ(*result, expected); - } - TEST_F(TestBulletNifLoader, should_ignore_tri_strips_data_with_empty_strips) { mNiTriStripsData.mStrips.clear(); diff --git a/components/nif/node.cpp b/components/nif/node.cpp index cf4df3c733..18742f1ea1 100644 --- a/components/nif/node.cpp +++ b/components/nif/node.cpp @@ -167,14 +167,14 @@ namespace Nif { NiAVObject::read(nif); - data.read(nif); - skin.read(nif); + mData.read(nif); + mSkin.read(nif); mMaterial.read(nif); if (nif->getVersion() == NIFFile::NIFVersion::VER_BGS && nif->getBethVersion() > NIFFile::BethVersion::BETHVER_FO3) { - shaderprop.read(nif); - alphaprop.read(nif); + mShaderProperty.read(nif); + mAlphaProperty.read(nif); } } @@ -182,12 +182,38 @@ namespace Nif { NiAVObject::post(nif); - data.post(nif); - skin.post(nif); - shaderprop.post(nif); - alphaprop.post(nif); - if (recType != RC_NiParticles && !skin.empty()) + mData.post(nif); + mSkin.post(nif); + mShaderProperty.post(nif); + mAlphaProperty.post(nif); + if (recType != RC_NiParticles && !mSkin.empty()) nif.setUseSkinning(true); + + if (!mData.empty()) + { + switch (recType) + { + case RC_NiTriShape: + case RC_BSLODTriShape: + if (mData->recType != RC_NiTriShapeData) + mData = NiGeometryDataPtr(nullptr); + break; + case RC_NiTriStrips: + if (mData->recType != RC_NiTriStripsData) + mData = NiGeometryDataPtr(nullptr); + break; + case RC_NiParticles: + if (mData->recType != RC_NiParticlesData) + mData = NiGeometryDataPtr(nullptr); + break; + case RC_NiLines: + if (mData->recType != RC_NiLinesData) + mData = NiGeometryDataPtr(nullptr); + break; + default: + break; + } + } } void BSLODTriShape::read(NIFStream* nif) diff --git a/components/nif/node.hpp b/components/nif/node.hpp index d539298995..6e28f3b647 100644 --- a/components/nif/node.hpp +++ b/components/nif/node.hpp @@ -119,7 +119,14 @@ namespace Nif void post(Reader& nif) override; }; - struct NiGeometry : NiAVObject + struct GeometryInterface + { + NiSkinInstancePtr mSkin; + BSShaderPropertyPtr mShaderProperty; + NiAlphaPropertyPtr mAlphaProperty; + }; + + struct NiGeometry : NiAVObject, GeometryInterface { /* Possible flags: 0x40 - mesh has no vertex normals ? @@ -138,17 +145,13 @@ namespace Nif void read(NIFStream* nif); }; - NiGeometryDataPtr data; - NiSkinInstancePtr skin; + NiGeometryDataPtr mData; MaterialData mMaterial; - BSShaderPropertyPtr shaderprop; - NiAlphaPropertyPtr alphaprop; void read(NIFStream* nif) override; void post(Reader& nif) override; }; - // TODO: consider checking the data record type here struct NiTriShape : NiGeometry { }; @@ -337,13 +340,10 @@ namespace Nif void read(NIFStream* nif, uint16_t flags); }; - struct BSTriShape : NiAVObject + struct BSTriShape : NiAVObject, GeometryInterface { osg::BoundingSpheref mBoundingSphere; std::array mBoundMinMax; - NiSkinInstancePtr mSkin; - BSShaderPropertyPtr mShaderProperty; - NiAlphaPropertyPtr mAlphaProperty; BSVertexDesc mVertDesc; uint32_t mDataSize; std::vector mVertData; diff --git a/components/nifbullet/bulletnifloader.cpp b/components/nifbullet/bulletnifloader.cpp index 7d1ccc99ff..1349c1dc97 100644 --- a/components/nifbullet/bulletnifloader.cpp +++ b/components/nifbullet/bulletnifloader.cpp @@ -79,14 +79,11 @@ namespace template auto handleNiGeometry(const Nif::NiGeometry& geometry, Function&& function) - -> decltype(function(static_cast(geometry.data.get()))) + -> decltype(function(static_cast(geometry.mData.get()))) { if (geometry.recType == Nif::RC_NiTriShape || geometry.recType == Nif::RC_BSLODTriShape) { - if (geometry.data->recType != Nif::RC_NiTriShapeData) - return {}; - - auto data = static_cast(geometry.data.getPtr()); + auto data = static_cast(geometry.mData.getPtr()); if (data->mTriangles.empty()) return {}; @@ -95,10 +92,7 @@ namespace if (geometry.recType == Nif::RC_NiTriStrips) { - if (geometry.data->recType != Nif::RC_NiTriStripsData) - return {}; - - auto data = static_cast(geometry.data.getPtr()); + auto data = static_cast(geometry.mData.getPtr()); if (data->mStrips.empty()) return {}; @@ -366,10 +360,10 @@ namespace NifBullet if (args.mHasMarkers && Misc::StringUtils::ciStartsWith(niGeometry.mName, "EditorMarker")) return; - if (niGeometry.data.empty() || niGeometry.data->mVertices.empty()) + if (niGeometry.mData.empty() || niGeometry.mData->mVertices.empty()) return; - if (!niGeometry.skin.empty()) + if (!niGeometry.mSkin.empty()) args.mAnimated = false; // TODO: handle NiSkinPartition diff --git a/components/nifosg/nifloader.cpp b/components/nifosg/nifloader.cpp index 23a86ba983..05f42db05a 100644 --- a/components/nifosg/nifloader.cpp +++ b/components/nifosg/nifloader.cpp @@ -128,10 +128,10 @@ namespace auto geometry = dynamic_cast(nifNode); if (geometry) { - if (!geometry->shaderprop.empty()) - out.emplace_back(geometry->shaderprop.getPtr()); - if (!geometry->alphaprop.empty()) - out.emplace_back(geometry->alphaprop.getPtr()); + if (!geometry->mShaderProperty.empty()) + out.emplace_back(geometry->mShaderProperty.getPtr()); + if (!geometry->mAlphaProperty.empty()) + out.emplace_back(geometry->mAlphaProperty.getPtr()); } } @@ -444,8 +444,8 @@ namespace NifOsg auto geometry = dynamic_cast(nifNode); // NiGeometry's NiAlphaProperty doesn't get handled here because it's a drawable property - if (geometry && !geometry->shaderprop.empty()) - handleProperty(geometry->shaderprop.getPtr(), applyTo, composite, imageManager, boundTextures, + if (geometry && !geometry->mShaderProperty.empty()) + handleProperty(geometry->mShaderProperty.getPtr(), applyTo, composite, imageManager, boundTextures, animflags, hasStencilProperty); } @@ -761,7 +761,7 @@ namespace NifOsg skip = args.mHasMarkers && Misc::StringUtils::ciStartsWith(nifNode->mName, "EditorMarker"); if (!skip) { - Nif::NiSkinInstancePtr skin = static_cast(nifNode)->skin; + Nif::NiSkinInstancePtr skin = static_cast(nifNode)->mSkin; if (skin.empty()) handleGeometry(nifNode, parent, node, composite, args.mBoundTextures, args.mAnimFlags); @@ -1121,13 +1121,13 @@ namespace NifOsg const Nif::NiAVObject* nifNode, ParticleSystem* partsys, const Nif::NiParticleSystemController* partctrl) { auto particleNode = static_cast(nifNode); - if (particleNode->data.empty() || particleNode->data->recType != Nif::RC_NiParticlesData) + if (particleNode->mData.empty()) { partsys->setQuota(partctrl->mParticles.size()); return; } - auto particledata = static_cast(particleNode->data.getPtr()); + auto particledata = static_cast(particleNode->mData.getPtr()); partsys->setQuota(particledata->mNumParticles); osg::BoundingBox box; @@ -1379,13 +1379,13 @@ namespace NifOsg const std::vector& boundTextures, int animflags) { const Nif::NiGeometry* niGeometry = static_cast(nifNode); - if (niGeometry->data.empty()) + if (niGeometry->mData.empty()) return; bool hasPartitions = false; - if (!niGeometry->skin.empty()) + if (!niGeometry->mSkin.empty()) { - const Nif::NiSkinInstance* skin = niGeometry->skin.getPtr(); + const Nif::NiSkinInstance* skin = niGeometry->mSkin.getPtr(); const Nif::NiSkinData* data = nullptr; const Nif::NiSkinPartition* partitions = nullptr; if (!skin->mData.empty()) @@ -1419,13 +1419,11 @@ namespace NifOsg } } - const Nif::NiGeometryData* niGeometryData = niGeometry->data.getPtr(); + const Nif::NiGeometryData* niGeometryData = niGeometry->mData.getPtr(); if (!hasPartitions) { if (niGeometry->recType == Nif::RC_NiTriShape || nifNode->recType == Nif::RC_BSLODTriShape) { - if (niGeometryData->recType != Nif::RC_NiTriShapeData) - return; auto data = static_cast(niGeometryData); const std::vector& triangles = data->mTriangles; if (triangles.empty()) @@ -1435,8 +1433,6 @@ namespace NifOsg } else if (niGeometry->recType == Nif::RC_NiTriStrips) { - if (niGeometryData->recType != Nif::RC_NiTriStripsData) - return; auto data = static_cast(niGeometryData); bool hasGeometry = false; for (const std::vector& strip : data->mStrips) @@ -1452,8 +1448,6 @@ namespace NifOsg } else if (niGeometry->recType == Nif::RC_NiLines) { - if (niGeometryData->recType != Nif::RC_NiLinesData) - return; auto data = static_cast(niGeometryData); const auto& line = data->mLines; if (line.empty()) @@ -1545,7 +1539,7 @@ namespace NifOsg // Assign bone weights osg::ref_ptr map(new SceneUtil::RigGeometry::InfluenceMap); - const Nif::NiSkinInstance* skin = static_cast(nifNode)->skin.getPtr(); + const Nif::NiSkinInstance* skin = static_cast(nifNode)->mSkin.getPtr(); const Nif::NiSkinData* data = skin->mData.getPtr(); const Nif::NiAVObjectList& bones = skin->mBones; for (std::size_t i = 0; i < bones.size(); ++i)