diff --git a/src/ObjWriting/XModel/Gltf/GltfWriter.cpp b/src/ObjWriting/XModel/Gltf/GltfWriter.cpp index ecac06eba..9ad3e2df2 100644 --- a/src/ObjWriting/XModel/Gltf/GltfWriter.cpp +++ b/src/ObjWriting/XModel/Gltf/GltfWriter.cpp @@ -10,6 +10,7 @@ #include #include +#include #include using namespace gltf; @@ -53,15 +54,28 @@ namespace quat[3] = eigenQuat.w(); } - void LhcToRhcIndices(unsigned short* indices) + template 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::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(sizeof(unsigned short) * object.m_faces.size() * 3u); + indicesBufferView.byteLength = static_cast(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(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(&bufferData[currentBufferOffset]); - faceIndices[0] = static_cast(face.vertexIndex[0]); - faceIndices[1] = static_cast(face.vertexIndex[1]); - faceIndices[2] = static_cast(face.vertexIndex[2]); - LhcToRhcIndices(faceIndices); + if (bigIndices) + { + auto* faceIndices = reinterpret_cast(&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(&bufferData[currentBufferOffset]); + faceIndices[0] = static_cast(face.vertexIndex[0]); + faceIndices[1] = static_cast(face.vertexIndex[1]); + faceIndices[2] = static_cast(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; diff --git a/test/ObjWritingTests/XModel/Gltf/GltfWriterTest.cpp b/test/ObjWritingTests/XModel/Gltf/GltfWriterTest.cpp new file mode 100644 index 000000000..b731f7e4e --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include + +using namespace gltf; + +namespace +{ + class MockGltfOutput final : public Output + { + public: + std::optional 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(buffer); + m_buffer.assign(bytes, bytes + bufferSize); + } + + void Finalize() const override {} + + mutable nlohmann::ordered_json m_json; + mutable std::vector 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() == 5125u); + + const auto bufferViewIndex = indexAccessor->at("bufferView").get(); + const auto byteOffset = output.m_json.at("bufferViews").at(bufferViewIndex).at("byteOffset").get(); + + 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); +}