diff --git a/src/Magnum/Trade/SceneData.cpp b/src/Magnum/Trade/SceneData.cpp index cc9d70a0a..7c8a2466b 100644 --- a/src/Magnum/Trade/SceneData.cpp +++ b/src/Magnum/Trade/SceneData.cpp @@ -575,6 +575,36 @@ SceneFieldData::SceneFieldData(const SceneField name, const Containers::StridedA CORRADE_ASSERT(fieldData.isContiguous<1>(), "Trade::SceneFieldData: second field view dimension is not contiguous", ); } +SceneFieldData::SceneFieldData(const SceneField name, const SceneMappingType mappingType, const Containers::StridedArrayView1D& mappingData, const char* const stringData, const SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, const SceneFieldFlags flags) noexcept: + _size{mappingData.size()}, + _name{name}, + _flags{flags|Implementation::implicitSceneFieldFlagsFor(fieldType)}, + _mappingTypeStringType{UnsignedByte(UnsignedByte(mappingType)|(UnsignedShort(fieldType) << 3))}, + _mappingStride{Short(mappingData.stride())}, + _mappingData{mappingData.data()}, + _field{Short(fieldData.stride()), + /* This expression is the reason why the constructor can't be constexpr + -- the only possibility for this to work would be if + fieldData.data() would give back a const char* pointer to avoid the + cast (which is dangerous on its own, as the data is inherently + sparse), and even then it'd probably fail due to the two pointers + being two unrelated pieces of memory */ + stringData - static_cast(fieldData.data())}, + _fieldData{fieldData.data()} { + CORRADE_ASSERT(mappingData.size() == fieldData.size(), + "Trade::SceneFieldData: expected" << name << "mapping and field view to have the same size but got" << mappingData.size() << "and" << fieldData.size(), ); + CORRADE_ASSERT(Implementation::isSceneFieldTypeCompatibleWithField(name, fieldType), + "Trade::SceneFieldData:" << fieldType << "is not a valid type for" << name, ); + CORRADE_ASSERT(!(flags & SceneFieldFlag::OffsetOnly), + "Trade::SceneFieldData: can't pass" << (flags & SceneFieldFlag::OffsetOnly) << "for a view", ); + CORRADE_ASSERT(Implementation::isSceneFieldTypeString(fieldType), + "Trade::SceneFieldData: can't use a string constructor for" << fieldType, ); + CORRADE_ASSERT(mappingData.stride() >= -32768 && mappingData.stride() <= 32767, + "Trade::SceneFieldData: expected mapping view stride to fit into 16 bits but got" << mappingData.stride(), ); + CORRADE_ASSERT(fieldData.stride() >= -32768 && fieldData.stride() <= 32767, + "Trade::SceneFieldData: expected field view stride to fit into 16 bits but got" << fieldData.stride(), ); +} + SceneFieldData::SceneFieldData(const SceneField name, const Containers::StridedArrayView2D& mappingData, const char* const stringData, const SceneFieldType fieldType, const Containers::StridedArrayView2D& fieldData, const SceneFieldFlags flags) noexcept: SceneFieldData{name, {}, Containers::StridedArrayView1D{{mappingData.data(), ~std::size_t{}}, mappingData.size()[0], mappingData.stride()[0]}, stringData, fieldType, Containers::StridedArrayView1D{{fieldData.data(), ~std::size_t{}}, fieldData.size()[0], fieldData.stride()[0]}, flags} { /* Yes, this calls into a constexpr function defined in the header -- because I feel that makes more sense than duplicating the full assert diff --git a/src/Magnum/Trade/SceneData.h b/src/Magnum/Trade/SceneData.h index c33b78ae8..41fe2c18d 100644 --- a/src/Magnum/Trade/SceneData.h +++ b/src/Magnum/Trade/SceneData.h @@ -986,8 +986,13 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { checking happens in the SceneField constructor, at which point the size would be gone anyway as SceneFieldData can store only the begin pointer inside. Using it would also mean we'd need to include its - full definition in this header. */ - constexpr explicit SceneFieldData(SceneField name, SceneMappingType mappingType, const Containers::StridedArrayView1D& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, SceneFieldFlags flags = {}) noexcept; + full definition in this header. + + Not constexpr because internally we store the 48-bit + `stringData - fieldData` offset, which is not possible to do in a + constexpr context. Only the offset-only constructor is usable for + creating constexpr string fields. */ + explicit SceneFieldData(SceneField name, SceneMappingType mappingType, const Containers::StridedArrayView1D& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, SceneFieldFlags flags = {}) noexcept; /** * @brief Construct a string field from 2D type-erased views @@ -1034,11 +1039,12 @@ class MAGNUM_TRADE_EXPORT SceneFieldData { * @relativeref{SceneFieldType,StringRangeNullTerminated32} and * @relativeref{SceneFieldType,StringRangeNullTerminated64}. */ - /* See above for why const char* is used instead of StringView */ - template constexpr explicit SceneFieldData(SceneField name, const Containers::StridedArrayView1D& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, SceneFieldFlags flags = {}) noexcept; + /* See above for why const char* is used instead of StringView and why + this function is not constexpr */ + template explicit SceneFieldData(SceneField name, const Containers::StridedArrayView1D& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, SceneFieldFlags flags = {}) noexcept; /** @overload */ - template constexpr explicit SceneFieldData(SceneField name, const Containers::ArrayView& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, SceneFieldFlags flags = {}) noexcept: SceneFieldData{name, Containers::stridedArrayView(mappingData), stringData, fieldType, fieldData, flags} {} + template explicit SceneFieldData(SceneField name, const Containers::ArrayView& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, SceneFieldFlags flags = {}) noexcept: SceneFieldData{name, Containers::stridedArrayView(mappingData), stringData, fieldType, fieldData, flags} {} /** * @brief Construct an offset-only field @@ -3684,25 +3690,7 @@ template constexpr SceneFieldData::SceneFieldData(const SceneF flags } {} -constexpr SceneFieldData::SceneFieldData(const SceneField name, const SceneMappingType mappingType, const Containers::StridedArrayView1D& mappingData, const char* const stringData, const SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, const SceneFieldFlags flags) noexcept: - _size{(CORRADE_CONSTEXPR_ASSERT(mappingData.size() == fieldData.size(), - "Trade::SceneFieldData: expected" << name << "mapping and field view to have the same size but got" << mappingData.size() << "and" << fieldData.size()), mappingData.size())}, - _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isSceneFieldTypeCompatibleWithField(name, fieldType), - "Trade::SceneFieldData:" << fieldType << "is not a valid type for" << name), name)}, - _flags{(CORRADE_CONSTEXPR_ASSERT(!(flags & SceneFieldFlag::OffsetOnly), - "Trade::SceneFieldData: can't pass" << (flags & SceneFieldFlag::OffsetOnly) << "for a view"), flags|Implementation::implicitSceneFieldFlagsFor(fieldType))}, - _mappingTypeStringType{(CORRADE_CONSTEXPR_ASSERT(Implementation::isSceneFieldTypeString(fieldType), - "Trade::SceneFieldData: can't use a string constructor for" << fieldType), UnsignedByte(UnsignedByte(mappingType)|(UnsignedShort(fieldType) << 3)))}, - _mappingStride{(CORRADE_CONSTEXPR_ASSERT(mappingData.stride() >= -32768 && mappingData.stride() <= 32767, - "Trade::SceneFieldData: expected mapping view stride to fit into 16 bits but got" << mappingData.stride()), Short(mappingData.stride()))}, - _mappingData{mappingData.data()}, - _field{ - (CORRADE_CONSTEXPR_ASSERT(fieldData.stride() >= -32768 && fieldData.stride() <= 32767, - "Trade::SceneFieldData: expected field view stride to fit into 16 bits but got" << fieldData.stride()), Short(fieldData.stride())), - stringData - static_cast(fieldData.data())}, - _fieldData{fieldData.data()} {} - -template constexpr SceneFieldData::SceneFieldData(const SceneField name, const Containers::StridedArrayView1D& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, const SceneFieldFlags flags) noexcept: SceneFieldData{name, Implementation::sceneMappingTypeFor::type>(), mappingData, stringData, fieldType, fieldData, flags} {} +template inline SceneFieldData::SceneFieldData(const SceneField name, const Containers::StridedArrayView1D& mappingData, const char* stringData, SceneFieldType fieldType, const Containers::StridedArrayView1D& fieldData, const SceneFieldFlags flags) noexcept: SceneFieldData{name, Implementation::sceneMappingTypeFor::type>(), mappingData, stringData, fieldType, fieldData, flags} {} constexpr SceneFieldData::SceneFieldData(const SceneField name, const std::size_t size, const SceneMappingType mappingType, const std::size_t mappingOffset, const std::ptrdiff_t mappingStride, const SceneFieldType fieldType, const std::size_t fieldOffset, const std::ptrdiff_t fieldStride, const UnsignedShort fieldArraySize, const SceneFieldFlags flags) noexcept: _size{size}, diff --git a/src/Magnum/Trade/Test/SceneDataTest.cpp b/src/Magnum/Trade/Test/SceneDataTest.cpp index 93520510d..015f35669 100644 --- a/src/Magnum/Trade/Test/SceneDataTest.cpp +++ b/src/Magnum/Trade/Test/SceneDataTest.cpp @@ -852,7 +852,7 @@ void SceneDataTest::constructField() { CORRADE_COMPARE(flags, SceneFieldFlag::ImplicitMapping); CORRADE_COMPARE(mappingType, SceneMappingType::UnsignedShort); /* These are not marked constexpr because it'd work only partially, not for - string fields (tested in constructFieldString()) */ + string fields (tested in constructFieldOffsetOnlyString()) */ CORRADE_COMPARE(crotations.fieldType(), SceneFieldType::Complexd); CORRADE_COMPARE(crotations.fieldArraySize(), 0); /* These are deliberately not constexpr to save header size a bit -- @@ -931,6 +931,9 @@ void SceneDataTest::constructFieldString() { CORRADE_COMPARE(names.fieldData(someArray).stride(), sizeof(Containers::Pair)); CORRADE_COMPARE(names.fieldData(someArray).data(), data.nameField); CORRADE_COMPARE(names.stringData(someArray), static_cast(data.nameString)); + + /* Construction of a string field is not constexpr due to arithmetic on two + (differently cast) pointers */ } void SceneDataTest::constructFieldStringNegativeStride() { @@ -1033,6 +1036,9 @@ void SceneDataTest::constructFieldTypeErasedString() { CORRADE_COMPARE(names.fieldData().stride(), sizeof(UnsignedShort)*2); CORRADE_COMPARE(names.fieldData().data(), nameFieldData); CORRADE_COMPARE(names.stringData(), static_cast(nameStringData)); + + /* Construction of a string field is not constexpr due to arithmetic on two + (differently cast) pointers */ } void SceneDataTest::constructFieldTypeErased2D() { @@ -1148,6 +1154,30 @@ void SceneDataTest::constructFieldOffsetOnlyString() { CORRADE_COMPARE(a.stringData(string), "eyehandnoseleg"_s); CORRADE_COMPARE((Containers::StringView{a.stringData(string) + fieldData[0].first(), fieldData[0].second()}), "hand"); CORRADE_COMPARE((Containers::StringView{a.stringData(string) + fieldData[1].first(), fieldData[1].second()}), "leg"); + + constexpr SceneFieldData ca{sceneFieldCustom(36), 2, SceneMappingType::UnsignedLong, offsetof(Data, object), sizeof(Data), 6, SceneFieldType::StringRange8, offsetof(Data, nameRange), sizeof(Data), SceneFieldFlag::OrderedMapping}; + constexpr SceneField name = ca.name(); + constexpr SceneFieldFlags flags = ca.flags(); + constexpr std::size_t size = ca.size(); + constexpr SceneMappingType mappingType = ca.mappingType(); + CORRADE_COMPARE(name, sceneFieldCustom(36)); + CORRADE_COMPARE(flags, SceneFieldFlag::OffsetOnly|SceneFieldFlag::OrderedMapping); + CORRADE_COMPARE(size, 2); + CORRADE_COMPARE(mappingType, SceneMappingType::UnsignedLong); + /* These are not marked constexpr because it wouldn't work for string + fields due to the type/size stored in an union */ + CORRADE_COMPARE(ca.fieldType(), SceneFieldType::StringRange8); + CORRADE_COMPARE(ca.fieldArraySize(), 0); + /* These are deliberately not constexpr to save header size a bit -- + compared to SceneField APIs they get used very little and it's mostly + useless in a constexpr context anyway */ + CORRADE_COMPARE(ca.mappingData(data).size(), 2); + CORRADE_COMPARE(ca.mappingData(data).stride(), sizeof(Data)); + CORRADE_COMPARE_AS(Containers::arrayCast(ca.mappingData(data)), + Containers::arrayView({2, 15}), + TestSuite::Compare::Container); + CORRADE_COMPARE(ca.fieldData(data).size(), 2); + CORRADE_COMPARE(ca.fieldData(data).stride(), sizeof(Data)); } void SceneDataTest::constructFieldOffsetOnlyStringNegativeStride() {