Skip to content
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

AssimpImporter: import unrecognized materials + compatibility with 5.1.1 #116

Closed
wants to merge 48 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
cff46e5
AssimpImporter: add light names
pezcode Nov 7, 2021
67c5985
AssimpImporter: import ambient lights as LightData::Type::Ambient
pezcode Nov 13, 2021
060bf34
AssimpImporter: import camera name and aspect ratio
pezcode Nov 13, 2021
3923e07
AssimpImporter: fix try_compile for assimp 5.1.0
pezcode Nov 13, 2021
aadd958
AssimpImporter: streamline version check in tests
pezcode Nov 13, 2021
32314c4
AssimpImporter: import scene name
pezcode Nov 13, 2021
5621134
AssimpImporter: import orthographic cameras
pezcode Nov 13, 2021
977ee6f
AssimpImporter: ignore flags in aiPrimitiveType mask
pezcode Nov 14, 2021
86f1e58
[wip] AssimpImporter: fix tests for 5.1.1
pezcode Nov 15, 2021
e2ee78c
TinyGltfImporter: make test files compatible with AssimpImporter
pezcode Nov 22, 2021
c579a93
AssimpImporter: patch broken FBX animation mTicksPerSecond
pezcode Nov 22, 2021
6564c8c
AssimpImporter: this transformation makes no sense
pezcode Nov 22, 2021
8c9516a
AssimpImporter: clean up comments
pezcode Nov 22, 2021
690e8bc
AssimpImporter: fix possible unsigned underflow
pezcode Nov 22, 2021
47cfea9
AssimpImporter: import raw material data
pezcode Nov 22, 2021
b4cac36
AssimpImporter: extract raw aiString and bool material properties
pezcode Nov 24, 2021
b0faba1
AssimpImporter: document new raw material data config options
pezcode Nov 25, 2021
66cff86
AssimpImporter: add some todos
pezcode Nov 25, 2021
b241ce3
AssimpImporter: don't need this anymore
pezcode Nov 25, 2021
69d0c26
AssimpImporter: test layered materials
pezcode Nov 25, 2021
2c329ae
AssimpImporter: test raw material data
pezcode Nov 25, 2021
7287d08
AssimpImporter: fix raw material test for assimp 5.1.0+
pezcode Nov 25, 2021
feb3527
AssimpImporter: enable the C language
pezcode Nov 25, 2021
41bf8fd
AssimpImporter: fix warnings
pezcode Nov 25, 2021
5942ec1
AssimpImporter: Containers::InPlaceInit is deprecated
pezcode Nov 25, 2021
5408d05
AssimpImporter: fix compiler errors
pezcode Nov 26, 2021
786d8a9
AssimpImporter: use ASCII .fbx material test files
pezcode Nov 26, 2021
18f5d0f
AssimpImporter: simplify material-raw.gltf
pezcode Nov 26, 2021
b7d5545
AssimpImporter: test texture coordinate set for raw material import
pezcode Nov 26, 2021
a5f1a94
AssimpImporter: no need for constexpr here
pezcode Nov 26, 2021
d41e8a0
AssimpImporter: doc++
pezcode Nov 26, 2021
cc0ea71
AssimpImporter: Utility::formatString understands StringView
pezcode Nov 26, 2021
137a9d0
AssimpImporter: clarify comments
pezcode Nov 26, 2021
3e78883
AssimpImporter: don't special-case glTF bugs for raw materials
pezcode Nov 26, 2021
094b50b
Revert "TinyGltfImporter: make test files compatible with AssimpImpor…
pezcode Nov 26, 2021
294fc00
AssimpImporter: use our own camera and skin-without-meshes gltf test …
pezcode Nov 26, 2021
7195b8f
AssimpImporter: detect if assimp was built with double support
pezcode Nov 26, 2021
467b476
AssimpImporter: don't test glTF texture coordinate set attributes for…
pezcode Nov 26, 2021
6111570
AssimpImporter: comments++
pezcode Nov 26, 2021
63a48e7
AssimpImporter: work around possibly undefined aiTextureType
pezcode Nov 26, 2021
c9d42e9
AssimpImporter: don't use possibly undefined material keys
pezcode Nov 26, 2021
670ad45
AssimpImporter: fix the camera name test
pezcode Nov 27, 2021
e41cd93
AssimpImporter: work around
pezcode Nov 27, 2021
7e4b8cc
AssimpImporter: don't assert for unknown material attribute types
pezcode Nov 27, 2021
3c713e8
AssimpImporter: cleanup
pezcode Nov 27, 2021
2d12495
AssimpImporter: skip layered and one raw material test on assimp < 5
pezcode Nov 27, 2021
ba7c80e
AssimpImporter: actually test raw import of new texture types
pezcode Nov 27, 2021
9366ae0
AssimpImporter: fix raw FBX property test on Assimp < 5
pezcode Nov 27, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/MagnumPlugins/AssimpImporter/AssimpImporter.conf
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,17 @@ mergeSkins=false
# AI_CONFIG_* values, can be changed only before the first file is opened
ImportColladaIgnoreUpDirection=false

# By default material attributes that aren't recognized as builtin attributes
# are imported with raw Assimp names and types. Enabling this option completely
# ignores unrecognized attributes instead.
ignoreUnrecognizedMaterialData=false

# Don't attempt to recognize builtin material attributes and always import them
# as raw material data (so e.g. instead of DiffuseColor you'll get
# $clr.diffuse). Implicitly disables ignoreUnrecognizedMaterialData. Mainly for
# testing purposes.
forceRawMaterialData=false

# aiPostProcessSteps, applied to each opened file
[configuration/postprocess]
JoinIdenticalVertices=true
Expand Down
415 changes: 362 additions & 53 deletions src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp

Large diffs are not rendered by default.

24 changes: 21 additions & 3 deletions src/MagnumPlugins/AssimpImporter/AssimpImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,18 @@ verbosity levels in each instance.

- Only materials with shading mode `aiShadingMode_Phong` are supported
- Two-sided property and alpha mode is not imported
- Unrecognized Assimp material attributes are imported with their raw names
and types. You can find the attribute names in [assimp/material.h](https://github.com/assimp/assimp/blob/master/include/assimp/material.h#L944).
Unknown or raw buffer types are imported as
@ref MaterialAttributeType::String. To ignore all unrecognized Assimp
materials instead, enable the @cb{.ini} ignoreUnrecognizedMaterialData @ce
@ref Trade-AssimpImporter-configuration "configuration option".
- To load all material attributes with the raw Assimp names and types, enable
the @cb{.ini} forceRawMaterialData @ce
@ref Trade-AssimpImporter-configuration "configuration option". You will
then get attributes like `$clr.diffuse` instead of
@ref MaterialAttribute::DiffuseColor. @ref MaterialData::types() will
always be empty with this option enabled.
- Assimp seems to ignore ambient textures in COLLADA files
- For some reason, Assimp 4.1 imports STL models with ambient set to
@cpp 0xffffff_srgbf @ce, which causes all other color information to be
Expand All @@ -278,8 +290,6 @@ verbosity levels in each instance.

- @ref LightData::intensity() is always @cpp 1.0f @ce, instead Assimp
premultiplies @ref LightData::color() with the intensity
- Ambient lights are imported as @ref LightData::Type::Point with attenuation
set to @cpp {1.0f, 0.0f, 0.0f} @ce
- The following properties are ignored:
- Specular color
- Custom light orientation vectors --- the orientation is always only
Expand All @@ -288,7 +298,9 @@ verbosity levels in each instance.

@subsection Trade-AssimpImporter-behavior-cameras Camera import

- Aspect and up vector properties are not imported
- Up vector property is not imported
- Orthographic camera support requires Assimp 5.1.0. Earlier versions import
them as perspective cameras with arbitrary field of view and aspect ratio.

@subsection Trade-AssimpImporter-behavior-meshes Mesh import

Expand Down Expand Up @@ -444,9 +456,13 @@ class MAGNUM_ASSIMPIMPORTER_EXPORT AssimpImporter: public AbstractImporter {

MAGNUM_ASSIMPIMPORTER_LOCAL Int doDefaultScene() const override;
MAGNUM_ASSIMPIMPORTER_LOCAL UnsignedInt doSceneCount() const override;
MAGNUM_ASSIMPIMPORTER_LOCAL Int doSceneForName(const std::string& name) override;
MAGNUM_ASSIMPIMPORTER_LOCAL std::string doSceneName(UnsignedInt id) override;
MAGNUM_ASSIMPIMPORTER_LOCAL Containers::Optional<SceneData> doScene(UnsignedInt id) override;

MAGNUM_ASSIMPIMPORTER_LOCAL UnsignedInt doCameraCount() const override;
MAGNUM_ASSIMPIMPORTER_LOCAL Int doCameraForName(const std::string& name) override;
MAGNUM_ASSIMPIMPORTER_LOCAL std::string doCameraName(UnsignedInt id) override;
MAGNUM_ASSIMPIMPORTER_LOCAL Containers::Optional<CameraData> doCamera(UnsignedInt id) override;

MAGNUM_ASSIMPIMPORTER_LOCAL UnsignedInt doObject3DCount() const override;
Expand All @@ -455,6 +471,8 @@ class MAGNUM_ASSIMPIMPORTER_EXPORT AssimpImporter: public AbstractImporter {
MAGNUM_ASSIMPIMPORTER_LOCAL Containers::Pointer<ObjectData3D> doObject3D(UnsignedInt id) override;

MAGNUM_ASSIMPIMPORTER_LOCAL UnsignedInt doLightCount() const override;
MAGNUM_ASSIMPIMPORTER_LOCAL Int doLightForName(const std::string& name) override;
MAGNUM_ASSIMPIMPORTER_LOCAL std::string doLightName(UnsignedInt id) override;
MAGNUM_ASSIMPIMPORTER_LOCAL Containers::Optional<LightData> doLight(UnsignedInt id) override;

MAGNUM_ASSIMPIMPORTER_LOCAL UnsignedInt doMeshCount() const override;
Expand Down
18 changes: 17 additions & 1 deletion src/MagnumPlugins/AssimpImporter/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,22 @@ else()
set(_ASSIMP_TRY_COMPILE_LINK_OR_INCLUDE LINK_LIBRARIES Assimp::Assimp)
endif()

# Newer versions of Assimp add C compile features that end up being propagated
# to installed CMake targets:
# https://github.com/assimp/assimp/commit/799384f2b85b8d3ee9c5b9e18bbe532b4dc7c63c
# This can fail in try_compile because it uses the file extension to determine
# the language and then doesn't look for a C compiler for the required feature.
# CMAKE_PROJECT_[PROJECT]_INCLUDE allows us to include a script after any
# project() calls in try_compile, which then injects a call to
# enable_language(C). CMAKE_TRY_COMPILE is the name of the project in the
# temporary CMakeList.txt created by try_compile:
# https://github.com/Kitware/CMake/blob/v3.4.0/Source/cmCoreTryCompile.cxx#L313
file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/checkAssimpVersionLanguageOverride.cmake "enable_language(C)")
# For some reason, this also needs C to be enabled in this scope
enable_language(C)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sigh. Gotta admit I don't like this part at all :/ I'm wondering if I could expand the above static library workaround to not need to link to the assimp target ever, sidestepping the problem with a C99 feature enabled on it.


# Go through all versions of interest and pick the highest one that compiles
foreach(_version 20201123 20190915 20151031)
foreach(_version 20210102 20201123 20200225 20191122 20190915 20160716 20151031)
try_compile(_works ${CMAKE_CURRENT_BINARY_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/checkAssimpVersion.cpp
${_ASSIMP_TRY_COMPILE_LINK_OR_INCLUDE}
Expand All @@ -71,6 +85,8 @@ foreach(_version 20201123 20190915 20151031)
# CORRADE_CXX_STANDARD property doesn't get passed through. So I
# have to use an internal variable for that instead.
${CORRADE_CXX11_STANDARD_FLAG}
CMAKE_FLAGS
-DCMAKE_PROJECT_CMAKE_TRY_COMPILE_INCLUDE=${CMAKE_CURRENT_BINARY_DIR}/checkAssimpVersionLanguageOverride.cmake
OUTPUT_VARIABLE _version_output)
if(_works)
set(ASSIMP_VERSION ${_version})
Expand Down
Loading