spnda / fastgltf

A modern C++17 glTF 2.0 library focused on speed, correctness, and usability
https://fastgltf.readthedocs.io/v0.8.x/
MIT License
312 stars 48 forks source link

Integration issues with the downloaded simdjson when using find_package() #35

Closed Bonkt closed 10 months ago

Bonkt commented 11 months ago

Im trying to build this library and use it in my project with find_package(), but get the following CMake error: I'm on ubuntu and generally find_package() works fine for most libraries im trying to build, but i'm not an cmake expert. I can't really find what im doing wrong or missing, and my skills is lacking when trying to trace back through the internal CMakeLists.txt.

How i installed the library: (with simdjson downloaded = ON)

  1. mkdir build; cd build;
  2. cmake..
  3. make
  4. sudo make install Am i missing any steps? CMake obviously finds the fastgltfConfig file, but there are problems with the simdjson installation. When building my own project with this library i get:

    CMake Error at CMakeLists.txt:71 (find_package):
    Found package configuration file:
    
    /usr/local/lib/cmake/fastgltf/fastgltfConfig.cmake
    
    but it set fastgltf_FOUND to FALSE so package "fastgltf" is considered to
    be NOT FOUND.  Reason given by package:
    
    The following imported targets are referenced, but are missing:
    fastgltf::fastgltf_simdjson

Output when running make install:

Consolidate compiler generated dependencies of target fastgltf_simdjson
[ 40%] Built target fastgltf_simdjson
Consolidate compiler generated dependencies of target fastgltf
[100%] Built target fastgltf
Install the project...
-- Install configuration: ""
-- Installing: /usr/local/include/simdjson.h
-- Installing: /usr/local/lib/libfastgltf_simdjson.a
-- Installing: /usr/local/lib/cmake/fastgltf/fastgltf_simdjsonTargets.cmake
-- Installing: /usr/local/lib/cmake/fastgltf/fastgltf_simdjsonTargets-noconfig.cmake
-- Up-to-date: /usr/local/include/fastgltf/base64.hpp
-- Up-to-date: /usr/local/include/fastgltf/glm_element_traits.hpp
-- Up-to-date: /usr/local/include/fastgltf/parser.hpp
-- Up-to-date: /usr/local/include/fastgltf/tools.hpp
-- Up-to-date: /usr/local/include/fastgltf/types.hpp
-- Up-to-date: /usr/local/include/fastgltf/util.hpp
-- Installing: /usr/local/lib/libfastgltf.a
-- Old export file "/usr/local/lib/cmake/fastgltf/fastgltfConfig.cmake" will be replaced.  Removing files [/usr/local/lib/cmake/fastgltf/fastgltfConfig-noconfig.cmake].
-- Installing: /usr/local/lib/cmake/fastgltf/fastgltfConfig.cmake
-- Installing: /usr/local/lib/cmake/fastgltf/fastgltfConfig-noconfig.cmake
spnda commented 11 months ago

I've figured out where this issue is coming from. I export the fastgltf_simdjson target into a file called fastgltf_simdjsonTargets.cmake in the cmake/fastgltf directory. (1) the file should be suffixed with Config and not Targets. (2) CMake does not look into the fastgltf directory when looking for a package with a different name. Therefore, I'd have to export the file into a separate directory, fastgltf_simdjson and call it appropriately. However, I dislike the configs being split into two directories, which is why I will investigate other solutions to this problem tomorrow. Perhaps I'll have to fuse the configs from both targets, but I am not sure how I would achieve that.

Thank you for bringing this to my attention.

TareHimself commented 10 months ago

Did u find another solution ?

spnda commented 10 months ago

@TareHimself I noticed you put a commit on your fork of fastgltf. Does that fix work generally with the subfolder for simdjson? (share/fastgltf/simdjson). I thought there had to be a dedicated folder, which is why I was reluctant because I might overwrite simdjson's own installation targets. If it does work, would you mind if I cherry-pick that commit onto my repository?

TareHimself commented 10 months ago

Yes it does , I can open a pr

TareHimself commented 10 months ago

I have simdjson in its own folder under fastgltf so its cmake/fastgltf/simdjson . I have no clue if it will work if I put both of them in the same folder

spnda commented 10 months ago

I only just tested this locally on my Mac and find_package(fastgltf CONFIG REQUIRED) still fails when the simdjson configs are in a subdirectory. Are you sure it works, and on what platform are you with which CMake version?

TareHimself commented 10 months ago

Windows I do

find_package(fastgltf_simdjson REQUIRED)
find_package(fastgltf CONFIG REQUIRED)

After adding both folders to cmake path

 # FastGLTF
macro(GetFastGLTF VERSION RESULT)

  function(BuildFastGLTF B_TYPE B_SRC B_DEST)
    # execute_process(
    #   COMMAND ${CMAKE_COMMAND} -DCMAKE_BUILD_TYPE=${B_TYPE} -DFASTGLTF_DOWNLOAD_SIMDJSON=OFF -DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH} -S ${B_SRC} -B ${B_DEST}
    # )
    execute_process(
      COMMAND ${CMAKE_COMMAND} -DCMAKE_BUILD_TYPE=${B_TYPE} -S ${B_SRC} -B ${B_DEST}
    )
  endfunction()

  BuildThirdPartyDep(fastgltf https://github.com/TareHimself/fastgltf ${VERSION} RESULT_DIR "" "BuildFastGLTF")

  set(${RESULT} ${RESULT_DIR})

  list(APPEND CMAKE_PREFIX_PATH ${RESULT_DIR}/lib/cmake/fastgltf)
  list(APPEND CMAKE_PREFIX_PATH ${RESULT_DIR}/lib/cmake/fastgltf/simdjson)
endmacro()
TareHimself commented 10 months ago

If I do not do find_package(fastgltf_simdjson REQUIRED) it fails

TareHimself commented 10 months ago

I am on cmake 3.28.1

TareHimself commented 10 months ago

Just updated the fork so it finds simdjson by itself, not sure if this works for a user_provided simdjson tho

spnda commented 10 months ago

Instead of using find_dependency(fastgltf_simdjson REQUIRED), which brings up the same issue as we originally had, we can just include the fastgltf_simdjsonConfig.cmake from the same install directory, which seems to completely fix the issue for me. Can you verify this on your end? I'll merge it then and fix some style issues with it afterwards.

TareHimself commented 10 months ago

But how would that work if it was not installed with fastgltf_simdjson ? and what was the issue with find_dependency ?

spnda commented 10 months ago

As I said, it's the same error as before. It won't look into the local or the cmake/fastgltf directory for files for the fastgltf_simdjson target.

Perhaps I should just link simdjson.cpp and simdjson.directly into the fastgltf target... That would probably be easier than this fighting with CMake.