Skip to content

Commit

Permalink
AssimpImporter: don't assume Assimp gives us reasonable attenuation.
Browse files Browse the repository at this point in the history
And patch it if it's not what's expected by LightData for directional
and ambient lights. This worked with COLLADA but breaks with Blender
where Assimp encodes the max distance into it for some reason.
  • Loading branch information
mosra committed Feb 4, 2022
1 parent efdc9c5 commit 7936009
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -889,10 +889,19 @@ Containers::Optional<LightData> AssimpImporter::doLight(UnsignedInt id) {
return {};
}

return LightData{lightType, color, 1.0f,
/* For a DIRECTIONAL and AMBIENT light this is (1, 0, 0), which is
exactly what we expect (yay!) */
{l->mAttenuationConstant, l->mAttenuationLinear, l->mAttenuationQuadratic},
/* In case of COLLADA, for a DIRECTIONAL and AMBIENT light this is (1, 0,
0), which is exactly what we expect (yay). In case of Blend files
however, this is (1, 0.16, 0.0064) or some other value, so we have to
patch it to avoid an assert in LightData later. */
/** @todo for blender the values are calculated from max distance, which
could be used to restore the `range` property: https://github.com/assimp/assimp/blob/985a5b9667b25390a00e217ee2086882a101d74a/code/AssetLib/Blender/BlenderLoader.cpp#L1251-L1262 */
Vector3 attenuation{l->mAttenuationConstant, l->mAttenuationLinear, l->mAttenuationQuadratic};
if((lightType == LightData::Type::Directional || lightType == LightData::Type::Ambient) && attenuation != Vector3{1.0f, 0.0f, 0.0f}) {
Warning{} << "Trade::AssimpImporter::light(): patching attenuation" << attenuation << "to" << Vector3{1.0f, 0.0f, 0.0f} << "for" << lightType;
attenuation = {1.0f, 0.0f, 0.0f};
}

return LightData{lightType, color, 1.0f, attenuation,
Rad{l->mAngleInnerCone}, Rad{l->mAngleOuterCone},
l};
}
Expand Down
4 changes: 4 additions & 0 deletions src/MagnumPlugins/AssimpImporter/AssimpImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ verbosity levels in each instance.
- Specular color
- Custom light orientation vectors --- the orientation is always only
inherited from the node containing the light
- @ref LightData::Type::Directional and @ref LightData::Type::Ambient expect
the attenuation to be constant, but Assimp doesn't follow that for certain
file formats (such as Blender). In that case the attenuation value from
Assimp is ignored.
- Area lights are not supported
- For certain file formats (such as COLLADA), if a light isn't referenced by
any node, it gets ignored during import
Expand Down
25 changes: 25 additions & 0 deletions src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ struct AssimpImporterTest: TestSuite::Tester {
void camera();
void cameraOrthographic();
void light();
void lightDirectionalBlender();
void lightUnsupported();
void cameraLightReferencedByTwoNodes();

Expand Down Expand Up @@ -249,6 +250,7 @@ AssimpImporterTest::AssimpImporterTest() {
&AssimpImporterTest::camera,
&AssimpImporterTest::cameraOrthographic,
&AssimpImporterTest::light,
&AssimpImporterTest::lightDirectionalBlender,
&AssimpImporterTest::lightUnsupported,
&AssimpImporterTest::cameraLightReferencedByTwoNodes,

Expand Down Expand Up @@ -1706,6 +1708,29 @@ void AssimpImporterTest::light() {
}), TestSuite::Compare::Container);
}

void AssimpImporterTest::lightDirectionalBlender() {
/* Versions before 5.1.3 say "BLEND: Expected at least one object with no
parent". */
if(_assimpVersion < 513)
CORRADE_SKIP("Blender 2.8+ files are supported only since Assimp 5.1.3.");

Containers::Pointer<AbstractImporter> importer = _manager.instantiate("AssimpImporter");
CORRADE_VERIFY(importer->openFile(Utility::Directory::join(ASSIMPIMPORTER_TEST_DIR, "light-directional.blend")));

/* While COLLADA files (the light() test above) have the attenuation as
expected, Assimp's Blender importer encodes max distance into it, which
has to be patched away */
std::ostringstream out;
Warning redirectWarning{&out};
Containers::Optional<LightData> light = importer->light("Sun");
CORRADE_VERIFY(light);
CORRADE_COMPARE(light->type(), LightData::Type::Directional);
CORRADE_COMPARE(light->color(), (Color3{0.3f, 0.4f, 0.5f}));
CORRADE_COMPARE(light->intensity(), 1.0f);
CORRADE_COMPARE(light->attenuation(), (Vector3{1.0f, 0.0f, 0.0f}));
CORRADE_COMPARE(out.str(), "Trade::AssimpImporter::light(): patching attenuation Vector(1, 0.16, 0.0064) to Vector(1, 0, 0) for Trade::LightData::Type::Directional\n");
}

void AssimpImporterTest::lightUnsupported() {
Containers::Pointer<AbstractImporter> importer = _manager.instantiate("AssimpImporter");

Expand Down
Binary file not shown.

0 comments on commit 7936009

Please sign in to comment.