Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[glTF] Support meshes without indices #474

Open
footballhead opened this issue May 31, 2024 · 1 comment
Open

[glTF] Support meshes without indices #474

footballhead opened this issue May 31, 2024 · 1 comment
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@footballhead
Copy link
Collaborator

GltfLoader::LoadMeshData currently complains when a mesh lacks indices. However, meshes are allowed to not have indices. 5.24.2 says:

When this is undefined, the primitive defines non-indexed geometry

This would allow BigWheels to load and render:

@footballhead footballhead added the enhancement New feature or request label May 31, 2024
@footballhead footballhead added the good first issue Good for newcomers label Jun 19, 2024
@footballhead
Copy link
Collaborator Author

footballhead commented Sep 13, 2024

There's evidence that this is partly implemented but incomplete. In GltfLoader::LoadMeshData (scene_gltf_loader.cpp):

  • GetFormat() can return FORMAT_UNDEFINED when indices are missing (pGltfPrimitive->indices == nullptr). There's a comment that says: "It's valid for this to be UNDEFINED, means the primitive doesn't have any index data."
  • There's a code path involving genTopologyIndices. This is set to true when indexFormat == grfx::FORMAT_UNDEFINED. This factors into code that populates targetGeometry.

However, there's also an independent check for IsNull() very early on when BatchInfos are being made:

        // We require index data so bail if there isn't index data.
        if (IsNull(pGltfPrimitive->indices)) {
            PPX_ASSERT_MSG(false, "GLTF mesh primitive does not have index data");
            return ppx::ERROR_SCENE_INVALID_SOURCE_GEOMETRY_INDEX_DATA;
        }

Additionally, none of the staging/target buffer sizing code considers the case where indexFormat == grfx::FORMAT_UNDEFINED

footballhead added a commit to footballhead/bigwheels that referenced this issue Oct 22, 2024
There's an early return with error if `IsNull(pGltfPrimitive->indices)`
so genTopology will never be `true`. This can be reverted as part of
issue google#474. Until then, it's confusing and creates a burden for
refactoring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant