-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
utf8 header not found #5279
utf8 header not found #5279
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i welcome the move from fixing the include-dirs and having a unified #include <utf8.h>
wherever it is needed. (the latter part is missing from this patch).
also, i find it somewhat strange that you are adding includes without mentioning that in the commit (it probably should be a separate commit, adding the utf8.h header wherever it has been forgotten)
@@ -23,6 +23,7 @@ | |||
#include <dxgi1_2.h> | |||
#include <DirectXMath.h> | |||
#include <d3dcompiler.h> | |||
#include <utf8.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you need to include <utf8.h>
here?
@@ -17,6 +17,7 @@ | |||
#include <stdio.h> | |||
#include <GL/gl.h> | |||
#include <GL/glu.h> | |||
#include <utf8.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and why do you need to include <utf8.h>
here?
@@ -923,7 +923,7 @@ IF(ASSIMP_HUNTER_ENABLED) | |||
hunter_add_package(utf8) | |||
find_package(utf8cpp CONFIG REQUIRED) | |||
ELSE() | |||
# utf8 is header-only, so Assimp doesn't need to do anything. | |||
INCLUDE_DIRECTORIES("../contrib/utf8cpp/source") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i agree that it is much better to just fix the include path, rather than including different files. based on context as is done now:
assimp/code/Common/BaseImporter.cpp
Lines 360 to 364 in 020554e
#ifdef ASSIMP_USE_HUNTER | |
#include <utf8.h> | |
#else | |
#include "../contrib/utf8cpp/source/utf8.h" | |
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
obviously this PR does not (yet) change the actual includes (e.g. in BaseImporter.cpp
.
after adding the include_directories, it could be as simple as:
#include <utf8.h>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks better for me doing it this way.
Merged, thanks a lot for your contribution. |
utf8 header not found while compiling assimp with samples.