Fix: Also adjust total matrix size for byte index

The total size of a single matrix wasn't changed in the last commit but needs to be as there's padding after each column, which affects the total size.
spnda · Feb 1, 2024 · a27b881 · a27b881
1 parent bd5291d
commit a27b881
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 4 deletions.
diff --git a/include/fastgltf/tools.hpp b/include/fastgltf/tools.hpp
@@ -186,10 +186,10 @@ constexpr DestType convertComponent(const SourceType& source, bool normalized) {
 	return static_cast<DestType>(source);
 }
 
-template <typename DestType, typename SourceType, AccessorType AccessorType, std::size_t Index>
+template <typename DestType, typename SourceType, AccessorType ElementAccessorType, std::size_t Index>
 constexpr DestType convertComponent(const std::byte* bytes, bool normalized) {
-    if constexpr (AccessorType == AccessorType::Mat2 || AccessorType == AccessorType::Mat3 || AccessorType == AccessorType::Mat4) {
-        const auto rowCount = getElementRowCount(AccessorType);
+    if constexpr (isMatrix(ElementAccessorType)) {
+        const auto rowCount = getElementRowCount(ElementAccessorType);
         const auto componentSize = sizeof(SourceType);
         if constexpr ((rowCount * componentSize) % 4 != 0) {
             // There's only four cases where this happens, but the glTF spec requires us to insert some padding for each column.

diff --git a/include/fastgltf/types.hpp b/include/fastgltf/types.hpp
@@ -306,23 +306,37 @@ namespace fastgltf {
     constexpr auto getElementRowCount(AccessorType type) noexcept {
         switch (type) {
             case AccessorType::Mat2:
+            case AccessorType::Vec2:
                 return 2;
             case AccessorType::Mat3:
+            case AccessorType::Vec3:
                 return 3;
             case AccessorType::Mat4:
+            case AccessorType::Vec4:
                 return 4;
             default:
                 return 1;
         }
     }
 
+    constexpr bool isMatrix(AccessorType type) noexcept {
+        return type == AccessorType::Mat2 || type == AccessorType::Mat3 || type == AccessorType::Mat4;
+    }
+
     constexpr auto getComponentBitSize(ComponentType componentType) noexcept {
     	static_assert(std::is_same_v<std::underlying_type_t<ComponentType>, std::uint32_t>);
     	return static_cast<std::uint16_t>(to_underlying(componentType) >> 16U);
     }
 
     constexpr auto getElementByteSize(AccessorType type, ComponentType componentType) noexcept {
-        return static_cast<std::uint16_t>(getNumComponents(type)) * (getComponentBitSize(componentType) / 8);
+        const auto componentSize = getComponentBitSize(componentType) / 8;
+        auto numComponents = getNumComponents(type);
+        const auto rowCount = getElementRowCount(type);
+        if (isMatrix(type) && (rowCount * componentSize) % 4 != 0) {
+            // Matrices need extra padding per-column which affects their size.
+            numComponents += rowCount * (4 - (rowCount % 4));
+        }
+        return static_cast<std::uint16_t>(numComponents) * componentSize;
     }
 
     /**

diff --git a/tests/accessor_tests.cpp b/tests/accessor_tests.cpp
@@ -64,6 +64,7 @@ TEST_CASE("Test matrix data padding", "[gltf-tools]") {
        1, 2,
        3, 4
     }};
+    REQUIRE(fastgltf::getElementByteSize(fastgltf::AccessorType::Mat2, fastgltf::ComponentType::UnsignedShort) == unpaddedMat2.size() * sizeof(std::uint16_t));
     auto umat2 = fastgltf::internal::getAccessorElementAt<glm::mat2x2>(
             fastgltf::ComponentType::UnsignedShort,
             reinterpret_cast<const std::byte*>(unpaddedMat2.data()));
@@ -75,6 +76,7 @@ TEST_CASE("Test matrix data padding", "[gltf-tools]") {
         1, 2, 0, 0,
         3, 4, 0, 0
     }};
+    REQUIRE(fastgltf::getElementByteSize(fastgltf::AccessorType::Mat2, fastgltf::ComponentType::UnsignedByte) == paddedMat2.size());
     auto mat2 = fastgltf::internal::getAccessorElementAt<glm::mat2x2>(
             fastgltf::ComponentType::UnsignedByte,
             reinterpret_cast<const std::byte*>(paddedMat2.data()));
@@ -86,6 +88,7 @@ TEST_CASE("Test matrix data padding", "[gltf-tools]") {
         4, 5, 6, 0,
         7, 8, 9, 0
     }};
+    REQUIRE(fastgltf::getElementByteSize(fastgltf::AccessorType::Mat3, fastgltf::ComponentType::UnsignedByte) == paddedMat3.size());
     auto mat3 = fastgltf::internal::getAccessorElementAt<glm::mat3x3>(
             fastgltf::ComponentType::UnsignedByte,
             reinterpret_cast<const std::byte*>(paddedMat3.data()));
@@ -99,6 +102,7 @@ TEST_CASE("Test matrix data padding", "[gltf-tools]") {
         4, 5, 6, 0,
         7, 8, 9, 0
     }};
+    REQUIRE(fastgltf::getElementByteSize(fastgltf::AccessorType::Mat3, fastgltf::ComponentType::UnsignedShort) == paddedMat3.size() * sizeof(std::uint16_t));
     auto mat3_2 = fastgltf::internal::getAccessorElementAt<glm::mat3x3>(
             fastgltf::ComponentType::UnsignedShort,
             reinterpret_cast<const std::byte*>(padded2BMat3.data()));