search

donmccurdy / three-gltf-viewer

Drag-and-drop preview for glTF 2.0 models in WebGL using three.js.
https://gltf-viewer.donmccurdy.com/
MIT License
2.1k stars 534 forks source link

Problem with Draco and unnormalized vertex colors #91

Closed lilleyse closed 6 years ago

lilleyse commented 6 years ago

Draco encoded attributes whose accessors set normalized : true are not being normalized correctly during rendering, leading to overblown colors in the second image.

There is a similar bug in Cesium (https://github.com/AnalyticalGraphicsInc/cesium/issues/6561) and Babylon - so either something is wrong the second model or it's just an unusual edge case.

vertex_colors.gltf.txt vertex_colors_draco.gltf.txt colors colors_wrong

donmccurdy commented 6 years ago

three.js isn't passing through the accessor.normalized value properly, so there is a bug on our side, but after decoding the data from Draco doesn't match the vertex colors of the other mesh, and I get float32 values instead of uint8, although the accessor is still marked as uint8. Seems like a few things going wrong here. 🙃

# original
[255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255 ]

# draco
[202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 202, 252, 147, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 163, 166, 123, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 56, 147, 89, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 32, 126, 152, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 128, 75, 16, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 217, 139, 126, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 34, 127, 130, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 76, 123, 68, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 107, 81, 84, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255, 255, 0, 0, 255 ]
donmccurdy commented 6 years ago

A fix for what I think is the three.js part of the issue here: https://github.com/mrdoob/three.js/compare/dev...donmccurdy:bug-gltfloader-draco-normalized but it doesn't appear any different with just that.

donmccurdy commented 6 years ago

Ok, the values are fine, just reordered... the problem was that we get a float32 rather than uint8 arraybuffer from Draco. From this issue:

In our current Javascript API we always convert all attribute values to float32 data type...

lilleyse commented 6 years ago

Yeah the reordering of values threw me off a bit too, but I guess it yields better compression that way.

In our current Javascript API we always convert all attribute values to float32 data type...

The API has changed since then and should supports retrieving data of any component type: https://github.com/google/draco/blob/master/src/draco/javascript/emscripten/draco_web_decoder.idl#L221-L236

donmccurdy commented 6 years ago

Ah, so it's the responsibility of the code using the decoder to know the type of each attribute? I guess we can work with that then.

donmccurdy commented 6 years ago

Fixed upstream: https://github.com/mrdoob/three.js/pull/14002