Browse Source

Trade: allow non-owning aray deleters passed through Importer APIs.

pull/371/head
Vladimír Vondruš 7 years ago
parent
commit
4011e3006d
  1. 17
      src/Magnum/Trade/AbstractImporter.cpp
  2. 8
      src/Magnum/Trade/AbstractImporter.h
  3. 114
      src/Magnum/Trade/Test/AbstractImporterTest.cpp

17
src/Magnum/Trade/AbstractImporter.cpp

@ -268,7 +268,10 @@ Containers::Optional<AnimationData> AbstractImporter::animation(const UnsignedIn
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::animation(): no file opened", {}); CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::animation(): no file opened", {});
CORRADE_ASSERT(id < doAnimationCount(), "Trade::AbstractImporter::animation(): index" << id << "out of range for" << doAnimationCount() << "entries", {}); CORRADE_ASSERT(id < doAnimationCount(), "Trade::AbstractImporter::animation(): index" << id << "out of range for" << doAnimationCount() << "entries", {});
Containers::Optional<AnimationData> animation = doAnimation(id); Containers::Optional<AnimationData> 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<void(*)(AnimationTrackData*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
"Trade::AbstractImporter::animation(): implementation is not allowed to use a custom Array deleter", {});
return animation; return animation;
} }
@ -430,7 +433,11 @@ Containers::Optional<MeshData> AbstractImporter::mesh(const UnsignedInt id) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::mesh(): no file opened", {}); CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::mesh(): no file opened", {});
CORRADE_ASSERT(id < doMeshCount(), "Trade::AbstractImporter::mesh(): index" << id << "out of range for" << doMeshCount() << "entries", {}); CORRADE_ASSERT(id < doMeshCount(), "Trade::AbstractImporter::mesh(): index" << id << "out of range for" << doMeshCount() << "entries", {});
Containers::Optional<MeshData> mesh = doMesh(id); Containers::Optional<MeshData> 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<void(*)(MeshAttributeData*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
"Trade::AbstractImporter::mesh(): implementation is not allowed to use a custom Array deleter", {});
return mesh; return mesh;
} }
@ -633,7 +640,7 @@ Containers::Optional<ImageData1D> AbstractImporter::image1D(const UnsignedInt id
} }
#endif #endif
Containers::Optional<ImageData1D> image = doImage1D(id, level); Containers::Optional<ImageData1D> 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; return image;
} }
@ -689,7 +696,7 @@ Containers::Optional<ImageData2D> AbstractImporter::image2D(const UnsignedInt id
} }
#endif #endif
Containers::Optional<ImageData2D> image = doImage2D(id, level); Containers::Optional<ImageData2D> 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; return image;
} }
@ -745,7 +752,7 @@ Containers::Optional<ImageData3D> AbstractImporter::image3D(const UnsignedInt id
} }
#endif #endif
Containers::Optional<ImageData3D> image = doImage3D(id, level); Containers::Optional<ImageData3D> 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; return image;
} }

