fix(GltfWriter): emit 32-bit glTF indices for models with >65535 vertices

src/ObjWriting/XModel/Gltf/GltfWriter.cpp

@@ -10,6 +10,7 @@
 
 #include <algorithm>
 #include <format>
+#include <limits>
 #include <numbers>
 
 using namespace gltf;
@@ -53,15 +54,28 @@ namespace
         quat[3] = eigenQuat.w();
     }
 
-    void LhcToRhcIndices(unsigned short* indices)
+    template<typename IndexType> void LhcToRhcIndices(IndexType* indices)
     {
-        const unsigned short two[3]{indices[0], indices[1], indices[2]};
+        const IndexType two[3]{indices[0], indices[1], indices[2]};
 
         indices[0] = two[2];
         indices[1] = two[1];
         indices[2] = two[0];
     }
 
+    // glTF index accessors must be able to address every vertex of the model's single shared vertex
+    // buffer. XModelFace::vertexIndex is 32-bit, so a model with more than 0xFFFF vertices cannot be
+    // represented with UNSIGNED_SHORT indices and must fall back to UNSIGNED_INT.
+    bool NeedsBigIndices(const XModelCommon& xmodel)
+    {
+        return xmodel.m_vertices.size() > std::numeric_limits<unsigned short>::max();
+    }
+
+    size_t GetIndexSize(const XModelCommon& xmodel)
+    {
+        return NeedsBigIndices(xmodel) ? sizeof(unsigned int) : sizeof(unsigned short);
+    }
+
     void LhcToRhcMatrix(Eigen::Matrix4f& matrix)
     {
         const Eigen::Matrix4f convertMatrix({
@@ -468,7 +482,7 @@ namespace
                 JsonBufferView indicesBufferView;
                 indicesBufferView.buffer = 0u;
                 indicesBufferView.byteOffset = bufferOffset;
-                indicesBufferView.byteLength = static_cast<unsigned>(sizeof(unsigned short) * object.m_faces.size() * 3u);
+                indicesBufferView.byteLength = static_cast<unsigned>(GetIndexSize(xmodel) * object.m_faces.size() * 3u);
                 indicesBufferView.target = JsonBufferViewTarget::ELEMENT_ARRAY_BUFFER;
                 bufferOffset += indicesBufferView.byteLength;
 
@@ -553,7 +567,8 @@ namespace
 
                 JsonAccessor indicesAccessor;
                 indicesAccessor.bufferView = m_first_index_buffer_view + i;
-                indicesAccessor.componentType = JsonAccessorComponentType::UNSIGNED_SHORT;
+                indicesAccessor.componentType =
+                    NeedsBigIndices(xmodel) ? JsonAccessorComponentType::UNSIGNED_INT : JsonAccessorComponentType::UNSIGNED_SHORT;
                 indicesAccessor.count = static_cast<unsigned>(object.m_faces.size() * 3u);
                 indicesAccessor.type = JsonAccessorType::SCALAR;
 
@@ -691,17 +706,31 @@ namespace
                 currentBufferOffset += sizeof(float) * 16u * xmodel.m_bones.size();
             }
 
+            const auto bigIndices = NeedsBigIndices(xmodel);
             for (const auto& object : xmodel.m_objects)
             {
                 for (const auto& face : object.m_faces)
                 {
-                    auto* faceIndices = reinterpret_cast<unsigned short*>(&bufferData[currentBufferOffset]);
-                    faceIndices[0] = static_cast<unsigned short>(face.vertexIndex[0]);
-                    faceIndices[1] = static_cast<unsigned short>(face.vertexIndex[1]);
-                    faceIndices[2] = static_cast<unsigned short>(face.vertexIndex[2]);
-                    LhcToRhcIndices(faceIndices);
+                    if (bigIndices)
+                    {
+                        auto* faceIndices = reinterpret_cast<unsigned int*>(&bufferData[currentBufferOffset]);
+                        faceIndices[0] = face.vertexIndex[0];
+                        faceIndices[1] = face.vertexIndex[1];
+                        faceIndices[2] = face.vertexIndex[2];
+                        LhcToRhcIndices(faceIndices);
 
-                    currentBufferOffset += sizeof(unsigned short) * 3u;
+                        currentBufferOffset += sizeof(unsigned int) * 3u;
+                    }
+                    else
+                    {
+                        auto* faceIndices = reinterpret_cast<unsigned short*>(&bufferData[currentBufferOffset]);
+                        faceIndices[0] = static_cast<unsigned short>(face.vertexIndex[0]);
+                        faceIndices[1] = static_cast<unsigned short>(face.vertexIndex[1]);
+                        faceIndices[2] = static_cast<unsigned short>(face.vertexIndex[2]);
+                        LhcToRhcIndices(faceIndices);
+
+                        currentBufferOffset += sizeof(unsigned short) * 3u;
+                    }
                 }
             }
 
@@ -728,7 +757,7 @@ namespace
 
             for (const auto& object : xmodel.m_objects)
             {
-                result += object.m_faces.size() * sizeof(unsigned short) * 3u;
+                result += object.m_faces.size() * GetIndexSize(xmodel) * 3u;
             }
 
             return result;

test/ObjWritingTests/XModel/Gltf/GltfWriterTest.cpp

@@ -0,0 +1,107 @@
+#include "XModel/Gltf/GltfWriter.h"
+
+#include "XModel/Gltf/GltfOutput.h"
+#include "XModel/XModelCommon.h"
+
+#include <catch2/catch_test_macros.hpp>
+#include <cstdint>
+#include <cstring>
+#include <nlohmann/json.hpp>
+#include <optional>
+#include <string>
+#include <vector>
+
+using namespace gltf;
+
+namespace
+{
+    class MockGltfOutput final : public Output
+    {
+    public:
+        std::optional<std::string> CreateBufferUri(const void*, size_t) const override
+        {
+            return "buffer.bin";
+        }
+
+        void EmitJson(const nlohmann::ordered_json& json) const override
+        {
+            m_json = json;
+        }
+
+        void EmitBuffer(const void* buffer, const size_t bufferSize) const override
+        {
+            const auto* bytes = static_cast<const uint8_t*>(buffer);
+            m_buffer.assign(bytes, bytes + bufferSize);
+        }
+
+        void Finalize() const override {}
+
+        mutable nlohmann::ordered_json m_json;
+        mutable std::vector<uint8_t> m_buffer;
+    };
+
+    XModelCommon GivenModelWithVertexCount(const unsigned vertexCount, const unsigned highIndex)
+    {
+        XModelCommon xmodel;
+        xmodel.m_name = "big_model";
+
+        XModelMaterial material;
+        material.ApplyDefaults();
+        material.name = "mtl";
+        xmodel.m_materials.emplace_back(std::move(material));
+
+        xmodel.m_vertices.resize(vertexCount, XModelVertex{});
+
+        XModelObject object;
+        object.name = "obj";
+        object.materialIndex = 0u;
+        object.m_faces.push_back(XModelFace{{0u, 1u, highIndex}});
+        xmodel.m_objects.emplace_back(std::move(object));
+
+        return xmodel;
+    }
+
+    const nlohmann::ordered_json* FindIndexAccessor(const nlohmann::ordered_json& gltf)
+    {
+        for (const auto& accessor : gltf.at("accessors"))
+        {
+            if (accessor.at("type") == "SCALAR")
+                return &accessor;
+        }
+        return nullptr;
+    }
+} // namespace
+
+TEST_CASE("GltfWriter: Uses 32-bit indices for models with more than 65535 vertices", "[gltf][xmodel][objwriter]")
+{
+    // A single vertex buffer is shared by every primitive, so an index >= 65536 cannot be encoded as
+    // UNSIGNED_SHORT. Stock behaviour truncated it modulo 65536, silently corrupting the mesh.
+    constexpr auto vertexCount = 70000u;
+    constexpr auto highIndex = 69999u;
+
+    const auto xmodel = GivenModelWithVertexCount(vertexCount, highIndex);
+
+    MockGltfOutput output;
+    const auto writer = Writer::CreateWriter(&output, "IW4", "test");
+    writer->Write(xmodel);
+
+    const auto* indexAccessor = FindIndexAccessor(output.m_json);
+    REQUIRE(indexAccessor != nullptr);
+
+    // 5125 == UNSIGNED_INT. Before the fix this was a hardcoded 5123 (UNSIGNED_SHORT).
+    REQUIRE(indexAccessor->at("componentType").get<unsigned>() == 5125u);
+
+    const auto bufferViewIndex = indexAccessor->at("bufferView").get<size_t>();
+    const auto byteOffset = output.m_json.at("bufferViews").at(bufferViewIndex).at("byteOffset").get<size_t>();
+
+    REQUIRE(output.m_buffer.size() >= byteOffset + sizeof(unsigned int) * 3u);
+
+    unsigned int writtenIndices[3];
+    std::memcpy(writtenIndices, &output.m_buffer[byteOffset], sizeof(writtenIndices));
+
+    // Winding is reversed for the LHC -> RHC conversion, so face {0, 1, highIndex} is stored as
+    // {highIndex, 1, 0}. The high index must survive intact rather than wrapping to highIndex % 65536.
+    REQUIRE(writtenIndices[0] == highIndex);
+    REQUIRE(writtenIndices[1] == 1u);
+    REQUIRE(writtenIndices[2] == 0u);
+}