Skip to content

Commit

Permalink
Change: Assert when reading accessor with different AccessorType
Browse files Browse the repository at this point in the history
Previously, when a user tried to read a Scalar type from a Vec3 accessor for example, the accessor tools would silently return.
  • Loading branch information
spnda committed May 20, 2024
1 parent 2439198 commit 7c21841
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 13 deletions.
17 changes: 11 additions & 6 deletions docs/tools.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/forenoonwatch>`_ with the help of `Eearslya <https://github.com/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 <bufferdataadapter>`.

.. 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
=============

Expand All @@ -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 <bufferdataadapter>`.

getAccessorElement
==================

Expand Down
13 changes: 6 additions & 7 deletions include/fastgltf/tools.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ class IterableAccessor {
using iterator = AccessorIterator<ElementType, BufferDataAdapter>;

explicit IterableAccessor(const Asset& asset, const Accessor& accessor, const BufferDataAdapter& adapter) : asset(asset), accessor(accessor) {
assert(accessor.type == ElementTraits<ElementType>::type && "The destination type needs to have the same AccessorType as the accessor.");
componentType = accessor.componentType;

const auto& view = asset.bufferViews[*accessor.bufferViewIndex];
Expand Down Expand Up @@ -492,14 +493,16 @@ FASTGLTF_EXPORT template <typename ElementType, typename BufferDataAdapter = Def
#if FASTGLTF_HAS_CONCEPTS
requires Element<ElementType>
#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<ElementType>;
static_assert(Traits::type != AccessorType::Invalid, "Accessor traits must provide a valid Accessor Type");
static_assert(std::is_default_constructible_v<ElementType>, "Element type must be default constructible");
static_assert(std::is_constructible_v<ElementType>, "Element type must be constructible");
static_assert(std::is_move_assignable_v<ElementType>, "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);

Expand Down Expand Up @@ -558,9 +561,7 @@ void iterateAccessor(const Asset& asset, const Accessor& accessor, Functor&& fun
static_assert(std::is_constructible_v<ElementType>, "Element type must be constructible");
static_assert(std::is_move_assignable_v<ElementType>, "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);
Expand Down Expand Up @@ -658,9 +659,7 @@ void copyFromAccessor(const Asset& asset, const Accessor& accessor, void* dest,
static_assert(std::is_constructible_v<ElementType>, "Element type must be constructible");
static_assert(std::is_move_assignable_v<ElementType>, "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<std::byte*>(dest);

Expand Down

0 comments on commit 7c21841

Please sign in to comment.