8
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 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. manager instance) around in order to have the `*Data` instances valid.
Moreover, all @ref Corrade::Containers::Array instances returned through Moreover, all @ref Corrade::Containers::Array instances returned through
@ref ImageData, @ref AnimationData and others are only allowed to have default @ref ImageData, @ref AnimationData and @ref MeshData are only allowed to have
deleters --- this is to avoid potential dangling function pointer calls when default deleters (or be non-owning instances created from
destructing such instances after the plugin module has been unloaded. @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 The only exception are various `importerState()` functions
@ref Trade-AbstractImporter-usage-state "described above", but in that case the @ref Trade-AbstractImporter-usage-state "described above", but in that case the

114
src/Magnum/Trade/Test/AbstractImporterTest.cpp

@ -106,6 +106,7 @@ struct AbstractImporterTest: TestSuite::Tester {
void animationNotImplemented(); void animationNotImplemented();
void animationNoFile(); void animationNoFile();
void animationOutOfRange(); void animationOutOfRange();
void animationNonOwningDeleters();
void animationCustomDataDeleter(); void animationCustomDataDeleter();
void animationCustomTrackDeleter(); void animationCustomTrackDeleter();
@ -168,6 +169,7 @@ struct AbstractImporterTest: TestSuite::Tester {
void meshNotImplemented(); void meshNotImplemented();
void meshNoFile(); void meshNoFile();
void meshOutOfRange(); void meshOutOfRange();
void meshNonOwningDeleters();
void meshCustomIndexDataDeleter(); void meshCustomIndexDataDeleter();
void meshCustomVertexDataDeleter(); void meshCustomVertexDataDeleter();
void meshCustomAttributesDeleter(); void meshCustomAttributesDeleter();
@ -240,6 +242,7 @@ struct AbstractImporterTest: TestSuite::Tester {
void image1DNoFile(); void image1DNoFile();
void image1DOutOfRange(); void image1DOutOfRange();
void image1DLevelOutOfRange(); void image1DLevelOutOfRange();
void image1DNonOwningDeleter();
void image1DCustomDeleter(); void image1DCustomDeleter();
void image2D(); void image2D();
@ -258,6 +261,7 @@ struct AbstractImporterTest: TestSuite::Tester {
void image2DNoFile(); void image2DNoFile();
void image2DOutOfRange(); void image2DOutOfRange();
void image2DLevelOutOfRange(); void image2DLevelOutOfRange();
void image2DNonOwningDeleter();
void image2DCustomDeleter(); void image2DCustomDeleter();
void image3D(); void image3D();
@ -276,6 +280,7 @@ struct AbstractImporterTest: TestSuite::Tester {
void image3DNoFile(); void image3DNoFile();
void image3DOutOfRange(); void image3DOutOfRange();
void image3DLevelOutOfRange(); void image3DLevelOutOfRange();
void image3DNonOwningDeleter();
void image3DCustomDeleter(); void image3DCustomDeleter();
void importerState(); void importerState();
@ -340,6 +345,7 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::animationNotImplemented, &AbstractImporterTest::animationNotImplemented,
&AbstractImporterTest::animationNoFile, &AbstractImporterTest::animationNoFile,
&AbstractImporterTest::animationOutOfRange, &AbstractImporterTest::animationOutOfRange,
&AbstractImporterTest::animationNonOwningDeleters,
&AbstractImporterTest::animationCustomDataDeleter, &AbstractImporterTest::animationCustomDataDeleter,
&AbstractImporterTest::animationCustomTrackDeleter, &AbstractImporterTest::animationCustomTrackDeleter,
@ -402,6 +408,7 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::meshNotImplemented, &AbstractImporterTest::meshNotImplemented,
&AbstractImporterTest::meshNoFile, &AbstractImporterTest::meshNoFile,
&AbstractImporterTest::meshOutOfRange, &AbstractImporterTest::meshOutOfRange,
&AbstractImporterTest::meshNonOwningDeleters,
&AbstractImporterTest::meshCustomIndexDataDeleter, &AbstractImporterTest::meshCustomIndexDataDeleter,
&AbstractImporterTest::meshCustomVertexDataDeleter, &AbstractImporterTest::meshCustomVertexDataDeleter,
&AbstractImporterTest::meshCustomAttributesDeleter, &AbstractImporterTest::meshCustomAttributesDeleter,
@ -474,6 +481,7 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::image1DNoFile, &AbstractImporterTest::image1DNoFile,
&AbstractImporterTest::image1DOutOfRange, &AbstractImporterTest::image1DOutOfRange,
&AbstractImporterTest::image1DLevelOutOfRange, &AbstractImporterTest::image1DLevelOutOfRange,
&AbstractImporterTest::image1DNonOwningDeleter,
&AbstractImporterTest::image1DCustomDeleter, &AbstractImporterTest::image1DCustomDeleter,
&AbstractImporterTest::image2D, &AbstractImporterTest::image2D,
@ -492,6 +500,7 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::image2DNoFile, &AbstractImporterTest::image2DNoFile,
&AbstractImporterTest::image2DOutOfRange, &AbstractImporterTest::image2DOutOfRange,
&AbstractImporterTest::image2DLevelOutOfRange, &AbstractImporterTest::image2DLevelOutOfRange,
&AbstractImporterTest::image2DNonOwningDeleter,
&AbstractImporterTest::image2DCustomDeleter, &AbstractImporterTest::image2DCustomDeleter,
&AbstractImporterTest::image3D, &AbstractImporterTest::image3D,
@ -510,6 +519,7 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::image3DNoFile, &AbstractImporterTest::image3DNoFile,
&AbstractImporterTest::image3DOutOfRange, &AbstractImporterTest::image3DOutOfRange,
&AbstractImporterTest::image3DLevelOutOfRange, &AbstractImporterTest::image3DLevelOutOfRange,
&AbstractImporterTest::image3DNonOwningDeleter,
&AbstractImporterTest::image3DCustomDeleter, &AbstractImporterTest::image3DCustomDeleter,
&AbstractImporterTest::importerState, &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"); 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<AnimationData> doAnimation(UnsignedInt) override {
return AnimationData{Containers::Array<char>{data, 1, Implementation::nonOwnedArrayDeleter},
Containers::Array<AnimationTrackData>{&track, 1,
reinterpret_cast<void(*)(AnimationTrackData*, std::size_t)>(Implementation::nonOwnedArrayDeleter)}};
}
char data[1];
AnimationTrackData track;
} importer;
auto data = importer.animation(0);
CORRADE_VERIFY(data);
CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
}
void AbstractImporterTest::animationCustomDataDeleter() { void AbstractImporterTest::animationCustomDataDeleter() {
struct: AbstractImporter { struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; } 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"); 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<MeshData> doMesh(UnsignedInt) override {
return MeshData{MeshPrimitive::Triangles,
Containers::Array<char>{indexData, 1, Implementation::nonOwnedArrayDeleter}, MeshIndexData{MeshIndexType::UnsignedByte, indexData},
Containers::Array<char>{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<const void*>(data->indexData()), importer.indexData);
}
void AbstractImporterTest::meshCustomIndexDataDeleter() { void AbstractImporterTest::meshCustomIndexDataDeleter() {
struct: AbstractImporter { struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; } 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"); 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<ImageData1D> doImage1D(UnsignedInt, UnsignedInt) override {
return ImageData1D{PixelFormat::RGBA8Unorm, {}, Containers::Array<char>{data, 1, Implementation::nonOwnedArrayDeleter}};
}
char data[1];
} importer;
auto data = importer.image1D(0);
CORRADE_VERIFY(data);
CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
}
void AbstractImporterTest::image1DCustomDeleter() { void AbstractImporterTest::image1DCustomDeleter() {
struct: AbstractImporter { struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; } 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"); 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<ImageData2D> doImage2D(UnsignedInt, UnsignedInt) override {
return ImageData2D{PixelFormat::RGBA8Unorm, {}, Containers::Array<char>{data, 1, Implementation::nonOwnedArrayDeleter}};
}
char data[1];
} importer;
auto data = importer.image2D(0);
CORRADE_VERIFY(data);
CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
}
void AbstractImporterTest::image2DCustomDeleter() { void AbstractImporterTest::image2DCustomDeleter() {
struct: AbstractImporter { struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; } 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"); 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<ImageData3D> doImage3D(UnsignedInt, UnsignedInt) override {
return ImageData3D{PixelFormat::RGBA8Unorm, {}, Containers::Array<char>{data, 1, Implementation::nonOwnedArrayDeleter}};
}
char data[1];
} importer;
auto data = importer.image3D(0);
CORRADE_VERIFY(data);
CORRADE_COMPARE(static_cast<const void*>(data->data()), importer.data);
}
void AbstractImporterTest::image3DCustomDeleter() { void AbstractImporterTest::image3DCustomDeleter() {
struct: AbstractImporter { struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; } ImporterFeatures doFeatures() const override { return {}; }

Loading…
Cancel
Save