fix(gltf): add HAS_COLORS define for COLOR_0 vertex attribute#2503
Open
nokonoko1203 wants to merge 1 commit intovisgl:masterfrom
Open
fix(gltf): add HAS_COLORS define for COLOR_0 vertex attribute#2503nokonoko1203 wants to merge 1 commit intovisgl:masterfrom
nokonoko1203 wants to merge 1 commit intovisgl:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
When a glTF mesh contains a
COLOR_0vertex attribute (e.g. point cloud data with per-vertex colors),parsePBRMaterial()did not set theHAS_COLORSshader define. The existing code already handled analogous attributes using the same#ifdefpattern.Without HAS_COLORS, the vertex shader's #ifdef HAS_COLORS block was excluded at compile time, so the in vec3 colors declaration was absent. The COLOR_0 data was uploaded to the GPU but never read by the shader — all vertices were rendered with the default instance color (white).
Downstream consumers such as deck.gl's ScenegraphLayer do not currently handle per-vertex colors from glTF meshes, but would need this define to conditionally declare vertex attribute inputs and blend per-vertex colors with instance colors (e.g. for rendering colored point clouds via 3D Tiles).
The Khronos https://github.com/KhronosGroup/glTF-Sample-Renderer also uses the same compile-time define pattern (HAS_COLOR_0_VEC3 / HAS_COLOR_0_VEC4) for vertex color support.
Change List