ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

Trade: check for custom MaterialData deleters in AbstractImporter. 

And adapt MaterialData and the deprecated PhongMaterialData constructor
to follow these rules. Basically the same as done for ImageData and
MeshData.
pull/459/head
Vladimír Vondruš 6 years ago
parent
0ec8e002fb
commit
b5fdfc2474
5 changed files with 103 additions and 1 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 4
      src/Magnum/Trade/AbstractImporter.cpp
  2. 3
      src/Magnum/Trade/MaterialData.cpp
  3. 5
      src/Magnum/Trade/MaterialData.h
  4. 3
      src/Magnum/Trade/PhongMaterialData.cpp
  5. 89
      src/Magnum/Trade/Test/AbstractImporterTest.cpp

4
src/Magnum/Trade/AbstractImporter.cpp
Unescape View File

@ -681,6 +681,10 @@ AbstractImporter::material(const UnsignedInt id) {
CORRADE_ASSERT(id < doMaterialCount(), "Trade::AbstractImporter::material(): index" << id << "out of range for" << doMaterialCount() << "entries", {});
Containers::Optional<MaterialData> material = doMaterial(id);
CORRADE_ASSERT(!material || (
(!material->_data.deleter() || material->_data.deleter() == reinterpret_cast<void(*)(MaterialAttributeData*, std::size_t)>(Implementation::nonOwnedArrayDeleter)) &&
(!material->_layerOffsets.deleter() || material->_layerOffsets.deleter() == reinterpret_cast<void(*)(UnsignedInt*, std::size_t)>(Implementation::nonOwnedArrayDeleter))),
"Trade::AbstractImporter::material(): implementation is not allowed to use a custom Array deleter", {});
/* GCC 4.8 and clang-cl needs an explicit conversion here */
#ifdef MAGNUM_BUILD_DEPRECATED

3
src/Magnum/Trade/MaterialData.cpp
Unescape View File

@ -32,6 +32,7 @@
#include "Magnum/Math/Vector4.h"
#include "Magnum/Math/Matrix.h"
#include "Magnum/Trade/Data.h"
#include "Magnum/Trade/Implementation/arrayUtilities.h"
namespace Magnum { namespace Trade {
@ -226,7 +227,7 @@ MaterialData::MaterialData(const MaterialTypes types, Containers::Array<Material
MaterialData::MaterialData(const MaterialTypes types, const std::initializer_list<MaterialAttributeData> attributeData, const std::initializer_list<UnsignedInt> layerData, const void* const importerState): MaterialData{types, Implementation::initializerListToArrayWithDefaultDeleter(attributeData), Implementation::initializerListToArrayWithDefaultDeleter(layerData), importerState} {}
MaterialData::MaterialData(const MaterialTypes types, DataFlags, const Containers::ArrayView<const MaterialAttributeData> attributeData, DataFlags, Containers::ArrayView<const UnsignedInt> layerData, const void* const importerState) noexcept: _data{Containers::Array<MaterialAttributeData>{const_cast<MaterialAttributeData*>(attributeData.data()), attributeData.size(), [](MaterialAttributeData*, std::size_t){}}}, _layerOffsets{Containers::Array<UnsignedInt>{const_cast<UnsignedInt*>(layerData.data()), layerData.size(), [](UnsignedInt*, std::size_t){}}}, _types{types}, _importerState{importerState} {
MaterialData::MaterialData(const MaterialTypes types, DataFlags, const Containers::ArrayView<const MaterialAttributeData> attributeData, DataFlags, Containers::ArrayView<const UnsignedInt> layerData, const void* const importerState) noexcept: _data{Containers::Array<MaterialAttributeData>{const_cast<MaterialAttributeData*>(attributeData.data()), attributeData.size(), reinterpret_cast<void(*)(MaterialAttributeData*, std::size_t)>(Implementation::nonOwnedArrayDeleter)}}, _layerOffsets{Containers::Array<UnsignedInt>{const_cast<UnsignedInt*>(layerData.data()), layerData.size(), reinterpret_cast<void(*)(UnsignedInt*, std::size_t)>(Implementation::nonOwnedArrayDeleter)}}, _types{types}, _importerState{importerState} {
#ifndef CORRADE_NO_ASSERT
/* Not checking what's already done in MaterialAttributeData constructor */
for(std::size_t i = 0; i != _data.size(); ++i)

5
src/Magnum/Trade/MaterialData.h
Unescape View File

@ -1498,6 +1498,11 @@ class MAGNUM_TRADE_EXPORT MaterialData {
const void* importerState() const { return _importerState; }
private:
/* For custom deleter checks. Not done in the constructors here because
the restriction is pointless when used outside of plugin
implementations. */
friend AbstractImporter;
static Containers::StringView attributeString(MaterialAttribute name);
/* Internal helpers that don't assert, unlike layerId() / attributeId() */
UnsignedInt layerFor(Containers::StringView layer) const;

3
src/Magnum/Trade/PhongMaterialData.cpp
Unescape View File

@ -91,6 +91,9 @@ PhongMaterialData::PhongMaterialData(const Flags flags, const Color4& ambientCol
arrayAppend(data, Containers::InPlaceInit, MaterialAttribute::Shininess, shininess);
/* Convert back to a non-growable Array as importers don't allow custom
deleters in plugin implementations */
arrayShrink(data, Containers::DefaultInit);
return data;
}(), importerState} {
/* The data can't be filled here because it won't be sorted correctly */

89
src/Magnum/Trade/Test/AbstractImporterTest.cpp
Unescape View File

@ -197,6 +197,9 @@ struct AbstractImporterTest: TestSuite::Tester {
void materialNameOutOfRange();
void materialNotImplemented();
void materialOutOfRange();
void materialNonOwningDeleters();
void materialCustomAttributeDataDeleter();
void materialCustomLayerDataDeleter();
void texture();
void textureNameNotImplemented();
@ -402,6 +405,9 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::materialNameOutOfRange,
&AbstractImporterTest::materialNotImplemented,
&AbstractImporterTest::materialOutOfRange,
&AbstractImporterTest::materialNonOwningDeleters,
&AbstractImporterTest::materialCustomAttributeDataDeleter,
&AbstractImporterTest::materialCustomLayerDataDeleter,
&AbstractImporterTest::texture,
&AbstractImporterTest::textureNameNotImplemented,
@ -3131,6 +3137,89 @@ void AbstractImporterTest::materialOutOfRange() {
CORRADE_COMPARE(out.str(), "Trade::AbstractImporter::material(): index 8 out of range for 8 entries\n");
}
void AbstractImporterTest::materialNonOwningDeleters() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doMaterialCount() const override { return 1; }
Containers::Optional<MaterialData> doMaterial(UnsignedInt) override {
return MaterialData{{}, {}, attributeData, {}, layerData};
}
UnsignedInt layerData[1]{};
MaterialAttributeData attributeData[1]{
{MaterialAttribute::DiffuseColor, Color4{}}
};
} importer;
auto data = importer.material(0);
CORRADE_VERIFY(data);
CORRADE_COMPARE(static_cast<const void*>(data->attributeData()), importer.attributeData);
CORRADE_COMPARE(static_cast<const void*>(data->layerData()), importer.layerData);
}
void AbstractImporterTest::materialCustomAttributeDataDeleter() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doMaterialCount() const override { return 1; }
Int doMaterialForName(const std::string&) override { return 0; }
Containers::Optional<MaterialData> doMaterial(UnsignedInt) override {
return MaterialData{{}, Containers::Array<MaterialAttributeData>{attributeData, 1, [](MaterialAttributeData*, std::size_t) {}}};
}
MaterialAttributeData attributeData[1]{
{MaterialAttribute::DiffuseColor, Color4{}}
};
} importer;
std::ostringstream out;
Error redirectError{&out};
importer.material(0);
importer.material("");
CORRADE_COMPARE(out.str(),
"Trade::AbstractImporter::material(): implementation is not allowed to use a custom Array deleter\n"
"Trade::AbstractImporter::material(): implementation is not allowed to use a custom Array deleter\n");
}
void AbstractImporterTest::materialCustomLayerDataDeleter() {
#ifdef CORRADE_NO_ASSERT
CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
#endif
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
UnsignedInt doMaterialCount() const override { return 1; }
Int doMaterialForName(const std::string&) override { return 0; }
Containers::Optional<MaterialData> doMaterial(UnsignedInt) override {
return MaterialData{{}, nullptr, Containers::Array<UnsignedInt>{layerData, 1, [](UnsignedInt*, std::size_t) {}}};
}
UnsignedInt layerData[1]{};
} importer;
std::ostringstream out;
Error redirectError{&out};
importer.material(0);
importer.material("");
CORRADE_COMPARE(out.str(),
"Trade::AbstractImporter::material(): implementation is not allowed to use a custom Array deleter\n"
"Trade::AbstractImporter::material(): implementation is not allowed to use a custom Array deleter\n");
}
void AbstractImporterTest::texture() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }

Write Preview
Loading…
Cancel
Save