Browse Source

Trade: properly include array size in attribute stride checks.

pull/449/head
Vladimír Vondruš 6 years ago
parent
commit
f5822e9384
  1. 7
      src/Magnum/Trade/MeshData.cpp
  2. 13
      src/Magnum/Trade/MeshData.h
  3. 4
      src/Magnum/Trade/Test/MeshDataTest.cpp

7
src/Magnum/Trade/MeshData.cpp

@ -26,6 +26,9 @@
#include "MeshData.h"
#include <Corrade/Utility/Algorithms.h>
#ifndef CORRADE_NO_ASSERT
#include <Corrade/Utility/Format.h>
#endif
#include "Magnum/Math/Color.h"
#include "Magnum/Math/PackingBatch.h"
@ -62,8 +65,8 @@ MeshAttributeData::MeshAttributeData(const MeshAttribute name, const VertexForma
because I feel that makes more sense than duplicating the full assert
logic */
/** @todo support zero / negative stride? would be hard to transfer to GL */
CORRADE_ASSERT(data.empty() || isVertexFormatImplementationSpecific(format) || std::ptrdiff_t(vertexFormatSize(format)) <= data.stride(),
"Trade::MeshAttributeData: expected stride to be positive and enough to fit" << format << Debug::nospace << ", got" << data.stride(), );
CORRADE_ASSERT(data.empty() || isVertexFormatImplementationSpecific(format) || std::ptrdiff_t(vertexFormatSize(format))*(arraySize ? arraySize : 1) <= data.stride(),
"Trade::MeshAttributeData: expected stride to be positive and enough to fit" << format << Debug::nospace << (arraySize ? Utility::format("[{}]", arraySize).data() : "") << Debug::nospace << ", got" << data.stride(), );
}
MeshAttributeData::MeshAttributeData(const MeshAttribute name, const VertexFormat format, UnsignedShort arraySize, const Containers::StridedArrayView2D<const char>& data) noexcept: MeshAttributeData{name, format, arraySize, Containers::StridedArrayView1D<const void>{{data.data(), ~std::size_t{}}, data.size()[0], data.stride()[0]}, nullptr} {

13
src/Magnum/Trade/MeshData.h

@ -359,10 +359,10 @@ class MAGNUM_TRADE_EXPORT MeshAttributeData {
* @param arraySize Array size
* @param data Attribute data
*
* Expects that @p data stride is large enough to fit @p type, @p type
* corresponds to @p name and @p arraySize is zero for builtin
* attributes. Passing @cpp 0 @ce to @p arraySize is equivalent to
* calling the above overload.
* Expects that @p data stride is large enough to fit all @p arraySize
* items of @p type, @p type corresponds to @p name and @p arraySize is
* zero for builtin attributes. Passing @cpp 0 @ce to @p arraySize is
* equivalent to calling the above overload.
*/
explicit MeshAttributeData(MeshAttribute name, VertexFormat format, UnsignedShort arraySize, const Containers::StridedArrayView1D<const void>& data) noexcept;
@ -475,6 +475,11 @@ class MAGNUM_TRADE_EXPORT MeshAttributeData {
* Additionally, for even more flexibility, the @p vertexCount can be
* overriden at @ref MeshData construction time, however all attributes
* are still required to have the same vertex count to catch accidents.
*
* Note that due to the @cpp constexpr @ce nature of this constructor,
* no @p format / @p arraySize checks against @p stride can be done.
* You're encouraged to use the @ref MeshAttributeData(MeshAttribute, VertexFormat, const Containers::StridedArrayView1D<const void>&)
* constructor if you want additional safeguards.
* @see @ref isOffsetOnly(), @ref arraySize(),
* @ref data(Containers::ArrayView<const void>) const
*/

4
src/Magnum/Trade/Test/MeshDataTest.cpp

@ -750,6 +750,7 @@ void MeshDataTest::constructAttributeWrongStride() {
std::ostringstream out;
Error redirectError{&out};
MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector3, Containers::arrayCast<const char>(positionData)};
MeshAttributeData{meshAttributeCustom(1), VertexFormat::Float, 4, Containers::arrayCast<const Vector3>(positionData)};
/* We need this one to be constexpr, which means there can't be a warning
about stride not matching the size */
MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector3, 0, 3*sizeof(Vector3), 1};
@ -758,6 +759,7 @@ void MeshDataTest::constructAttributeWrongStride() {
MeshAttributeData{65000};
CORRADE_COMPARE(out.str(),
"Trade::MeshAttributeData: expected stride to be positive and enough to fit VertexFormat::Vector3, got 1\n"
"Trade::MeshAttributeData: expected stride to be positive and enough to fit VertexFormat::Float[4], got 12\n"
"Trade::MeshAttributeData: expected stride to be positive and at most 32k, got -16\n"
"Trade::MeshAttributeData: expected stride to be positive and at most 32k, got 65000\n"
"Trade::MeshAttributeData: at most 32k padding supported, got 65000\n"
@ -931,7 +933,7 @@ void MeshDataTest::constructArrayAttributeNotAllowed() {
/* This is not */
std::ostringstream out;
Error redirectError{&out};
MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector2, 3, Containers::arrayView(positionData)};
MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector2b, 3, Containers::arrayView(positionData)};
MeshAttributeData{meshAttributeCustom(35), vertexFormatWrap(0xdead), 3, Containers::arrayView(positionData)};
MeshAttributeData{MeshAttribute::Position, positions2D};
MeshAttributeData{MeshAttribute::Position, VertexFormat::Vector2, 3, positions2Dchar};

Loading…
Cancel
Save