diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index d9abe242e..50882268d 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -268,7 +268,10 @@ Containers::Optional AbstractImporter::animation(const UnsignedIn CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::animation(): no file opened", {}); CORRADE_ASSERT(id < doAnimationCount(), "Trade::AbstractImporter::animation(): index" << id << "out of range for" << doAnimationCount() << "entries", {}); Containers::Optional animation = doAnimation(id); - CORRADE_ASSERT(!animation || (!animation->_data.deleter() && !animation->_tracks.deleter()), "Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter", {}); + CORRADE_ASSERT(!animation || + ((!animation->_data.deleter() || animation->_data.deleter() == Implementation::nonOwnedArrayDeleter) && + (!animation->_tracks.deleter() || animation->_tracks.deleter() == reinterpret_cast(Implementation::nonOwnedArrayDeleter))), + "Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter", {}); return animation; } @@ -430,7 +433,11 @@ Containers::Optional AbstractImporter::mesh(const UnsignedInt id) { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::mesh(): no file opened", {}); CORRADE_ASSERT(id < doMeshCount(), "Trade::AbstractImporter::mesh(): index" << id << "out of range for" << doMeshCount() << "entries", {}); Containers::Optional mesh = doMesh(id); - CORRADE_ASSERT(!mesh || (!mesh->_indexData.deleter() && !mesh->_vertexData.deleter() && !mesh->_attributes.deleter()), "Trade::AbstractImporter::mesh(): implementation is not allowed to use a custom Array deleter", {}); + CORRADE_ASSERT(!mesh || ( + (!mesh->_indexData.deleter() || mesh->_indexData.deleter() == Implementation::nonOwnedArrayDeleter) && + (!mesh->_vertexData.deleter() || mesh->_vertexData.deleter() == Implementation::nonOwnedArrayDeleter) && + (!mesh->_attributes.deleter() || mesh->_attributes.deleter() == reinterpret_cast(Implementation::nonOwnedArrayDeleter))), + "Trade::AbstractImporter::mesh(): implementation is not allowed to use a custom Array deleter", {}); return mesh; } @@ -633,7 +640,7 @@ Containers::Optional AbstractImporter::image1D(const UnsignedInt id } #endif Containers::Optional image = doImage1D(id, level); - CORRADE_ASSERT(!image || !image->_data.deleter(), "Trade::AbstractImporter::image1D(): implementation is not allowed to use a custom Array deleter", {}); + CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter, "Trade::AbstractImporter::image1D(): implementation is not allowed to use a custom Array deleter", {}); return image; } @@ -689,7 +696,7 @@ Containers::Optional AbstractImporter::image2D(const UnsignedInt id } #endif Containers::Optional image = doImage2D(id, level); - CORRADE_ASSERT(!image || !image->_data.deleter(), "Trade::AbstractImporter::image2D(): implementation is not allowed to use a custom Array deleter", {}); + CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter, "Trade::AbstractImporter::image2D(): implementation is not allowed to use a custom Array deleter", {}); return image; } @@ -745,7 +752,7 @@ Containers::Optional AbstractImporter::image3D(const UnsignedInt id } #endif Containers::Optional image = doImage3D(id, level); - CORRADE_ASSERT(!image || !image->_data.deleter(), "Trade::AbstractImporter::image3D(): implementation is not allowed to use a custom Array deleter", {}); + CORRADE_ASSERT(!image || !image->_data.deleter() || image->_data.deleter() == Implementation::nonOwnedArrayDeleter, "Trade::AbstractImporter::image3D(): implementation is not allowed to use a custom Array deleter", {}); return image; } diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index 024c7ffaa..212c73575 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -194,9 +194,11 @@ dependency on the importer instance and neither on the dynamic plugin module. In other words, you don't need to keep the importer instance (or the plugin manager instance) around in order to have the `*Data` instances valid. Moreover, all @ref Corrade::Containers::Array instances returned through -@ref ImageData, @ref AnimationData and others are only allowed to have default -deleters --- this is to avoid potential dangling function pointer calls when -destructing such instances after the plugin module has been unloaded. +@ref ImageData, @ref AnimationData and @ref MeshData are only allowed to have +default deleters (or be non-owning instances created from +@ref Corrade::Containers::ArrayView) --- this is to avoid potential dangling +function pointer calls when destructing such instances after the plugin module +has been unloaded. The only exception are various `importerState()` functions @ref Trade-AbstractImporter-usage-state "described above", but in that case the diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 9f44d98d7..0e5c7ebc8 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -106,6 +106,7 @@ struct AbstractImporterTest: TestSuite::Tester { void animationNotImplemented(); void animationNoFile(); void animationOutOfRange(); + void animationNonOwningDeleters(); void animationCustomDataDeleter(); void animationCustomTrackDeleter(); @@ -168,6 +169,7 @@ struct AbstractImporterTest: TestSuite::Tester { void meshNotImplemented(); void meshNoFile(); void meshOutOfRange(); + void meshNonOwningDeleters(); void meshCustomIndexDataDeleter(); void meshCustomVertexDataDeleter(); void meshCustomAttributesDeleter(); @@ -240,6 +242,7 @@ struct AbstractImporterTest: TestSuite::Tester { void image1DNoFile(); void image1DOutOfRange(); void image1DLevelOutOfRange(); + void image1DNonOwningDeleter(); void image1DCustomDeleter(); void image2D(); @@ -258,6 +261,7 @@ struct AbstractImporterTest: TestSuite::Tester { void image2DNoFile(); void image2DOutOfRange(); void image2DLevelOutOfRange(); + void image2DNonOwningDeleter(); void image2DCustomDeleter(); void image3D(); @@ -276,6 +280,7 @@ struct AbstractImporterTest: TestSuite::Tester { void image3DNoFile(); void image3DOutOfRange(); void image3DLevelOutOfRange(); + void image3DNonOwningDeleter(); void image3DCustomDeleter(); void importerState(); @@ -340,6 +345,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::animationNotImplemented, &AbstractImporterTest::animationNoFile, &AbstractImporterTest::animationOutOfRange, + &AbstractImporterTest::animationNonOwningDeleters, &AbstractImporterTest::animationCustomDataDeleter, &AbstractImporterTest::animationCustomTrackDeleter, @@ -402,6 +408,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::meshNotImplemented, &AbstractImporterTest::meshNoFile, &AbstractImporterTest::meshOutOfRange, + &AbstractImporterTest::meshNonOwningDeleters, &AbstractImporterTest::meshCustomIndexDataDeleter, &AbstractImporterTest::meshCustomVertexDataDeleter, &AbstractImporterTest::meshCustomAttributesDeleter, @@ -474,6 +481,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::image1DNoFile, &AbstractImporterTest::image1DOutOfRange, &AbstractImporterTest::image1DLevelOutOfRange, + &AbstractImporterTest::image1DNonOwningDeleter, &AbstractImporterTest::image1DCustomDeleter, &AbstractImporterTest::image2D, @@ -492,6 +500,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::image2DNoFile, &AbstractImporterTest::image2DOutOfRange, &AbstractImporterTest::image2DLevelOutOfRange, + &AbstractImporterTest::image2DNonOwningDeleter, &AbstractImporterTest::image2DCustomDeleter, &AbstractImporterTest::image3D, @@ -510,6 +519,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::image3DNoFile, &AbstractImporterTest::image3DOutOfRange, &AbstractImporterTest::image3DLevelOutOfRange, + &AbstractImporterTest::image3DNonOwningDeleter, &AbstractImporterTest::image3DCustomDeleter, &AbstractImporterTest::importerState, @@ -1368,6 +1378,28 @@ void AbstractImporterTest::animationOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::animation(): index 8 out of range for 8 entries\n"); } +void AbstractImporterTest::animationNonOwningDeleters() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doAnimationCount() const override { return 1; } + Containers::Optional doAnimation(UnsignedInt) override { + return AnimationData{Containers::Array{data, 1, Implementation::nonOwnedArrayDeleter}, + Containers::Array{&track, 1, + reinterpret_cast(Implementation::nonOwnedArrayDeleter)}}; + } + + char data[1]; + AnimationTrackData track; + } importer; + + auto data = importer.animation(0); + CORRADE_VERIFY(data); + CORRADE_COMPARE(static_cast(data->data()), importer.data); +} + void AbstractImporterTest::animationCustomDataDeleter() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return {}; } @@ -2238,6 +2270,31 @@ void AbstractImporterTest::meshOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::mesh(): index 8 out of range for 8 entries\n"); } +void AbstractImporterTest::meshNonOwningDeleters() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doMeshCount() const override { return 1; } + Containers::Optional doMesh(UnsignedInt) override { + return MeshData{MeshPrimitive::Triangles, + Containers::Array{indexData, 1, Implementation::nonOwnedArrayDeleter}, MeshIndexData{MeshIndexType::UnsignedByte, indexData}, + Containers::Array{nullptr, 0, Implementation::nonOwnedArrayDeleter}, + meshAttributeDataNonOwningArray(attributes)}; + } + + char indexData[1]; + MeshAttributeData attributes[1]{ + MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector3, nullptr} + }; + } importer; + + auto data = importer.mesh(0); + CORRADE_VERIFY(data); + CORRADE_COMPARE(static_cast(data->indexData()), importer.indexData); +} + void AbstractImporterTest::meshCustomIndexDataDeleter() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return {}; } @@ -3263,6 +3320,25 @@ void AbstractImporterTest::image1DLevelOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image1D(): level 3 out of range for 3 entries\n"); } +void AbstractImporterTest::image1DNonOwningDeleter() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doImage1DCount() const override { return 1; } + Containers::Optional doImage1D(UnsignedInt, UnsignedInt) override { + return ImageData1D{PixelFormat::RGBA8Unorm, {}, Containers::Array{data, 1, Implementation::nonOwnedArrayDeleter}}; + } + + char data[1]; + } importer; + + auto data = importer.image1D(0); + CORRADE_VERIFY(data); + CORRADE_COMPARE(static_cast(data->data()), importer.data); +} + void AbstractImporterTest::image1DCustomDeleter() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return {}; } @@ -3530,6 +3606,25 @@ void AbstractImporterTest::image2DLevelOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image2D(): level 3 out of range for 3 entries\n"); } +void AbstractImporterTest::image2DNonOwningDeleter() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doImage2DCount() const override { return 1; } + Containers::Optional doImage2D(UnsignedInt, UnsignedInt) override { + return ImageData2D{PixelFormat::RGBA8Unorm, {}, Containers::Array{data, 1, Implementation::nonOwnedArrayDeleter}}; + } + + char data[1]; + } importer; + + auto data = importer.image2D(0); + CORRADE_VERIFY(data); + CORRADE_COMPARE(static_cast(data->data()), importer.data); +} + void AbstractImporterTest::image2DCustomDeleter() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return {}; } @@ -3798,6 +3893,25 @@ void AbstractImporterTest::image3DLevelOutOfRange() { CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::image3D(): level 3 out of range for 3 entries\n"); } +void AbstractImporterTest::image3DNonOwningDeleter() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doImage3DCount() const override { return 1; } + Containers::Optional doImage3D(UnsignedInt, UnsignedInt) override { + return ImageData3D{PixelFormat::RGBA8Unorm, {}, Containers::Array{data, 1, Implementation::nonOwnedArrayDeleter}}; + } + + char data[1]; + } importer; + + auto data = importer.image3D(0); + CORRADE_VERIFY(data); + CORRADE_COMPARE(static_cast(data->data()), importer.data); +} + void AbstractImporterTest::image3DCustomDeleter() { struct: AbstractImporter { ImporterFeatures doFeatures() const override { return {}; }