search

JoeyDeVries / LearnOpenGL

Code repository of all OpenGL chapters from the book and its accompanying website https://learnopengl.com
https://learnopengl.com
Other
11.1k stars 2.81k forks source link

fix #333 model load error #374

Open simpzan opened 1 year ago

simpzan commented 1 year ago

use assimp headers in the system instead of the ones in the repo, because the ones in the repo may not compatible with the assimp shared library in the system.

JoeyDeVries commented 1 year ago

I'm a bit hesitant on accepting this one, as I'm not sure what it'll do on Linux/Windows builds. Can you be certain this'll work multi-platform, or do we need a bit of extra testing?

simpzan commented 1 year ago

I just tested this patch on ubuntu 20.04, it can't fix the error. I need to remove includes/assimp/ to fix the issue. I don't have windows machine to test how to fix the issue on windows.

simpzan commented 1 year ago

@JoeyDeVries any reason to include assimp header in the repo? can I remove them?

JoeyDeVries commented 1 year ago

If I remember why it's set up that way is that, compared to linux/macos, Windows doesn't have an 'install to system library' version of Assimp so it involves downloading either pre-built lib/dlls (with include files), or built yourself fully from code alone. This isn't always great for new readers starting out. In addition to that there's also chance of code breaking because the latest Assimp version updated to different header definitions or code; the code is quite old (5+ years).

I did try to build on a Windows machine just now with your changes and it indeed errored at CMake cause it couldn't find any Assimp lib/include on Windows without the folder:

CMake Error at cmake/modules/FindASSIMP.cmake:30 (MESSAGE):
  Could not find libASSIMP
Call Stack (most recent call first):
  CMakeLists.txt:27 (find_package)

Unless you find a way to fix this problem I can't merge it unfortunately.