From 7c21841d937909a392356360c18d050c4ff9c028 Mon Sep 17 00:00:00 2001 From: sean Date: Mon, 20 May 2024 20:11:17 +0200 Subject: [PATCH] Change: Assert when reading accessor with different AccessorType Previously, when a user tried to read a Scalar type from a Vec3 accessor for example, the accessor tools would silently return. --- docs/tools.rst | 17 +++++++++++------ include/fastgltf/tools.hpp | 13 ++++++------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/docs/tools.rst b/docs/tools.rst index 035db7a17..d513eeaf2 100644 --- a/docs/tools.rst +++ b/docs/tools.rst @@ -9,6 +9,17 @@ The header contains various functions and utilities for reading, copying, and co All of these tools also directly support sparse accessors to help add support for these without having to understand how they work. This functionality was written by `forenoonwatch `_ with the help of `Eearslya `_ and me. +.. warning:: + + By default, these functions will only be able to load from buffers where the source is either a ``sources::ByteView``, a ``sources::Array``, or a ``sources::Vector``. + For other data sources, you'll need to provide a functor similar to the already provided ``DefaultBufferDataAdapter`` to the last parameter of each function. + For more detailed documentation about this see :ref:`this section `. + +.. warning:: + + When an accessor contains, for example, Vec3 data, the functions will assert when you try to specify a type which is not a vector of 3. + That means that the type you specify in the template for ``T`` needs to have ``AccessorType::Vec3`` specified for its ``ElementTraits``. + ElementTraits ============= @@ -33,12 +44,6 @@ For example, ``glm::vec3`` would be a vector of 3 floats, which would be defined **fastgltf** also includes its own vector and matrix types, which can be used instead. These also have a ``ElementTraits`` specialization included in the standard header. -.. warning:: - - By default, these functions will only be able to load from buffers where the source is either a ``sources::ByteView``, a ``sources::Array``, or a ``sources::Vector``. - For other data sources, you'll need to provide a functor similar to the already provided ``DefaultBufferDataAdapter`` to the last parameter of each function. - For more detailed documentation about this see :ref:`this section `. - getAccessorElement ================== diff --git a/include/fastgltf/tools.hpp b/include/fastgltf/tools.hpp index 6cc90ac1d..74811beab 100644 --- a/include/fastgltf/tools.hpp +++ b/include/fastgltf/tools.hpp @@ -456,6 +456,7 @@ class IterableAccessor { using iterator = AccessorIterator; explicit IterableAccessor(const Asset& asset, const Accessor& accessor, const BufferDataAdapter& adapter) : asset(asset), accessor(accessor) { + assert(accessor.type == ElementTraits::type && "The destination type needs to have the same AccessorType as the accessor."); componentType = accessor.componentType; const auto& view = asset.bufferViews[*accessor.bufferViewIndex]; @@ -492,7 +493,7 @@ FASTGLTF_EXPORT template #endif -ElementType getAccessorElement(const Asset& asset, const Accessor& accessor, size_t index, +auto getAccessorElement(const Asset& asset, const Accessor& accessor, size_t index, const BufferDataAdapter& adapter = {}) { using Traits = ElementTraits; static_assert(Traits::type != AccessorType::Invalid, "Accessor traits must provide a valid Accessor Type"); @@ -500,6 +501,8 @@ ElementType getAccessorElement(const Asset& asset, const Accessor& accessor, siz static_assert(std::is_constructible_v, "Element type must be constructible"); static_assert(std::is_move_assignable_v, "Element type must be move-assignable"); + assert(accessor.type == Traits::type && "The destination type needs to have the same AccessorType as the accessor."); + if (accessor.sparse) { auto indicesBytes = adapter(asset, accessor.sparse->indicesBufferView).subspan(accessor.sparse->indicesByteOffset); @@ -558,9 +561,7 @@ void iterateAccessor(const Asset& asset, const Accessor& accessor, Functor&& fun static_assert(std::is_constructible_v, "Element type must be constructible"); static_assert(std::is_move_assignable_v, "Element type must be move-assignable"); - if (accessor.type != Traits::type) { - return; - } + assert(accessor.type == Traits::type && "The destination type needs to have the same AccessorType as the accessor."); if (accessor.sparse && accessor.sparse->count > 0) { auto indicesBytes = adapter(asset, accessor.sparse->indicesBufferView).subspan(accessor.sparse->indicesByteOffset); @@ -658,9 +659,7 @@ void copyFromAccessor(const Asset& asset, const Accessor& accessor, void* dest, static_assert(std::is_constructible_v, "Element type must be constructible"); static_assert(std::is_move_assignable_v, "Element type must be move-assignable"); - if (accessor.type != Traits::type) { - return; - } + assert(accessor.type == Traits::type && "The destination type needs to have the same AccessorType as the accessor."); auto* dstBytes = static_cast(dest);