From da5eabc91f2f80374c4ff95bcb10be5eac7df367 Mon Sep 17 00:00:00 2001 From: Philip Rideout Date: Fri, 6 Dec 2019 13:17:03 -0800 Subject: [PATCH] Fix bad state after setIndirectLight(null). This cleans up 3 related issues, but only the first of these caused an actual user-visible bug: (1) When the IBL was null, prepareLighting was failing to bind the black 1x1 texture. (2) If you added and removed an IBL from a Scene, then Scene was not returned to its initial state. (3) The default 1x1 IBL used an intensity of 1 rather than 30000, which is inconsistent with the null IBL. Fixes #1940. --- RELEASE_NOTES.md | 2 ++ filament/src/Engine.cpp | 1 - filament/src/IndirectLight.cpp | 2 +- filament/src/Scene.cpp | 3 +-- filament/src/View.cpp | 35 ++++++++++++++++------------ filament/src/details/IndirectLight.h | 2 +- 6 files changed, 25 insertions(+), 20 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index c94eb4cf8f..50858fe397 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -8,6 +8,8 @@ A new header is inserted each time a *tag* is created. - Improve high roughness material rendering by default when regenerating environments maps +- Fix bad state after removing an IBL from the Scene. + ## v1.4.3 - Fixed an assertion when a parameter array occurs last in a material definition. diff --git a/filament/src/Engine.cpp b/filament/src/Engine.cpp index a71d0b8f33..e3cae5b350 100644 --- a/filament/src/Engine.cpp +++ b/filament/src/Engine.cpp @@ -236,7 +236,6 @@ void FEngine::init() { mDefaultIbl = upcast(IndirectLight::Builder() .reflections(mDefaultIblTexture) .irradiance(3, reinterpret_cast(sh)) - .intensity(1.0f) .build(*this)); // Always initialize the default material, most materials' depth shaders fallback on it. diff --git a/filament/src/IndirectLight.cpp b/filament/src/IndirectLight.cpp index 021f302637..c00152f8c4 100644 --- a/filament/src/IndirectLight.cpp +++ b/filament/src/IndirectLight.cpp @@ -45,7 +45,7 @@ struct IndirectLight::BuilderDetails { Texture const* mIrradianceMap = nullptr; float3 mIrradianceCoefs[9] = {}; mat3f mRotation = {}; - float mIntensity = 30000.0f; + float mIntensity = FIndirectLight::DEFAULT_INTENSITY; }; using BuilderType = IndirectLight; diff --git a/filament/src/Scene.cpp b/filament/src/Scene.cpp index 50c8caa41f..c4a99e2049 100644 --- a/filament/src/Scene.cpp +++ b/filament/src/Scene.cpp @@ -42,8 +42,7 @@ namespace details { // ------------------------------------------------------------------------------------------------ FScene::FScene(FEngine& engine) : - mEngine(engine), - mIndirectLight(engine.getDefaultIndirectLight()) { + mEngine(engine) { } FScene::~FScene() noexcept = default; diff --git a/filament/src/View.cpp b/filament/src/View.cpp index 4fa2904553..33ba2d8820 100644 --- a/filament/src/View.cpp +++ b/filament/src/View.cpp @@ -337,24 +337,29 @@ void FView::prepareLighting(FEngine& engine, FEngine::DriverApi& driver, ArenaSc u.setUniform(offsetof(PerViewUib, exposure), exposure); u.setUniform(offsetof(PerViewUib, ev100), ev100); - // IBL - FIndirectLight const* const ibl = scene->getIndirectLight(); + // If the scene does not have an IBL, use the black 1x1 IBL and honor the fallback intensity + // associated with the skybox. + FIndirectLight const* ibl = scene->getIndirectLight(); + float intensity; if (UTILS_LIKELY(ibl)) { - float2 iblMaxMipLevel{ ibl->getMaxMipLevel(), 1u << ibl->getMaxMipLevel() }; - u.setUniform(offsetof(PerViewUib, iblMaxMipLevel), iblMaxMipLevel); - u.setUniform(offsetof(PerViewUib, iblLuminance), ibl->getIntensity() * exposure); - u.setUniformArray(offsetof(PerViewUib, iblSH), ibl->getSH(), 9); - if (ibl->getReflectionMap()) { - mPerViewSb.setSampler(PerViewSib::IBL_SPECULAR, { - ibl->getReflectionMap(), { - .filterMag = SamplerMagFilter::LINEAR, - .filterMin = SamplerMinFilter::LINEAR_MIPMAP_LINEAR - }}); - } + intensity = ibl->getIntensity(); } else { + ibl = engine.getDefaultIndirectLight(); FSkybox const* const skybox = scene->getSkybox(); - const float intensity = skybox ? skybox->getIntensity() : FIndirectLight::DEFAULT_INTENSITY; - u.setUniform(offsetof(PerViewUib, iblLuminance), intensity * exposure); + intensity = skybox ? skybox->getIntensity() : FIndirectLight::DEFAULT_INTENSITY; + } + + // Set up uniforms and sampler for the IBL, guaranteed to be non-null at this point. + float2 iblMaxMipLevel{ ibl->getMaxMipLevel(), 1u << ibl->getMaxMipLevel() }; + u.setUniform(offsetof(PerViewUib, iblMaxMipLevel), iblMaxMipLevel); + u.setUniform(offsetof(PerViewUib, iblLuminance), intensity * exposure); + u.setUniformArray(offsetof(PerViewUib, iblSH), ibl->getSH(), 9); + if (ibl->getReflectionMap()) { + mPerViewSb.setSampler(PerViewSib::IBL_SPECULAR, { + ibl->getReflectionMap(), { + .filterMag = SamplerMagFilter::LINEAR, + .filterMin = SamplerMinFilter::LINEAR_MIPMAP_LINEAR + }}); } // Directional light (always at index 0) diff --git a/filament/src/details/IndirectLight.h b/filament/src/details/IndirectLight.h index c4321dc8eb..171958b1e8 100644 --- a/filament/src/details/IndirectLight.h +++ b/filament/src/details/IndirectLight.h @@ -35,7 +35,7 @@ class FEngine; class FIndirectLight : public IndirectLight { public: - static constexpr float DEFAULT_INTENSITY = 30000.0f; // lux + static constexpr float DEFAULT_INTENSITY = 30000.0f; // lux of the sun FIndirectLight(FEngine& engine, const Builder& builder) noexcept;