Page MenuHome

Draco library for glTF export
ClosedPublic

Authored by Benjamin Schmithüsen (UX3D-schmithuesen) on Mar 12 2019, 11:59 AM.

Diff Detail

Repository
rB Blender

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Mar 12 2019, 6:59 PM

I get this build error, with cmake 3.10.2 on Linux. It's because these contain only .h files, and don't actually need to be linked.

CMake Error: CMake can not determine linker language for target: draco_compression_mesh_traverser
CMake Error: CMake can not determine linker language for target: draco_enc_config

If it's not too complex, we would prefer the CMakeLists.txt that compiles draco itself to be a custom one that use Blender functions like blender_add_lib. An example of this is in extern/glog/CMakeLists.txt.

The reason is that we want all libraries to compiled with the same compiler flags, debug options and so on. If there is a draco project embedded in the blender project with its own mechanism it's harder for us to ensure it works as we need it on all platforms.

CMakeLists.txt
780

This should use option() See how other options are defined a bit higher in this file.

extern/CMakeLists.txt
45

Perhaps just call the subdirectory draco, in case we use it outside of gltf in the future.

extern/gltf-draco/CMakeLists.txt
1–2 ↗(On Diff #14126)

Does this really need its own project and minimum cmake version?

6 ↗(On Diff #14126)

The installation should be done in source/creator/CMakeLists.txt, without the hardcoded Blender and Python version.

This revision now requires changes to proceed.Mar 12 2019, 6:59 PM

• replace Google draco CMakeLists.txt with custom one that uses blender_add_lib
• rename folders and libraries
• simplify extern/draco/CMakeLists.txt (no more own project)
• move library installation to source/creator/CMakeLists.txt
• make WITH_DRACO an “option”

I think I covered all your comments @Brecht Van Lommel (brecht)

Is this based on the master branch of the Draco repository, or the gltf_2.0_draco_extension branch? Maybe they're both in sync and tracking the same branch anyway, but I'm not sure and we may need to double-check.

  • Remove unintended changes included in patch.
  • Ensure WITH_DRACO is set in all the right places.
  • Add license headers.

This seems ready to commit, but will wait for you to check if the branch is correct.

This revision is now accepted and ready to land.Apr 8 2019, 11:29 PM

Is this based on the master branch of the Draco repository, or the gltf_2.0_draco_extension branch? Maybe they're both in sync and tracking the same branch anyway, but I'm not sure and we may need to double-check.

gltf_2.0_draco_extenion is on the latest release while master is slightly ahead, but they are not diverging. This diff based on 063994c362871d6f149c24c669122e4ef3fa8196 which is the 1.3.4 release.

@Brecht Van Lommel (brecht) Thank's for approving, we can merge this now!

This revision was automatically updated to reflect the committed changes.
bartus sz (bartus) added inline comments.
source/creator/CMakeLists.txt
645

undefined DRACO_LIB_NAME breaks install script at source/creator/cmake_instal.cmake

236 if("x${CMAKE_INSTALL_COMPONENT}x" STREQUAL "xUnspecifiedx" OR NOT CMAKE_INSTALL_COMPONENT)
237   file(INSTALL DESTINATION "${CMAKE_INSTALL_PREFIX}/share/blender/2.80/python/lib/python3.7/site-packages" TYPE PROGRAM MESSAGE_LAZY FILES "/tmp/blender-2.8-git/src/blender-build/lib/lib.so")
238 endif()