From 95c93c928a0f84235ac9935935174c24b8a00f59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 13 Apr 2020 19:01:54 +0200 Subject: [PATCH] Trade: reorganize MeshAttributeData internals for better serializability. Put the pointer that differs in size on 32 and 64bit platforms last so we have at least the prefix always stable. This also makes it possible to extend the class to store 8/16 bytes of arbitrary data in there (bounds etc.). Not doing that yet, just preparing for possible extension. --- src/Magnum/Trade/MeshData.h | 71 ++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/src/Magnum/Trade/MeshData.h b/src/Magnum/Trade/MeshData.h index bf9f8ff74..6d3274882 100644 --- a/src/Magnum/Trade/MeshData.h +++ b/src/Magnum/Trade/MeshData.h @@ -338,7 +338,7 @@ class MAGNUM_TRADE_EXPORT MeshAttributeData { * initialization of the attribute array for @ref MeshData, expected to * be replaced with concrete values later. */ - constexpr explicit MeshAttributeData() noexcept: _data{}, _vertexCount{}, _format{}, _stride{}, _name{}, _arraySize{}, _isOffsetOnly{false} {} + constexpr explicit MeshAttributeData() noexcept: _format{}, _name{}, _isOffsetOnly{false}, _vertexCount{}, _stride{}, _arraySize{}, _data{} {} /** * @brief Type-erased constructor @@ -485,10 +485,10 @@ class MAGNUM_TRADE_EXPORT MeshAttributeData { * passed to @ref MeshData. * @see @ref stride() */ - constexpr explicit MeshAttributeData(Int padding): _data{nullptr}, _vertexCount{0}, _format{}, _stride{ + constexpr explicit MeshAttributeData(Int padding): _format{}, _name{}, _isOffsetOnly{false}, _vertexCount{0}, _stride{ (CORRADE_CONSTEXPR_ASSERT(padding >= -32768 && padding <= 32767, "Trade::MeshAttributeData: at most 32k padding supported, got" << padding), Short(padding)) - }, _name{}, _arraySize{}, _isOffsetOnly{false} {} + }, _arraySize{}, _data{nullptr} {} /** * @brief If the attribute is offset-only @@ -559,29 +559,34 @@ class MAGNUM_TRADE_EXPORT MeshAttributeData { } private: + friend MeshData; + constexpr explicit MeshAttributeData(MeshAttribute name, VertexFormat format, UnsignedShort arraySize, const Containers::StridedArrayView1D& data, std::nullptr_t) noexcept; - friend MeshData; - union Data { - /* FFS C++ why this doesn't JUST WORK goddamit?! */ - constexpr Data(const void* pointer = nullptr): pointer{pointer} {} - constexpr Data(std::size_t offset): offset{offset} {} + VertexFormat _format; + MeshAttribute _name; + bool _isOffsetOnly; + /* 1 byte free for more stuff on 64b (23, aligned to 24) and on 32b + (19, aligned to 20) */ - const void* pointer; - std::size_t offset; - } _data; /* Vertex count in MeshData is currently 32-bit, so this doesn't need to be 64-bit either */ UnsignedInt _vertexCount; - VertexFormat _format; /* According to https://opengl.gpuinfo.org/displaycapability.php?name=GL_MAX_VERTEX_ATTRIB_STRIDE, current largest reported stride is 4k so 32k should be enough */ Short _stride; - MeshAttribute _name; UnsignedShort _arraySize; - bool _isOffsetOnly; - /* 1 byte free for more stuff on 64b (23, aligned to 24) and on 32b - (19, aligned to 20) */ + + /* Data pointer last. Its size varies between 32 and 64 bit and having + it last reduces the amount of pain when serializing. */ + union Data { + /* FFS C++ why this doesn't JUST WORK goddamit?! */ + constexpr Data(const void* pointer = nullptr): pointer{pointer} {} + constexpr Data(std::size_t offset): offset{offset} {} + + const void* pointer; + std::size_t offset; + } _data; }; /** @relatesalso MeshAttributeData @@ -2069,30 +2074,32 @@ namespace Implementation { #endif constexpr MeshAttributeData::MeshAttributeData(const MeshAttribute name, const VertexFormat format, const UnsignedShort arraySize, const Containers::StridedArrayView1D& data, std::nullptr_t) noexcept: - _data{data.data()}, _vertexCount{UnsignedInt(data.size())}, _format{format}, + _format{(CORRADE_CONSTEXPR_ASSERT(!arraySize || !isVertexFormatImplementationSpecific(format), + "Trade::MeshAttributeData: array attributes can't have an implementation-specific format"), format)}, + _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isVertexFormatCompatibleWithAttribute(name, format), + "Trade::MeshAttributeData:" << format << "is not a valid format for" << name), name)}, + _isOffsetOnly{false}, _vertexCount{UnsignedInt(data.size())}, /** @todo support zero / negative stride? would be hard to transfer to GL */ _stride{(CORRADE_CONSTEXPR_ASSERT(!(UnsignedInt(data.stride()) & 0xffff8000), "Trade::MeshAttributeData: expected stride to be positive and at most 32k, got" << data.stride()), - Short(data.stride())) - }, _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isVertexFormatCompatibleWithAttribute(name, format), - "Trade::MeshAttributeData:" << format << "is not a valid format for" << name), name) - }, _arraySize{(CORRADE_CONSTEXPR_ASSERT(!arraySize || Implementation::isAttributeArrayAllowed(name), - "Trade::MeshAttributeData:" << name << "can't be an array attribute"), arraySize) - }, _isOffsetOnly{(CORRADE_CONSTEXPR_ASSERT(!arraySize || !isVertexFormatImplementationSpecific(format), - "Trade::MeshAttributeData: array attributes can't have an implementation-specific format"), false)} {} + Short(data.stride()))}, + _arraySize{(CORRADE_CONSTEXPR_ASSERT(!arraySize || Implementation::isAttributeArrayAllowed(name), + "Trade::MeshAttributeData:" << name << "can't be an array attribute"), arraySize)}, + _data{data.data()} {} constexpr MeshAttributeData::MeshAttributeData(const MeshAttribute name, const VertexFormat format, const std::size_t offset, const UnsignedInt vertexCount, const std::ptrdiff_t stride, UnsignedShort arraySize) noexcept: - _data{offset}, _vertexCount{vertexCount}, _format{format}, + _format{(CORRADE_CONSTEXPR_ASSERT(!arraySize || !isVertexFormatImplementationSpecific(format), + "Trade::MeshAttributeData: array attributes can't have an implementation-specific format"), format)}, + _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isVertexFormatCompatibleWithAttribute(name, format), + "Trade::MeshAttributeData:" << format << "is not a valid format for" << name), name)}, + _isOffsetOnly{true}, _vertexCount{vertexCount}, /** @todo support zero / negative stride? would be hard to transfer to GL */ _stride{(CORRADE_CONSTEXPR_ASSERT(!(UnsignedInt(stride) & 0xffff8000), "Trade::MeshAttributeData: expected stride to be positive and at most 32k, got" << stride), - Short(stride)) - }, _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isVertexFormatCompatibleWithAttribute(name, format), - "Trade::MeshAttributeData:" << format << "is not a valid format for" << name), name) - }, _arraySize{(CORRADE_CONSTEXPR_ASSERT(!arraySize || Implementation::isAttributeArrayAllowed(name), - "Trade::MeshAttributeData:" << name << "can't be an array attribute"), arraySize) - }, _isOffsetOnly{(CORRADE_CONSTEXPR_ASSERT(!arraySize || !isVertexFormatImplementationSpecific(format), - "Trade::MeshAttributeData: array attributes can't have an implementation-specific format"), true)} {} + Short(stride))}, + _arraySize{(CORRADE_CONSTEXPR_ASSERT(!arraySize || Implementation::isAttributeArrayAllowed(name), + "Trade::MeshAttributeData:" << name << "can't be an array attribute"), arraySize)}, + _data{offset} {} template constexpr MeshAttributeData::MeshAttributeData(MeshAttribute name, const Containers::StridedArrayView1D& data) noexcept: MeshAttributeData{name, Implementation::vertexFormatFor::type>(), 0, data, nullptr} {}