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] Load scenes with UINT8 indices #482

Merged
merged 10 commits into from
Nov 26, 2024

Conversation

footballhead
Copy link
Collaborator

@footballhead footballhead commented Jun 11, 2024

Load scenes that contain 8-bit index buffers. How the index buffer ends up being packed depends on the capabilities of the device:

  • On Vulkan with VK_KHR_index_type_uint8 extension, the UINT8 indices can be packed into a INDEX_TYPE_UINT8 buffer. This is the optimal case as it doesn't require additional memory.
  • Otherwise, the UINT8 indices need to be repacked into UINT16 indices. This comes at the cost of a larger GPU buffer.

This allows us to load more Khronos glTF-Sample-Assets like TextureCoordinateTest.

image

Fixes #453

@footballhead footballhead requested a review from apazylbe June 11, 2024 22:01
Without extensions, Vulkan only supports UINT16 and UINT32 index
buffers. However, glTF allows UINT8 index buffers. We can support scenes
with UINT8 indices by repacking them into a UINT16 buffer instead. This
comes at the cost of a larger GPU buffer.

This allows us to load more Khronos glTF-Sample-Assets like
TextureCoordinateTest.

Fixes google#453
@Keenuts
Copy link
Member

Keenuts commented Jun 12, 2024

Seems like the required extension is https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_8bit_storage.html
Promoted to core in VK 1.2. BigWheels default is Vulkan1.2, so I'd say we should probably leave the buffer as-is, and request the extension?

@footballhead
Copy link
Collaborator Author

Sg I'll look at adding UINT8 support to Geometry instead

@footballhead
Copy link
Collaborator Author

Actually I'm looking at https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_index_type_uint8.html which I believe is required for VK_INDEX_TYPE_UINT8_KHR. That's what I think I need for UINT8 support in Geometry. I don't see that it's been promoted

@Keenuts
Copy link
Member

Keenuts commented Jun 13, 2024

Ahh right, index buffer, not storage buffer, sorry I missed that!
Not promoted, but seems like it's widely supported (>70% on Sasha willems' DB) since it was a VK_EXT_index_type_int8 before.

@apazylbe
Copy link
Collaborator

I tried on three devices I have (1 software lavapipe, and 2 android) and they all support VK_EXT_index_type_uint8. According to https://vulkan.gpuinfo.org/listextensions.php support for the extension is at 62%. Would it make sense to support UINT8 through the extension, or perhaps fall back to repacking if it is not available?

@footballhead
Copy link
Collaborator Author

I like the idea of supporting the extension with this as a fallback. I'll look into how to make that happen.

@footballhead footballhead changed the title [glTF] Load UINT8 indices into UINT16 buffer [glTF] Load scenes with UINT8 indices Sep 17, 2024
@footballhead
Copy link
Collaborator Author

I've incorporated #515 and #486 into this PR so it should be ready for review

@apazylbe apazylbe requested a review from Keenuts September 24, 2024 14:17
@footballhead
Copy link
Collaborator Author

@apazylbe I made some non-trivial changes that I'd like you to review.

@footballhead footballhead requested a review from apazylbe November 4, 2024 22:01
@footballhead
Copy link
Collaborator Author

@apazylbe merge conflicts are solved

@apazylbe apazylbe merged commit 5aa9f65 into google:main Nov 26, 2024
5 checks passed
@footballhead footballhead deleted the gltf_uint8_indices branch November 26, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[glTF] Support accessors with UNSIGNED_BYTE 5121 componentType
3 participants