From 30bd1cd3b6924ac8608b8d8d90cbc34fbaa9f50c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 26 Sep 2022 16:46:33 +0200 Subject: [PATCH] Trade: add MaterialDataType::Buffer. Because storing arbitrary data as a string was not good: - It *never* followed alignment requirements due to the last byte being used for size. Instead the size is now stored before the data, and thus the data is always on the 64 byte boundary. - As it could contain arbitrary binary data, it could cause magnum-sceneconverter --material-info to print garbage, corrupt the terminal or, worst case, crash. Not good. - It stored an implicit \0, which was unnecessary. --- doc/snippets/MagnumTrade.cpp | 5 +- .../Implementation/sceneConverterUtilities.h | 3 + .../SceneTools/Test/SceneConverterTest.cpp | 1 + .../info-materials.txt | 1 + src/Magnum/Trade/MaterialData.cpp | 97 ++++++++- src/Magnum/Trade/MaterialData.h | 196 ++++++++++++------ src/Magnum/Trade/MaterialLayerData.h | 6 +- src/Magnum/Trade/Test/MaterialDataTest.cpp | 194 ++++++++++++++++- 8 files changed, 425 insertions(+), 78 deletions(-) diff --git a/doc/snippets/MagnumTrade.cpp b/doc/snippets/MagnumTrade.cpp index fe078fb71..73ae13e27 100644 --- a/doc/snippets/MagnumTrade.cpp +++ b/doc/snippets/MagnumTrade.cpp @@ -533,12 +533,15 @@ Trade::MaterialData data{Trade::MaterialType::Phong, {}, attributes}; { GL::Texture2D baseColorTexture; /* [MaterialData-populating-custom] */ +char sha1[20]{DOXYGEN_ELLIPSIS()}; + Trade::MaterialData data{Trade::MaterialType::PbrMetallicRoughness, { {Trade::MaterialAttribute::BaseColor, 0x3bd267ff_srgbaf}, {Trade::MaterialAttribute::TextureMatrix, Matrix3::scaling({0.5f, 1.0f})}, {"baseColorTexturePointer", &baseColorTexture}, {"highlightColor", 0x00ffff_srgbf}, - {"name", "Canary Green Plastic, really ugly"} + {"name", "Canary Green Plastic, really ugly"}, + {"hash", Containers::ArrayView{sha1}}, }}; // Retrieving the texture pointer diff --git a/src/Magnum/SceneTools/Implementation/sceneConverterUtilities.h b/src/Magnum/SceneTools/Implementation/sceneConverterUtilities.h index 2de8de8f9..551cf0576 100644 --- a/src/Magnum/SceneTools/Implementation/sceneConverterUtilities.h +++ b/src/Magnum/SceneTools/Implementation/sceneConverterUtilities.h @@ -902,6 +902,9 @@ bool printInfo(const Debug::Flags useColor, const bool useColor24, const Utility case Trade::MaterialAttributeType::String: d << info.data.attribute(i, j); break; + case Trade::MaterialAttributeType::Buffer: + d << info.data.attribute>(i, j).size() << "bytes"; + break; case Trade::MaterialAttributeType::TextureSwizzle: d << Debug::packed << info.data.attribute(i, j); break; diff --git a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp index 4b9622333..16d9e5f25 100644 --- a/src/Magnum/SceneTools/Test/SceneConverterTest.cpp +++ b/src/Magnum/SceneTools/Test/SceneConverterTest.cpp @@ -1217,6 +1217,7 @@ void SceneConverterTest::infoImplementationMaterials() { /* These shouldn't have a color swatch rendered */ {"notAColour4", Vector4{0.1f, 0.2f, 0.3f, 0.4f}}, {"notAColour3", Vector3{0.2f, 0.3f, 0.4f}}, + {"data", Containers::ArrayView{"0123456789abcdef", 17}}, {"deadBeef", reinterpret_cast(0xdeadbeef)}, {"undeadBeef", reinterpret_cast(0xbeefbeef)}, }}; diff --git a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-materials.txt b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-materials.txt index 35175acc6..1c6f90c31 100644 --- a/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-materials.txt +++ b/src/Magnum/SceneTools/Test/SceneConverterTestFiles/info-materials.txt @@ -9,6 +9,7 @@ Material 0: 0, 1, 0.75, 0, 0, 1} RoughnessTextureSwizzle @ TextureSwizzle: B + data @ Buffer: 17 bytes deadBeef @ Pointer: 0xdeadbeef notAColour3 @ Vector3: {0.2, 0.3, 0.4} notAColour4 @ Vector4: {0.1, 0.2, 0.3, 0.4} diff --git a/src/Magnum/Trade/MaterialData.cpp b/src/Magnum/Trade/MaterialData.cpp index d9f2eecab..94e95f257 100644 --- a/src/Magnum/Trade/MaterialData.cpp +++ b/src/Magnum/Trade/MaterialData.cpp @@ -156,7 +156,8 @@ std::size_t materialAttributeTypeSize(const MaterialAttributeType type) { return sizeof(void*); case MaterialAttributeType::String: - CORRADE_ASSERT_UNREACHABLE("Trade::materialAttributeTypeSize(): string size is unknown", {}); + case MaterialAttributeType::Buffer: + CORRADE_ASSERT_UNREACHABLE("Trade::materialAttributeTypeSize(): string and buffer size is unknown", {}); } CORRADE_ASSERT_UNREACHABLE("Trade::materialAttributeTypeSize(): invalid type" << type, {}); @@ -177,18 +178,33 @@ MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, std::memcpy(_data.data + 1, name.data(), name.size()); std::memcpy(_data.data + Implementation::MaterialAttributeDataSize - stringValue.size() - 2, stringValue.data(), stringValue.size()); _data.data[Implementation::MaterialAttributeDataSize - 1] = stringValue.size(); - return; - } - /* The 2 extra bytes are for a null byte after the name and a type */ - CORRADE_ASSERT(name.size() + size + 2 <= Implementation::MaterialAttributeDataSize, - "Trade::MaterialAttributeData: name" << name << "too long, expected at most" << Implementation::MaterialAttributeDataSize - size - 2 << "bytes for" << type << "but got" << name.size(), ); - _data.type = type; - std::memcpy(_data.data + 1, name.data(), name.size()); - std::memcpy(_data.data + Implementation::MaterialAttributeDataSize - size, value, size); + /* Special handling for buffers. Similar to strings, except that the size + is stored right after the null-terminated name, and the value has no + null terminator. */ + } else if(type == MaterialAttributeType::Buffer) { + const auto& bufferValue = *static_cast*>(value); + /* The 3 extra bytes are for a null byte after the name, a type and a + value size */ + CORRADE_ASSERT(name.size() + bufferValue.size() + 3 <= Implementation::MaterialAttributeDataSize, + "Trade::MaterialAttributeData: name" << name << "and a" << bufferValue.size() << Debug::nospace << "-byte value too long, expected at most" << Implementation::MaterialAttributeDataSize - 3 << "bytes in total but got" << name.size() + bufferValue.size(), ); + _data.type = MaterialAttributeType::Buffer; + std::memcpy(_data.data + 1, name.data(), name.size()); + _data.data[name.size() + 2] = bufferValue.size(); + std::memcpy(_data.data + Implementation::MaterialAttributeDataSize - bufferValue.size(), bufferValue.data(), bufferValue.size()); + + /* Fixed-size values */ + } else { + /* The 2 extra bytes are for a null byte after the name and a type */ + CORRADE_ASSERT(name.size() + size + 2 <= Implementation::MaterialAttributeDataSize, + "Trade::MaterialAttributeData: name" << name << "too long, expected at most" << Implementation::MaterialAttributeDataSize - size - 2 << "bytes for" << type << "but got" << name.size(), ); + _data.type = type; + std::memcpy(_data.data + 1, name.data(), name.size()); + std::memcpy(_data.data + Implementation::MaterialAttributeDataSize - size, value, size); + } } -MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, const MaterialAttributeType type, const void* const value) noexcept: MaterialAttributeData{name, type, type == MaterialAttributeType::String ? ~std::size_t{} : materialAttributeTypeSize(type), value} {} +MaterialAttributeData::MaterialAttributeData(const Containers::StringView name, const MaterialAttributeType type, const void* const value) noexcept: MaterialAttributeData{name, type, type == MaterialAttributeType::String || type == MaterialAttributeType::Buffer ? ~std::size_t{} : materialAttributeTypeSize(type), value} {} MaterialAttributeData::MaterialAttributeData(const MaterialAttribute name, const MaterialAttributeType type, const void* const value) noexcept { CORRADE_ASSERT(UnsignedInt(name) - 1 < Containers::arraySize(AttributeMap), @@ -226,6 +242,15 @@ MaterialAttributeData::MaterialAttributeData(const MaterialLayer layerName) noex const void* MaterialAttributeData::value() const { if(_data.type == MaterialAttributeType::String) return _data.s.nameValue + Implementation::MaterialAttributeDataSize - _data.s.size - 3; + if(_data.type == MaterialAttributeType::Buffer) { + /* Using an internal StringView API because it's never slower than + memchr() */ + /** @todo have an optimized aligned fixed-size variant? */ + const char* const nameEnd = Containers::Implementation::stringFindCharacter(_data.data, Implementation::MaterialAttributeDataSize, '\0'); + CORRADE_INTERNAL_ASSERT(nameEnd); + const std::size_t size = *(nameEnd + 1); + return _data.data + Implementation::MaterialAttributeDataSize - size; + } return _data.data + Implementation::MaterialAttributeDataSize - materialAttributeTypeSize(_data.type); } @@ -237,6 +262,17 @@ template<> MAGNUM_TRADE_EXPORT Containers::StringView MaterialAttributeData::val "Trade::MaterialAttributeData::value():" << (_data.data + 1) << "of" << _data.type << "can't be retrieved as a string", {}); return {_data.s.nameValue + Implementation::MaterialAttributeDataSize - _data.s.size - 3, _data.s.size, Containers::StringViewFlag::NullTerminated}; } +template<> MAGNUM_TRADE_EXPORT Containers::ArrayView MaterialAttributeData::value>() const { + CORRADE_ASSERT(_data.type == MaterialAttributeType::Buffer, + "Trade::MaterialAttributeData::value():" << (_data.data + 1) << "of" << _data.type << "can't be retrieved as a buffer", {}); + /* Using an internal StringView API because it's never slower than + memchr() */ + /** @todo have an optimized aligned fixed-size variant? */ + const char* const nameEnd = Containers::Implementation::stringFindCharacter(_data.data, Implementation::MaterialAttributeDataSize, '\0'); + CORRADE_INTERNAL_ASSERT(nameEnd); + const std::size_t size = *(nameEnd + 1); + return {_data.data + Implementation::MaterialAttributeDataSize - size, size}; +} #endif MaterialData::MaterialData(const MaterialTypes types, Containers::Array&& attributeData, Containers::Array&& layerData, const void* const importerState) noexcept: _data{std::move(attributeData)}, _layerOffsets{std::move(layerData)}, _types{types}, _attributeDataFlags{DataFlag::Owned|DataFlag::Mutable}, _layerDataFlags{DataFlag::Owned|DataFlag::Mutable}, _importerState{importerState} { @@ -971,6 +1007,46 @@ template<> MAGNUM_TRADE_EXPORT Containers::MutableStringView MaterialData::mutab "Trade::MaterialData::mutableAttribute():" << (data._data.data + 1) << "of" << data._data.type << "can't be retrieved as a string", {}); return {const_cast(data._data.s.nameValue) + Implementation::MaterialAttributeDataSize - data._data.s.size - 3, data._data.s.size, Containers::StringViewFlag::NullTerminated}; } + +template<> MAGNUM_TRADE_EXPORT Containers::ArrayView MaterialData::attribute>(const UnsignedInt layer, const UnsignedInt id) const { + /* Can't delegate to attribute() returning const void* because that doesn't + include the size */ + CORRADE_ASSERT(layer < layerCount(), + "Trade::MaterialData::attribute(): index" << layer << "out of range for" << layerCount() << "layers", {}); + CORRADE_ASSERT(id < attributeCount(layer), + "Trade::MaterialData::attribute(): index" << id << "out of range for" << attributeCount(layer) << "attributes in layer" << layer, {}); + const Trade::MaterialAttributeData& data = _data[layerOffset(layer) + id]; + CORRADE_ASSERT(data._data.type == MaterialAttributeType::Buffer, + "Trade::MaterialData::attribute():" << (data._data.data + 1) << "of" << data._data.type << "can't be retrieved as a buffer", {}); + /* Using an internal StringView API because it's never slower than + memchr() */ + /** @todo have an optimized aligned fixed-size variant? */ + const char* const nameEnd = Containers::Implementation::stringFindCharacter(data._data.data, Implementation::MaterialAttributeDataSize, '\0'); + CORRADE_INTERNAL_ASSERT(nameEnd); + const std::size_t size = *(nameEnd + 1); + return {data._data.data + Implementation::MaterialAttributeDataSize - size, size}; +} + +template<> MAGNUM_TRADE_EXPORT Containers::ArrayView MaterialData::mutableAttribute>(const UnsignedInt layer, const UnsignedInt id) { + CORRADE_ASSERT(_attributeDataFlags & DataFlag::Mutable, + "Trade::MaterialData::mutableAttribute(): attribute data not mutable", {}); + /* Can't delegate to mutableAttribute() returning void* because that + doesn't include the size */ + CORRADE_ASSERT(layer < layerCount(), + "Trade::MaterialData::mutableAttribute(): index" << layer << "out of range for" << layerCount() << "layers", {}); + CORRADE_ASSERT(id < attributeCount(layer), + "Trade::MaterialData::mutableAttribute(): index" << id << "out of range for" << attributeCount(layer) << "attributes in layer" << layer, {}); + const Trade::MaterialAttributeData& data = _data[layerOffset(layer) + id]; + CORRADE_ASSERT(data._data.type == MaterialAttributeType::Buffer, + "Trade::MaterialData::mutableAttribute():" << (data._data.data + 1) << "of" << data._data.type << "can't be retrieved as a buffer", {}); + /* Using an internal StringView API because it's never slower than + memchr() */ + /** @todo have an optimized aligned fixed-size variant? */ + const char* const nameEnd = Containers::Implementation::stringFindCharacter(data._data.data, Implementation::MaterialAttributeDataSize, '\0'); + CORRADE_INTERNAL_ASSERT(nameEnd); + const std::size_t size = *(nameEnd + 1); + return {const_cast(data._data.data) + Implementation::MaterialAttributeDataSize - size, size}; +} #endif const void* MaterialData::tryAttribute(const UnsignedInt layer, const Containers::StringView name) const { @@ -1122,6 +1198,7 @@ Debug& operator<<(Debug& debug, const MaterialAttributeType value) { _c(Pointer) _c(MutablePointer) _c(String) + _c(Buffer) _c(TextureSwizzle) #undef _c /* LCOV_EXCL_STOP */ diff --git a/src/Magnum/Trade/MaterialData.h b/src/Magnum/Trade/MaterialData.h index 042081b93..1ec0ac33d 100644 --- a/src/Magnum/Trade/MaterialData.h +++ b/src/Magnum/Trade/MaterialData.h @@ -1222,6 +1222,13 @@ enum class MaterialAttributeType: UnsignedByte { */ String, + /** + * Opaque data. Can be stored using any type convertible to + * @relativeref{Corrade,Containers::ArrayView}, retrieval has to be done + * using @ref Corrade::Containers::ArrayView "Containers::ArrayView". + */ + Buffer, + /** One of the values from @ref MaterialTextureSwizzle */ TextureSwizzle }; @@ -1279,7 +1286,7 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { */ template::value>::type + , class = typename std::enable_if::value && !std::is_convertible>::value>::type #endif > constexpr /*implicit*/ MaterialAttributeData(Containers::StringView name, const T& value) noexcept; @@ -1299,10 +1306,24 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { */ constexpr /*implicit*/ MaterialAttributeData(Containers::StringView name, Containers::StringView value) noexcept; + /** + * @brief Construct with a string name and a buffer value + * @param name Attribute name + * @param value Attribute value + * + * The combined length of @p name and @p value is expected to fit into + * 61 bytes. Type is set to @ref MaterialAttributeType::Buffer. + * + * This function is useful for creating custom material attributes. + * Currently there isn't any builtin @ref MaterialAttribute with a + * buffer data type. + */ + /*implicit*/ MaterialAttributeData(Containers::StringView name, Containers::ArrayView value) noexcept: MaterialAttributeData{name, MaterialAttributeType::Buffer, 0, &value} {} + #ifndef DOXYGEN_GENERATING_OUTPUT /* "Sure can't be constexpr" overloads to avoid going through the *insane* overload puzzle when not needed */ - template::value>::type> /*implicit*/ MaterialAttributeData(const char* name, const T& value) noexcept: MaterialAttributeData{name, Implementation::MaterialAttributeTypeFor::type(), sizeof(T), &value} {} + template::value && !std::is_convertible>::value>::type> /*implicit*/ MaterialAttributeData(const char* name, const T& value) noexcept: MaterialAttributeData{name, Implementation::MaterialAttributeTypeFor::type(), sizeof(T), &value} {} /*implicit*/ MaterialAttributeData(const char* name, Containers::StringView value) noexcept: MaterialAttributeData{name, MaterialAttributeType::String, 0, &value} {} #endif @@ -1337,21 +1358,30 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { */ /*implicit*/ MaterialAttributeData(MaterialAttribute name, Containers::StringView value) noexcept: MaterialAttributeData{name, MaterialAttributeType::String, &value} {} + /* No MaterialAttributeData(MaterialAttribute, Containers::ArrayView) + variant as there's no builtin MaterialAttributeType::Buffer + attribute yet */ + /** * @brief Construct from a type-erased value * @param name Attribute name * @param type Attribute type * @param value Type-erased value * - * In case @p type is not @ref MaterialAttributeType::String, copies a - * number of bytes according to @ref materialAttributeTypeSize() from - * @p value. The @p name together with @p value is expected to fit into - * 62 bytes. + * In case @p type is neither @ref MaterialAttributeType::String nor + * @ref MaterialAttributeType::Buffer, copies a number of bytes + * according to @ref materialAttributeTypeSize() from @p value. The + * @p name together with @p value is expected to fit into 62 bytes. * * In case @p type is @ref MaterialAttributeType::String, @p value is * expected to point to a @ref Containers::StringView. The combined * length of @p name and @p value strings is expected to fit into 60 * bytes. + * + * In case @p type is @ref MaterialAttributeType::String, @p value is + * expected to point to a @relativeref{Corrade,Containers::ArrayView}. + * The combined length of @p name and @p value views is expected to fit + * into 61 bytes. */ /*implicit*/ MaterialAttributeData(Containers::StringView name, MaterialAttributeType type, const void* value) noexcept; @@ -1402,6 +1432,10 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { * @ref Containers::StringView). This doesn't preserve the actual * string size in case the string data contain @cpp '\0' @ce bytes, * thus prefer to use typed access in that case. + * + * In case of a @ref MaterialAttributeType::Buffer, returns a + * pointer to the data with no size information. Prefer to use typed + * access in that case. */ const void* value() const; @@ -1420,15 +1454,18 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { /* Most of this is needed only for the constexpr constructor (yay C++), the actual data layout is - |--------------------- x B -------------------| + |------------------------- x B -----------------------| - +--------+------- .. -----+-------------------+ - | type | name .. \0 | data | - | 1 B | (x - n - 2) B | n B | - +--------+------- .. -----+------------+------+ - | String | name .. \0 | data .. \0 | size | - | 1 B | (x - n - 4) B | n B | 1 B | - +--------+------- .. -----+------------+------+ + +--------+------- .. -----+-------- .. ---------------+ + | type | name .. \0 | data | + | 1 B | (x - n - 2) B | n B | + +--------+------- .. -----+-------- .. --------+------+ + | String | name .. \0 | data .. \0 | size | + | 1 B | (x - n - 4) B | n B | 1 B | + +--------+------- .. -----+-------- .. -------++------+ + | Buffer | name \0 | size | .. \0 | data | + | 1 B | m + 1 B | 1 B | (x - m - n - 3) B | n B | + +--------+---------+------+-------- .. -------+-------+ where @@ -1437,14 +1474,22 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { - `data` is of size matching `type`, at the offset of `(x - materialAttributeTypeSize(type))` B, or in case of strings at offset `(x - string.size() - 2)` B, with one byte for storing - size and one null terminator, + size and one null terminator, or in case of buffers at offset + `(x - buffer.size())` B, - `name` is a null-terminated string filling the rest This way the name is always at the same offset to make binary search lookup fast and efficient, and data being at the end (instead of right after the null-terminated string) makes them accessible in O(1) as well. In case of string values, to achieve O(1) access, the size - is stored as the last byte and the string data is right before. */ + is stored as the last byte and the string data is right before. + + The only exception is arbitrary data buffers. There, similarly to + plain values, it's important that the data are aligned, which means + we can't store the 1-byte size at the end. Instead, it's put right + after the null-terminated name, which means it takes O(m) to + retrieve. But since names have a constant upper bound on their length + and buffers are not so common, it shouldn't be too problematic. */ struct StringData { template constexpr explicit StringData(MaterialAttributeType type, Containers::StringView name, Containers::StringView value, Containers::Implementation::Sequence): type{type}, nameValue{(sequence < name.size() ? name[sequence] : (sequence - (Implementation::MaterialAttributeDataSize - value.size() - 3) < value.size() ? value[sequence - (Implementation::MaterialAttributeDataSize - value.size() - 3)] : '\0'))...}, size{UnsignedByte(value.size())} {} constexpr explicit StringData(MaterialAttributeType type, Containers::StringView name, Containers::StringView value): StringData{type, name, value, typename Containers::Implementation::GenerateSequence::Type{}} {} @@ -1521,6 +1566,10 @@ class MAGNUM_TRADE_EXPORT MaterialAttributeData { MaterialAttributeType type; char data[Implementation::MaterialAttributeDataSize]; StringData s; + /* Buffer values can't be filled in a constexpr way so they don't + have a dedicated union type. The filling is done in + MaterialAttributeData(Containers::StringView, MaterialAttributeType, std::size_t, const void*) + manually instead. */ Data _1; Data p; Data _4; @@ -1772,9 +1821,9 @@ already sorted by name. While attribute names beginning with uppercase letters and whitespace are reserved for builtin Magnum attributes, anything beginning with a lowercase letter or a printable non-letter character can be a custom attribute. For -greater flexibility, custom attributes can be also strings or pointers, -allowing you to store arbitrary properties such as image filenames or direct -texture pointers instead of IDs: +greater flexibility, custom attributes can be also strings, untyped buffers +or pointers, allowing you to store arbitrary properties such as image +filenames or direct texture pointers instead of IDs: @snippet MagnumTrade.cpp MaterialData-populating-custom @@ -2516,6 +2565,9 @@ class MAGNUM_TRADE_EXPORT MaterialData { * @ref Containers::StringView). This doesn't preserve the actual * string size in case the string data contain zero bytes, thus * prefer to use typed access in that case. + * - In case of a @ref MaterialAttributeType::Buffer returns a + * pointer to the data with no size information, Prefer to use + * typed access in that case. */ const void* attribute(UnsignedInt layer, UnsignedInt id) const; @@ -2543,6 +2595,9 @@ class MAGNUM_TRADE_EXPORT MaterialData { * @ref Containers::StringView). This doesn't preserve the actual * string size in case the string data contain zero bytes, thus * prefer to use typed access in that case. + * - In case of a @ref MaterialAttributeType::Buffer returns a + * pointer to the data with no size information, Prefer to use + * typed access in that case. * * @see @ref hasAttribute(), @ref tryAttribute(), @ref attributeOr() */ @@ -2574,6 +2629,9 @@ class MAGNUM_TRADE_EXPORT MaterialData { * @ref Containers::StringView). This doesn't preserve the actual * string size in case the string data contain zero bytes, thus * prefer to use typed access in that case. + * - In case of a @ref MaterialAttributeType::Buffer returns a + * pointer to the data with no size information, Prefer to use + * typed access in that case. * * @see @ref hasLayer() */ @@ -2605,6 +2663,9 @@ class MAGNUM_TRADE_EXPORT MaterialData { * @ref Containers::StringView). This doesn't preserve the actual * string size in case the string data contain zero bytes, thus * prefer to use typed access in that case. + * - In case of a @ref MaterialAttributeType::Buffer returns a + * pointer to the data with no size information, Prefer to use + * typed access in that case. * * @see @ref hasLayer(), @ref hasAttribute() */ @@ -2691,14 +2752,17 @@ class MAGNUM_TRADE_EXPORT MaterialData { * * Like @ref attribute(UnsignedInt, UnsignedInt) const but returns a * mutable reference. Expects that the material is mutable. In case of - * a string, you're expected to use - * @relativeref{Corrade,Containers::MutableStringView} instead of - * @relativeref{Corrade,Containers::StringView} for @p T and you get a - * @relativeref{Corrade,Containers::MutableStringView} back by-value, - * not by-reference. Changing the string size is not possible. + * a string / buffer, you're expected to use + * @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} instead of + * @relativeref{Corrade,Containers::StringView} / + * @relativeref{Corrade,Containers::ArrayView} for @p T and + * you get a @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} back by-value, not + * by-reference. Changing the string / buffer size is not possible. * @see @ref attributeDataFlags() */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(UnsignedInt layer, UnsignedInt id); + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(UnsignedInt layer, UnsignedInt id); /** * @brief Value of a named attribute in given material layer @@ -2719,15 +2783,18 @@ class MAGNUM_TRADE_EXPORT MaterialData { * * Like @ref attribute(UnsignedInt, Containers::StringView) const but * returns a mutable reference. Expects that the material is mutable. - * In case of a string, you're expected to use - * @relativeref{Corrade,Containers::MutableStringView} instead of - * @relativeref{Corrade,Containers::StringView} for @p T and you get a - * @relativeref{Corrade,Containers::MutableStringView} back by-value, - * not by-reference. Changing the string size is not possible. + * In case of a string / buffer, you're expected to use + * @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} instead of + * @relativeref{Corrade,Containers::StringView} / + * @relativeref{Corrade,Containers::ArrayView} for @p T and + * you get a @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} back by-value, not + * by-reference. Changing the string / buffer size is not possible. * @see @ref attributeDataFlags() */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(UnsignedInt layer, Containers::StringView name); - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(UnsignedInt layer, MaterialAttribute name); /**< @overload */ + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(UnsignedInt layer, Containers::StringView name); + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(UnsignedInt layer, MaterialAttribute name); /**< @overload */ /** * @brief Value of an attribute in a named material layer @@ -2749,15 +2816,18 @@ class MAGNUM_TRADE_EXPORT MaterialData { * * Like @ref attribute(Containers::StringView, UnsignedInt) const but * returns a mutable reference. Expects that the material is mutable. - * In case of a string, you're expected to use - * @relativeref{Corrade,Containers::MutableStringView} instead of - * @relativeref{Corrade,Containers::StringView} for @p T and you get a - * @relativeref{Corrade,Containers::MutableStringView} back by-value, - * not by-reference. Changing the string size is not possible. + * In case of a string / buffer, you're expected to use + * @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} instead of + * @relativeref{Corrade,Containers::StringView} / + * @relativeref{Corrade,Containers::ArrayView} for @p T and + * you get a @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} back by-value, not + * by-reference. Changing the string / buffer size is not possible. * @see @ref attributeDataFlags() */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(Containers::StringView layer, UnsignedInt id); - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(MaterialLayer layer, UnsignedInt id); /**< @overload */ + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(Containers::StringView layer, UnsignedInt id); + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(MaterialLayer layer, UnsignedInt id); /**< @overload */ /** * @brief Value of a named attribute in a named material layer @@ -2780,17 +2850,20 @@ class MAGNUM_TRADE_EXPORT MaterialData { * * Like @ref attribute(Containers::StringView, Containers::StringView) const * but returns a mutable reference. Expects that the material is - * mutable. In case of a string, you're expected to use - * @relativeref{Corrade,Containers::MutableStringView} instead of - * @relativeref{Corrade,Containers::StringView} for @p T and you get a - * @relativeref{Corrade,Containers::MutableStringView} back by-value, - * not by-reference. Changing the string size is not possible. + * mutable. In case of a string / buffer, you're expected to use + * @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} instead of + * @relativeref{Corrade,Containers::StringView} / + * @relativeref{Corrade,Containers::ArrayView} for @p T and + * you get a @relativeref{Corrade,Containers::MutableStringView} / + * @relativeref{Corrade,Containers::ArrayView} back by-value, not + * by-reference. Changing the string / buffer size is not possible. * @see @ref attributeDataFlags() */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(Containers::StringView layer, Containers::StringView name); - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(Containers::StringView layer, MaterialAttribute name); /**< @overload */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(MaterialLayer layer, Containers::StringView name); /**< @overload */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(MaterialLayer layer, MaterialAttribute name); /**< @overload */ + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(Containers::StringView layer, Containers::StringView name); + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(Containers::StringView layer, MaterialAttribute name); /**< @overload */ + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(MaterialLayer layer, Containers::StringView name); /**< @overload */ + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(MaterialLayer layer, MaterialAttribute name); /**< @overload */ /** * @brief Value of an attribute in the base material @@ -2808,7 +2881,7 @@ class MAGNUM_TRADE_EXPORT MaterialData { * Equivalent to calling @ref mutableAttribute(UnsignedInt, UnsignedInt) * with @p layer set to @cpp 0 @ce. */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(UnsignedInt id) { + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(UnsignedInt id) { return mutableAttribute(0, id); } @@ -2831,10 +2904,10 @@ class MAGNUM_TRADE_EXPORT MaterialData { * Equivalent to calling @ref mutableAttribute(UnsignedInt, Containers::StringView) * with @p layer set to @cpp 0 @ce. */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(Containers::StringView name) { + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(Containers::StringView name) { return mutableAttribute(0, name); } - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(MaterialAttribute name) { + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(MaterialAttribute name) { return mutableAttribute(0, name); } /**< @overload */ @@ -3193,6 +3266,7 @@ template T MaterialAttributeData::value() const { #ifndef DOXYGEN_GENERATING_OUTPUT template<> Containers::StringView MaterialAttributeData::value() const; +template<> Containers::ArrayView MaterialAttributeData::value>() const; #endif template T MaterialData::attribute(const UnsignedInt layer, const UnsignedInt id) const { @@ -3208,7 +3282,7 @@ template T MaterialData::attribute(const UnsignedInt layer, const Unsig return *reinterpret_cast(value); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const UnsignedInt layer, const UnsignedInt id) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const UnsignedInt layer, const UnsignedInt id) { void* const value = mutableAttribute(layer, id); #ifdef CORRADE_GRACEFUL_ASSERT if(!value) return *reinterpret_cast(this); @@ -3224,6 +3298,8 @@ template typename std::conditional Containers::StringView MaterialData::attribute(UnsignedInt, UnsignedInt) const; template<> Containers::MutableStringView MaterialData::mutableAttribute(UnsignedInt, UnsignedInt); +template<> Containers::ArrayView MaterialData::attribute>(UnsignedInt, UnsignedInt) const; +template<> Containers::ArrayView MaterialData::mutableAttribute>(UnsignedInt, UnsignedInt); #endif template T MaterialData::attribute(const UnsignedInt layer, const Containers::StringView name) const { @@ -3235,7 +3311,7 @@ template T MaterialData::attribute(const UnsignedInt layer, const Conta return attribute(layer, id); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const UnsignedInt layer, const Containers::StringView name) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const UnsignedInt layer, const Containers::StringView name) { CORRADE_ASSERT(layer < layerCount(), "Trade::MaterialData::mutableAttribute(): index" << layer << "out of range for" << layerCount() << "layers", *reinterpret_cast(this)); const UnsignedInt id = findAttributeIdInternal(layer, name); @@ -3250,7 +3326,7 @@ template T MaterialData::attribute(const UnsignedInt layer, const Mater return attribute(layer, string); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const UnsignedInt layer, const MaterialAttribute name) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const UnsignedInt layer, const MaterialAttribute name) { const Containers::StringView string = Implementation::materialAttributeNameInternal(name); CORRADE_ASSERT(string.data(), "Trade::MaterialData::mutableAttribute(): invalid name" << name, *reinterpret_cast(this)); return mutableAttribute(layer, string); @@ -3265,7 +3341,7 @@ template T MaterialData::attribute(const Containers::StringView layer, return attribute(layerId, id); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const Containers::StringView layer, const UnsignedInt id) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const Containers::StringView layer, const UnsignedInt id) { const UnsignedInt layerId = findLayerIdInternal(layer); CORRADE_ASSERT(layerId != ~UnsignedInt{}, "Trade::MaterialData::mutableAttribute(): layer" << layer << "not found", *reinterpret_cast(this)); @@ -3284,7 +3360,7 @@ template T MaterialData::attribute(const Containers::StringView layer, return attribute(layerId, id); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const Containers::StringView layer, const Containers::StringView name) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const Containers::StringView layer, const Containers::StringView name) { const UnsignedInt layerId = findLayerIdInternal(layer); CORRADE_ASSERT(layerId != ~UnsignedInt{}, "Trade::MaterialData::mutableAttribute(): layer" << layer << "not found", *reinterpret_cast(this)); @@ -3300,7 +3376,7 @@ template T MaterialData::attribute(const Containers::StringView layer, return attribute(layer, string); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const Containers::StringView layer, const MaterialAttribute name) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const Containers::StringView layer, const MaterialAttribute name) { const Containers::StringView string = Implementation::materialAttributeNameInternal(name); CORRADE_ASSERT(string.data(), "Trade::MaterialData::mutableAttribute(): invalid name" << name, *reinterpret_cast(this)); return mutableAttribute(layer, string); @@ -3312,7 +3388,7 @@ template T MaterialData::attribute(const MaterialLayer layer, const Uns return attribute(string, id); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const MaterialLayer layer, const UnsignedInt id) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const MaterialLayer layer, const UnsignedInt id) { const Containers::StringView string = Implementation::materialLayerNameInternal(layer); CORRADE_ASSERT(string.data(), "Trade::MaterialData::mutableAttribute(): invalid name" << layer, *reinterpret_cast(this)); return mutableAttribute(string, id); @@ -3324,7 +3400,7 @@ template T MaterialData::attribute(const MaterialLayer layer, const Con return attribute(string, name); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const MaterialLayer layer, const Containers::StringView name) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const MaterialLayer layer, const Containers::StringView name) { const Containers::StringView string = Implementation::materialLayerNameInternal(layer); CORRADE_ASSERT(string.data(), "Trade::MaterialData::mutableAttribute(): invalid name" << layer, *reinterpret_cast(this)); return mutableAttribute(string, name); @@ -3336,7 +3412,7 @@ template T MaterialData::attribute(const MaterialLayer layer, const Mat return attribute(string, name); } -template typename std::conditional::value, Containers::MutableStringView, T&>::type MaterialData::mutableAttribute(const MaterialLayer layer, const MaterialAttribute name) { +template typename std::conditional::value || std::is_same>::value, T, T&>::type MaterialData::mutableAttribute(const MaterialLayer layer, const MaterialAttribute name) { const Containers::StringView string = Implementation::materialLayerNameInternal(layer); CORRADE_ASSERT(string.data(), "Trade::MaterialData::mutableAttribute(): invalid name" << layer, *reinterpret_cast(this)); return mutableAttribute(string, name); diff --git a/src/Magnum/Trade/MaterialLayerData.h b/src/Magnum/Trade/MaterialLayerData.h index 6af51c24c..2acaaac54 100644 --- a/src/Magnum/Trade/MaterialLayerData.h +++ b/src/Magnum/Trade/MaterialLayerData.h @@ -241,13 +241,13 @@ template class MaterialLayerData: public MaterialData { * * Same as calling @ref MaterialData::attribute() with @p layer. */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(UnsignedInt id) { + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(UnsignedInt id) { return MaterialData::mutableAttribute(layer, id); } - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(Containers::StringView name) { + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(Containers::StringView name) { return MaterialData::mutableAttribute(layer, name); } /**< @overload */ - template typename std::conditional::value, Containers::MutableStringView, T&>::type mutableAttribute(MaterialAttribute name) { + template typename std::conditional::value || std::is_same>::value, T, T&>::type mutableAttribute(MaterialAttribute name) { return MaterialData::mutableAttribute(layer, name); } /**< @overload */ diff --git a/src/Magnum/Trade/Test/MaterialDataTest.cpp b/src/Magnum/Trade/Test/MaterialDataTest.cpp index d071911ef..709f63e43 100644 --- a/src/Magnum/Trade/Test/MaterialDataTest.cpp +++ b/src/Magnum/Trade/Test/MaterialDataTest.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -68,7 +69,9 @@ struct MaterialDataTest: TestSuite::Tester { void constructAttributePointer(); void constructAttributeMutablePointer(); void constructAttributeStringNameStringValue(); + void constructAttributeStringNameBufferValue(); void constructAttributeNameStringValue(); + void constructAttributeNameBufferValue(); void constructAttributeTextureSwizzle(); void constructAttributeLayer(); @@ -78,12 +81,16 @@ struct MaterialDataTest: TestSuite::Tester { void constructAttributeInvalidType(); void constructAttributeEmptyName(); void constructAttributeEmptyNameString(); + void constructAttributeEmptyNameBuffer(); void constructAttributeTooLarge(); void constructAttributeTooLargeString(); + void constructAttributeTooLargeBuffer(); void constructAttributeTooLargeNameString(); + void constructAttributeTooLargeNameBuffer(); void constructAttributeWrongAccessType(); void constructAttributeWrongAccessPointerType(); void constructAttributeWrongAccessTypeString(); + void constructAttributeWrongAccessTypeBuffer(); void construct(); void constructEmptyAttribute(); @@ -113,6 +120,7 @@ struct MaterialDataTest: TestSuite::Tester { void access(); void accessPointer(); void accessString(); + void accessBuffer(); void accessTextureSwizzle(); void accessMutable(); void accessOptional(); @@ -122,6 +130,7 @@ struct MaterialDataTest: TestSuite::Tester { void accessWrongType(); void accessWrongPointerType(); void accessWrongTypeString(); + void accessWrongTypeBuffer(); void accessLayers(); void accessLayersDefaults(); @@ -224,7 +233,9 @@ MaterialDataTest::MaterialDataTest() { &MaterialDataTest::constructAttributePointer, &MaterialDataTest::constructAttributeMutablePointer, &MaterialDataTest::constructAttributeStringNameStringValue, + &MaterialDataTest::constructAttributeStringNameBufferValue, &MaterialDataTest::constructAttributeNameStringValue, + &MaterialDataTest::constructAttributeNameBufferValue, &MaterialDataTest::constructAttributeTextureSwizzle, &MaterialDataTest::constructAttributeLayer, @@ -234,12 +245,16 @@ MaterialDataTest::MaterialDataTest() { &MaterialDataTest::constructAttributeInvalidType, &MaterialDataTest::constructAttributeEmptyName, &MaterialDataTest::constructAttributeEmptyNameString, + &MaterialDataTest::constructAttributeEmptyNameBuffer, &MaterialDataTest::constructAttributeTooLarge, &MaterialDataTest::constructAttributeTooLargeString, + &MaterialDataTest::constructAttributeTooLargeBuffer, &MaterialDataTest::constructAttributeTooLargeNameString, + &MaterialDataTest::constructAttributeTooLargeNameBuffer, &MaterialDataTest::constructAttributeWrongAccessType, &MaterialDataTest::constructAttributeWrongAccessPointerType, &MaterialDataTest::constructAttributeWrongAccessTypeString, + &MaterialDataTest::constructAttributeWrongAccessTypeBuffer, &MaterialDataTest::construct, &MaterialDataTest::constructEmptyAttribute}); @@ -272,6 +287,7 @@ MaterialDataTest::MaterialDataTest() { &MaterialDataTest::access, &MaterialDataTest::accessPointer, &MaterialDataTest::accessString, + &MaterialDataTest::accessBuffer, &MaterialDataTest::accessTextureSwizzle, &MaterialDataTest::accessMutable, &MaterialDataTest::accessOptional, @@ -281,6 +297,7 @@ MaterialDataTest::MaterialDataTest() { &MaterialDataTest::accessWrongType, &MaterialDataTest::accessWrongPointerType, &MaterialDataTest::accessWrongTypeString, + &MaterialDataTest::accessWrongTypeBuffer, &MaterialDataTest::accessLayers, &MaterialDataTest::accessLayersDefaults, @@ -395,10 +412,12 @@ void MaterialDataTest::attributeTypeSizeInvalid() { materialAttributeTypeSize(MaterialAttributeType(0x0)); materialAttributeTypeSize(MaterialAttributeType(0xfe)); materialAttributeTypeSize(MaterialAttributeType::String); + materialAttributeTypeSize(MaterialAttributeType::Buffer); CORRADE_COMPARE(out.str(), "Trade::materialAttributeTypeSize(): invalid type Trade::MaterialAttributeType(0x0)\n" "Trade::materialAttributeTypeSize(): invalid type Trade::MaterialAttributeType(0xfe)\n" - "Trade::materialAttributeTypeSize(): string size is unknown\n"); + "Trade::materialAttributeTypeSize(): string and buffer size is unknown\n" + "Trade::materialAttributeTypeSize(): string and buffer size is unknown\n"); } void MaterialDataTest::attributeMap() { @@ -653,6 +672,42 @@ void MaterialDataTest::constructAttributeStringNameStringValue() { CORRADE_COMPARE(typeErased.value()[typeErased.value().size()], '\0'); } +void MaterialDataTest::constructAttributeStringNameBufferValue() { + /* Explicitly using a non-null-terminated view on input to check the null + byte isn't read by accident*/ + MaterialAttributeData attribute{"name that's long", Containers::arrayView({0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f})}; + CORRADE_COMPARE(attribute.name(), "name that's long"); + CORRADE_COMPARE(attribute.name().flags(), Containers::StringViewFlag::NullTerminated); + CORRADE_COMPARE(attribute.name()[attribute.name().size()], '\0'); + CORRADE_COMPARE(attribute.type(), MaterialAttributeType::Buffer); + /* The pointer should be aligned */ + CORRADE_COMPARE_AS(attribute.value(), 4, TestSuite::Compare::Aligned); + CORRADE_COMPARE(static_cast(attribute.value())[5], 5.0f); + CORRADE_COMPARE_AS(Containers::arrayCast(attribute.value>()), Containers::arrayView({ + 0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f + }), TestSuite::Compare::Container); + + /* Compared to a StringView attribute there's no constexpr variant of the + constructor */ + + /* Type-erased variant. The above overload delegates into this one so it's + testing the same code path, but keep it here for consistency with + the constructAttributeStringNameStringValue() case. */ + const Float value[]{0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f}; + Containers::ArrayView view = value; + MaterialAttributeData typeErased{"name that's long", MaterialAttributeType::Buffer, &view}; + CORRADE_COMPARE(typeErased.name(), "name that's long"); + CORRADE_COMPARE(typeErased.name().flags(), Containers::StringViewFlag::NullTerminated); + CORRADE_COMPARE(typeErased.name()[typeErased.name().size()], '\0'); + CORRADE_COMPARE(typeErased.type(), MaterialAttributeType::Buffer); + /* The pointer should be aligned */ + CORRADE_COMPARE_AS(attribute.value(), 4, TestSuite::Compare::Aligned); + CORRADE_COMPARE(static_cast(attribute.value())[5], 5.0f); + CORRADE_COMPARE_AS(Containers::arrayCast(attribute.value>()), Containers::arrayView({ + 0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f + }), TestSuite::Compare::Container); +} + void MaterialDataTest::constructAttributeNameStringValue() { /* Explicitly using a non-null-terminated view on input to check the null byte isn't read by accident*/ @@ -680,6 +735,10 @@ void MaterialDataTest::constructAttributeNameStringValue() { CORRADE_COMPARE(typeErased.value()[typeErased.value().size()], '\0'); } +void MaterialDataTest::constructAttributeNameBufferValue() { + CORRADE_SKIP("No builtin attributes with" << MaterialAttributeType::Buffer << "at the moment."); +} + void MaterialDataTest::constructAttributeTextureSwizzle() { MaterialAttributeData attribute{"swizzle", MaterialTextureSwizzle::GBA}; CORRADE_COMPARE(attribute.name(), "swizzle"); @@ -786,6 +845,19 @@ void MaterialDataTest::constructAttributeEmptyNameString() { "Trade::MaterialAttributeData: name is not allowed to be empty\n"); } +void MaterialDataTest::constructAttributeEmptyNameBuffer() { + CORRADE_SKIP_IF_NO_ASSERT(); + + /* This has no reason to not be allowed */ + MaterialAttributeData{"hello this buffer is empty", Containers::ArrayView{}}; + + std::ostringstream out; + Error redirectError{&out}; + MaterialAttributeData{"", Containers::ArrayView{"E", 2}}; + CORRADE_COMPARE(out.str(), + "Trade::MaterialAttributeData: name is not allowed to be empty\n"); +} + void MaterialDataTest::constructAttributeTooLarge() { CORRADE_SKIP_IF_NO_ASSERT(); @@ -821,6 +893,18 @@ void MaterialDataTest::constructAttributeTooLargeString() { "Trade::MaterialAttributeData: name attribute is long and value This is a problem, got a long piece of text! too long, expected at most 60 bytes in total but got 61\n"); } +void MaterialDataTest::constructAttributeTooLargeBuffer() { + CORRADE_SKIP_IF_NO_ASSERT(); + + int data[10]; /* 40 bytes */ + + std::ostringstream out; + Error redirectError{&out}; + MaterialAttributeData{"attribute is very long", data}; + CORRADE_COMPARE(out.str(), + "Trade::MaterialAttributeData: name attribute is very long and a 40-byte value too long, expected at most 61 bytes in total but got 62\n"); +} + void MaterialDataTest::constructAttributeTooLargeNameString() { CORRADE_SKIP_IF_NO_ASSERT(); @@ -831,6 +915,10 @@ void MaterialDataTest::constructAttributeTooLargeNameString() { "Trade::MaterialAttributeData: name LayerName and value This is a problem, got a huge, yuuge value to store too long, expected at most 60 bytes in total but got 61\n"); } +void MaterialDataTest::constructAttributeTooLargeNameBuffer() { + CORRADE_SKIP("No builtin attributes with" << MaterialAttributeType::Buffer << "at the moment."); +} + void MaterialDataTest::constructAttributeWrongAccessType() { CORRADE_SKIP_IF_NO_ASSERT(); @@ -873,6 +961,15 @@ void MaterialDataTest::constructAttributeWrongAccessTypeString() { CORRADE_COMPARE(out.str(), "Trade::MaterialAttributeData::value(): thing3 of Trade::MaterialAttributeType::Matrix4x3 can't be retrieved as a string\n"); } +void MaterialDataTest::constructAttributeWrongAccessTypeBuffer() { + CORRADE_SKIP_IF_NO_ASSERT(); + + std::ostringstream out; + Error redirectError{&out}; + MaterialAttributeData{"thing3", Matrix4x3{}}.value>(); + CORRADE_COMPARE(out.str(), "Trade::MaterialAttributeData::value(): thing3 of Trade::MaterialAttributeType::Matrix4x3 can't be retrieved as a buffer\n"); +} + void MaterialDataTest::construct() { int state; MaterialData data{MaterialType::Phong, { @@ -1765,6 +1862,20 @@ void MaterialDataTest::accessString() { CORRADE_COMPARE(data.attribute(0)[data.attribute(0).size()], '\0'); } +void MaterialDataTest::accessBuffer() { + MaterialData data{{}, { + {"name?", Containers::arrayView({0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f})} + }}; + CORRADE_COMPARE(data.attributeType("name?"), MaterialAttributeType::Buffer); + + /* The pointer should be aligned */ + CORRADE_COMPARE_AS(data.attribute(0), 4, TestSuite::Compare::Aligned); + CORRADE_COMPARE(static_cast(data.attribute(0))[5], 5.0f); + CORRADE_COMPARE_AS(Containers::arrayCast(data.attribute>(0)), Containers::arrayView({ + 0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f + }), TestSuite::Compare::Container); +} + void MaterialDataTest::accessTextureSwizzle() { MaterialData data{{}, { {"normalSwizzle", MaterialTextureSwizzle::BA} @@ -1782,6 +1893,8 @@ void MaterialDataTest::accessMutable() { MaterialData data{{}, { {MaterialAttribute::LayerName, "aye"_s}, {MaterialAttribute::Roughness, 1.0f}, + /** @todo test builtin buffer attribute once it exists */ + {"data", Containers::arrayView({0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f})}, }}; *static_cast(data.mutableAttribute(1)) *= 2.0f; @@ -1799,6 +1912,16 @@ void MaterialDataTest::accessMutable() { ++data.mutableAttribute(MaterialAttribute::LayerName)[0]; ++data.mutableAttribute(" LayerName")[0]; CORRADE_COMPARE(data.attribute(MaterialAttribute::LayerName), "gye"_s); + + static_cast(data.mutableAttribute(2))[1] *= 2.0f; + /** @todo test also builtin buffer attribute access once it exists */ + static_cast(data.mutableAttribute("data"))[2] *= 2.0f; + Containers::arrayCast(data.mutableAttribute>(2))[3] *= 2.0f; + /** @todo test also builtin buffer attribute access once it exists */ + Containers::arrayCast(data.mutableAttribute>("data"))[4] *= 2.0f; + CORRADE_COMPARE_AS(Containers::arrayCast(data.attribute>("data")), Containers::arrayView({ + 0.0f, 2.0f, 4.0f, 6.0f, 8.0f, 5.0f, 6.0f + }), TestSuite::Compare::Container); } void MaterialDataTest::accessOptional() { @@ -1985,6 +2108,40 @@ void MaterialDataTest::accessWrongTypeString() { "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a string\n"); } +void MaterialDataTest::accessWrongTypeBuffer() { + CORRADE_SKIP_IF_NO_ASSERT(); + + MaterialData data{{}, { + {"Shininess", 0.0f} + }}; + + std::ostringstream out; + Error redirectError{&out}; + data.attribute>(0); + data.attribute>(MaterialAttribute::Shininess); + data.attribute>("Shininess"); + data.mutableAttribute>(0); + data.mutableAttribute>(MaterialAttribute::Shininess); + data.mutableAttribute>("Shininess"); + data.tryAttribute>(MaterialAttribute::Shininess); + data.tryAttribute>("Shininess"); + data.attributeOr(MaterialAttribute::Shininess, Containers::ArrayView{}); + data.attributeOr("Shininess", Containers::ArrayView{}); + CORRADE_COMPARE(out.str(), + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::mutableAttribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::mutableAttribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::mutableAttribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + /* tryAttribute() and attributeOr() delegate to attribute() so the + assert is the same */ + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n" + "Trade::MaterialData::attribute(): Shininess of Trade::MaterialAttributeType::Float can't be retrieved as a buffer\n"); +} + void MaterialDataTest::accessLayers() { MaterialData data{{}, { {MaterialAttribute::LayerName, "decals"}, @@ -2855,11 +3012,13 @@ void MaterialDataTest::accessMutableNotAllowed() { const MaterialAttributeData attributes[]{ {MaterialAttribute::DiffuseColor, 0x335566ff_rgbaf}, {MaterialAttribute::LayerName, "ClearCoat"}, - {MaterialAttribute::Roughness, 0.5f} + {MaterialAttribute::Roughness, 0.5f}, + /** @todo test builtin buffer attribute once it exists */ + {"data", Containers::ArrayView{}}, }; const UnsignedInt layers[]{ - 1, 3 + 1, 4 }; MaterialData data{{}, {}, attributes, {}, layers}; @@ -2882,6 +3041,9 @@ void MaterialDataTest::accessMutableNotAllowed() { data.mutableAttribute(1, 0); data.mutableAttribute(1, " LayerName"); data.mutableAttribute(1, MaterialAttribute::LayerName); + data.mutableAttribute>(1, 2); + data.mutableAttribute>(1, "data"); + /** @todo test also builtin buffer attribute access once it exists */ data.mutableAttribute("ClearCoat", 1); data.mutableAttribute("ClearCoat", "Roughness"); @@ -2892,6 +3054,9 @@ void MaterialDataTest::accessMutableNotAllowed() { data.mutableAttribute("ClearCoat", 0); data.mutableAttribute("ClearCoat", " LayerName"); data.mutableAttribute("ClearCoat", MaterialAttribute::LayerName); + data.mutableAttribute>("ClearCoat", 2); + data.mutableAttribute>("ClearCoat", "data"); + /** @todo test also builtin buffer attribute access once it exists */ data.mutableAttribute(MaterialLayer::ClearCoat, 1); data.mutableAttribute(MaterialLayer::ClearCoat, "Roughness"); @@ -2902,6 +3067,9 @@ void MaterialDataTest::accessMutableNotAllowed() { data.mutableAttribute(MaterialLayer::ClearCoat, 0); data.mutableAttribute(MaterialLayer::ClearCoat, " LayerName"); data.mutableAttribute(MaterialLayer::ClearCoat, MaterialAttribute::LayerName); + data.mutableAttribute>(MaterialLayer::ClearCoat, 2); + data.mutableAttribute>(MaterialLayer::ClearCoat, "data"); + /** @todo test also builtin buffer attribute access once it exists */ CORRADE_COMPARE(out.str(), "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" @@ -2918,6 +3086,8 @@ void MaterialDataTest::accessMutableNotAllowed() { "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" + "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" + "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" @@ -2927,6 +3097,8 @@ void MaterialDataTest::accessMutableNotAllowed() { "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" + "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" + "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" @@ -2935,6 +3107,8 @@ void MaterialDataTest::accessMutableNotAllowed() { "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" + "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" + "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n" "Trade::MaterialData::mutableAttribute(): attribute data not mutable\n"); } @@ -3047,7 +3221,9 @@ void MaterialDataTest::templateLayerAccessMutable() { MaterialLayerData data{{}, { {MaterialLayer::ClearCoat}, {MaterialAttribute::Roughness, 1.0f}, - }, {0, 2}}; + /** @todo test builtin buffer attribute once it exists */ + {"data", Containers::arrayView({0.0f, 1.0f, 2.0f, 3.0f, 4.0f, 5.0f, 6.0f})}, + }, {0, 3}}; *static_cast(data.mutableAttribute(1)) *= 2.0f; *static_cast(data.mutableAttribute(MaterialAttribute::Roughness)) *= 2.0f; @@ -3057,6 +3233,16 @@ void MaterialDataTest::templateLayerAccessMutable() { data.mutableAttribute("Roughness") *= 2.0f; CORRADE_COMPARE(data.attribute(MaterialAttribute::Roughness), 64.0f); + static_cast(data.mutableAttribute(2))[1] *= 2.0f; + /** @todo test also builtin buffer attribute access once it exists */ + static_cast(data.mutableAttribute("data"))[2] *= 2.0f; + Containers::arrayCast(data.mutableAttribute>(2))[3] *= 2.0f; + /** @todo test also builtin buffer attribute access once it exists */ + Containers::arrayCast(data.mutableAttribute>("data"))[4] *= 2.0f; + CORRADE_COMPARE_AS(Containers::arrayCast(data.attribute>("data")), Containers::arrayView({ + 0.0f, 2.0f, 4.0f, 6.0f, 8.0f, 5.0f, 6.0f + }), TestSuite::Compare::Container); + /* Resetting back so the layer name always stays the same so the next call can find it. Other than that, the result should be same as in accessLayerIndexMutable(). */