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

Attentuation for Directional/Sun Lights when importing Blender files #120

Closed
amras0000 opened this issue Feb 4, 2022 · 3 comments
Closed

Comments

@amras0000
Copy link

amras0000 commented Feb 4, 2022

_importer = _manager.loadAndInstantiate("BlenderImporter");
_importer->openFile("lights.blend")
Containers::Optional<Trade::LightData> l = _importer->light(0);

Blender 3.0.0 allows for 4 types of light by default: Point, Sun, Spot, and Area. Here are the results when I run the importer on each of these lights:

Loading point.blend ...
Found a light of type Trade::LightData::Type::Point !
Loading spot.blend ...
Found a light of type Trade::LightData::Type::Spot !
Loading area.blend ...
Trade::AssimpImporter::light(): light type 5 is not supported
No light Found !
Loading sun.blend ...
Trade::LightData: attenuation has to be (1, 0, 0) for an ambient or directional light but got Vector(0, 0, 0)
Aborted (core dumped)

Area lights are not supported, and gracefully pass with a warning and a NullOpt. Directional (ie. sun) lights however, note a different attenuation notation and crash before light() exits - meaning the user has no opportunity to fix the attenuation to the expected value.

Test code and blender files are attached.

magnum-blender-directional-light.zip

@pezcode
Copy link
Contributor

pezcode commented Feb 4, 2022

Looks like this comment was too optimistic:

/* For a DIRECTIONAL and AMBIENT light this is (1, 0, 0), which is

That'll need hardcoded attentuation {1, 0, 0} for ambient and directional lights.

Edit: indeed the assimp docs mention that attenuation is "naturally undefined for directional lights".

@mosra mosra added this to the 2021.0a milestone Feb 4, 2022
@mosra
Copy link
Owner

mosra commented Feb 4, 2022

Oh, fun, new Assimp bugs! :)

We have this tested for COLLADA, where gives us the correct value, and the expectation was that it would have a consistent behavior across various file formats. Such value also makes sense -- the attenuation isn't "undefined" but rather constant, which is what {1, 0, 0} is.

But, in a typical Assimp fashion, it doesn't, and it just does whatever it feels like doing, differently for each file type. Thanks for the repro code, I'll look into this and add a workaround.

@mosra
Copy link
Owner

mosra commented Feb 4, 2022

Should be fixed with 7936009. If not, please complain :)

@mosra mosra closed this as completed Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants