google / draco

Draco is a library for compressing and decompressing 3D geometric meshes and point clouds. It is intended to improve the storage and transmission of 3D graphics.
https://google.github.io/draco/
Apache License 2.0
6.49k stars 963 forks source link

About checking "points_64 > faces_ 64 * 3" in mesh_sequential_decoder.cc #474

Closed ft-lab closed 4 years ago

ft-lab commented 6 years ago

I'm sorry for sentences that are difficult to understand by automatic translation.

Hi, I am developing glTF Importer/Exporter in C++. Using draco 1.3.4.

On Encode, For Morph Targets implementation, encoder.SetEncodingMethod(draco::MESH_SEQUENTIAL_ENCODING); using.

When multiple primitives in one mesh and sharing vertices data, the indices and vertices data stored in draco are redundant. (It also includes unreferenced vertex data)

When importing Draco compressed glTF file under this condition, Sometimes reading failed. decoder.DecodeMeshFromBuffer(&buffer) returns 'Failed to decode geometry data.'.

As a result of investigation, it seemed to be caused by the following.

MeshSequentialDecoder::DecodeConnectivity() in src/draco/compression/mesh/mesh_sequential_decoder.cc

if (points_64 > faces_64 * 3) return false;

Here points_64 was a much bigger number than faces_64. I thought that it would be better to comment out this part, What do you think.

I created a sample glb file for confirmation (Using my exporter).

https://ft-lab.github.io/gltf/MorphTargets/face_test_vert_share_draco.glb

The following conditions are specified.

If you look at this glb file with Babylon.js(3.2.0) on VSCode(glTF Tools), you can see the error 'Failed to decode geometry data.'.

FrankGalligan commented 4 years ago

Sorry but the current Draco spec does not cover morph targets.