From d14e29d4d3e783e1cd74daec355cd3b74ef4eeb3 Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Thu, 13 Aug 2020 10:58:27 -0700 Subject: [PATCH] Audit material variants (#2948) --- .../include/private/filament/Variant.h | 30 +++++++++---- libs/filamat/src/shaders/ShaderGenerator.cpp | 1 + libs/matdbg/CMakeLists.txt | 1 + libs/matdbg/src/CommonWriter.cpp | 42 +++++++++++++++++++ libs/matdbg/src/CommonWriter.h | 6 +++ libs/matdbg/src/JsonWriter.cpp | 14 +------ libs/matdbg/src/TextWriter.cpp | 5 ++- shaders/src/inputs.vs | 2 +- shaders/src/main.vs | 2 +- 9 files changed, 79 insertions(+), 24 deletions(-) create mode 100644 libs/matdbg/src/CommonWriter.cpp diff --git a/libs/filabridge/include/private/filament/Variant.h b/libs/filabridge/include/private/filament/Variant.h index 0d09af1a02..008c96f953 100644 --- a/libs/filabridge/include/private/filament/Variant.h +++ b/libs/filabridge/include/private/filament/Variant.h @@ -48,7 +48,7 @@ namespace filament { // Reserved X 0 X 1 0 0 // // Standard variants: - // Vertex shader 0 0 X X 0 X + // Vertex shader 0 0 X X X X // Fragment shader X 0 0 X X X uint8_t key = 0; @@ -63,6 +63,7 @@ namespace filament { static constexpr uint8_t FOG = 0x20; // fog static constexpr uint8_t VERTEX_MASK = DIRECTIONAL_LIGHTING | + DYNAMIC_LIGHTING | SHADOW_RECEIVER | SKINNING_OR_MORPHING | DEPTH; @@ -76,7 +77,8 @@ namespace filament { static constexpr uint8_t DEPTH_MASK = DIRECTIONAL_LIGHTING | DYNAMIC_LIGHTING | SHADOW_RECEIVER | - DEPTH; + DEPTH | + FOG; // the depth variant deactivates all variants that make no sense when writing the depth // only -- essentially, all fragment-only variants. @@ -98,18 +100,30 @@ namespace filament { inline void setFog(bool v) noexcept { set(v, FOG); } inline constexpr bool isDepthPass() const noexcept { - return (key & DEPTH_MASK) == DEPTH_VARIANT; + return isValidDepthVariant(key); + } + + inline static constexpr bool isValidDepthVariant(uint8_t variantKey) noexcept { + // For a variant to be a valid depth variant, all of the bits in DEPTH_MASK must be 0, + // except for DEPTH. + return (variantKey & DEPTH_MASK) == DEPTH_VARIANT; } static constexpr bool isReserved(uint8_t variantKey) noexcept { // reserved variants that should just be skipped - return (variantKey & DEPTH_MASK) > DEPTH || (variantKey & 0b010111u) == 0b000100u; + // 1. If the DEPTH bit is set, then it must be a valid depth variant. Otherwise, the + // variant is reserved. + // 2. If SRE is set, either DYN or DIR must also be set (it makes no sense to have + // shadows without lights). + return + ((variantKey & DEPTH) && !isValidDepthVariant(variantKey)) || + (variantKey & 0b010111u) == 0b000100u; } static constexpr uint8_t filterVariantVertex(uint8_t variantKey) noexcept { - // filter out vertex variants that are not needed. For e.g. dynamic lighting - // doesn't affect the vertex shader. - return variantKey; + // filter out vertex variants that are not needed. For e.g. fog doesn't affect the + // vertex shader. + return variantKey & VERTEX_MASK; } static constexpr uint8_t filterVariantFragment(uint8_t variantKey) noexcept { @@ -120,7 +134,7 @@ namespace filament { static constexpr uint8_t filterVariant(uint8_t variantKey, bool isLit) noexcept { // special case for depth variant - if ((variantKey & DEPTH_MASK) == DEPTH_VARIANT) { + if (isValidDepthVariant(variantKey)) { return variantKey; } // when the shading mode is unlit, remove all the lighting variants diff --git a/libs/filamat/src/shaders/ShaderGenerator.cpp b/libs/filamat/src/shaders/ShaderGenerator.cpp index e45d08ac6e..52e0cd7934 100644 --- a/libs/filamat/src/shaders/ShaderGenerator.cpp +++ b/libs/filamat/src/shaders/ShaderGenerator.cpp @@ -189,6 +189,7 @@ std::string ShaderGenerator::createVertexProgram(filament::backend::ShaderModel bool litVariants = lit || material.hasShadowMultiplier; cg.generateDefine(vs, "HAS_DIRECTIONAL_LIGHTING", litVariants && variant.hasDirectionalLighting()); + cg.generateDefine(vs, "HAS_DYNAMIC_LIGHTING", litVariants && variant.hasDynamicLighting()); cg.generateDefine(vs, "HAS_SHADOWING", litVariants && variant.hasShadowReceiver()); cg.generateDefine(vs, "HAS_SHADOW_MULTIPLIER", material.hasShadowMultiplier); cg.generateDefine(vs, "HAS_SKINNING_OR_MORPHING", variant.hasSkinningOrMorphing()); diff --git a/libs/matdbg/CMakeLists.txt b/libs/matdbg/CMakeLists.txt index e57af9754e..7b8ea355ee 100644 --- a/libs/matdbg/CMakeLists.txt +++ b/libs/matdbg/CMakeLists.txt @@ -23,6 +23,7 @@ set(PUBLIC_HDRS set(SRCS src/CommonWriter.h + src/CommonWriter.cpp src/DebugServer.cpp src/JsonWriter.cpp src/ShaderReplacer.cpp diff --git a/libs/matdbg/src/CommonWriter.cpp b/libs/matdbg/src/CommonWriter.cpp new file mode 100644 index 0000000000..f5f062c944 --- /dev/null +++ b/libs/matdbg/src/CommonWriter.cpp @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "CommonWriter.h" + +#include + +namespace filament::matdbg { + +std::string formatVariantString(uint8_t variant) noexcept { + std::string variantString = ""; + + // NOTE: The 3-character nomenclature used here is consistent with the ASCII art seen in the + // Variant header file and allows the information to fit in a reasonable amount of space on + // the page. The HTML file has a legend. + if (variant) { + if (variant & Variant::DIRECTIONAL_LIGHTING) variantString += "DIR|"; + if (variant & Variant::DYNAMIC_LIGHTING) variantString += "DYN|"; + if (variant & Variant::SHADOW_RECEIVER) variantString += "SRE|"; + if (variant & Variant::SKINNING_OR_MORPHING) variantString += "SKN|"; + if (variant & Variant::DEPTH) variantString += "DEP|"; + if (variant & Variant::FOG) variantString += "FOG|"; + variantString = variantString.substr(0, variantString.length() - 1); + } + + return variantString; +} + +} // namespace filament::matdbg diff --git a/libs/matdbg/src/CommonWriter.h b/libs/matdbg/src/CommonWriter.h index bcc50c0fe3..e8e7f7ddca 100644 --- a/libs/matdbg/src/CommonWriter.h +++ b/libs/matdbg/src/CommonWriter.h @@ -29,6 +29,8 @@ #include #include +#include + namespace filament { namespace matdbg { @@ -214,5 +216,9 @@ const char* toString(backend::SamplerFormat format) noexcept { } } +// Returns a human-readable variant description. +// For example: DYN|DIR +std::string formatVariantString(uint8_t variant) noexcept; + } // namespace matdbg } // namespace filament diff --git a/libs/matdbg/src/JsonWriter.cpp b/libs/matdbg/src/JsonWriter.cpp index 1c46d3c723..77590aa7ce 100644 --- a/libs/matdbg/src/JsonWriter.cpp +++ b/libs/matdbg/src/JsonWriter.cpp @@ -118,20 +118,8 @@ static bool printParametersInfo(ostream& json, const ChunkContainer& container) static void printShaderInfo(ostream& json, const std::vector& info) { for (uint64_t i = 0; i < info.size(); ++i) { const auto& item = info[i]; - string variantString = ""; - - // NOTE: The 3-character nomenclature used here is consistent with the ASCII art seen in the - // Variant header file and allows the information to fit in a reasonable amount of space on - // the page. The HTML file has a legend. - if (item.variant) { - if (item.variant & filament::Variant::DIRECTIONAL_LIGHTING) variantString += "DIR|"; - if (item.variant & filament::Variant::DYNAMIC_LIGHTING) variantString += "DYN|"; - if (item.variant & filament::Variant::SHADOW_RECEIVER) variantString += "SRE|"; - if (item.variant & filament::Variant::SKINNING_OR_MORPHING) variantString += "SKN|"; - if (item.variant & filament::Variant::DEPTH) variantString += "DEP|"; - variantString = variantString.substr(0, variantString.length() - 1); - } + string variantString = formatVariantString(item.variant); string ps = (item.pipelineStage == backend::ShaderType::VERTEX) ? "vertex " : "fragment"; json << " {" diff --git a/libs/matdbg/src/TextWriter.cpp b/libs/matdbg/src/TextWriter.cpp index f1dfca7618..b3b2f4ce37 100644 --- a/libs/matdbg/src/TextWriter.cpp +++ b/libs/matdbg/src/TextWriter.cpp @@ -290,7 +290,10 @@ static void printShaderInfo(ostream& text, const vector& info) { text << " "; text << "0x" << hex << setfill('0') << setw(2) << right << (int) item.variant; - text << setfill(' ') << dec << endl; + text << setfill(' ') << dec; + text << " "; + text << formatVariantString(item.variant); + text << endl; } text << endl; } diff --git a/shaders/src/inputs.vs b/shaders/src/inputs.vs index 35304e555d..c48935d8ce 100644 --- a/shaders/src/inputs.vs +++ b/shaders/src/inputs.vs @@ -80,6 +80,6 @@ LAYOUT_LOCATION(10) out highp vec4 vertex_uv01; LAYOUT_LOCATION(11) out highp vec4 vertex_lightSpacePosition; #endif -#if defined(HAS_SHADOWING) +#if defined(HAS_SHADOWING) && defined(HAS_DYNAMIC_LIGHTING) LAYOUT_LOCATION(12) out highp vec4 vertex_spotLightSpacePosition[MAX_SHADOW_CASTING_SPOTS]; #endif diff --git a/shaders/src/main.vs b/shaders/src/main.vs index 13f234c4e3..ccfea0bc08 100644 --- a/shaders/src/main.vs +++ b/shaders/src/main.vs @@ -92,7 +92,7 @@ void main() { frameUniforms.lightDirection, frameUniforms.shadowBias.y, getLightFromWorldMatrix()); #endif -#if defined(HAS_SHADOWING) +#if defined(HAS_SHADOWING) && defined(HAS_DYNAMIC_LIGHTING) for (uint l = 0u; l < uint(MAX_SHADOW_CASTING_SPOTS); l++) { vec3 dir = shadowUniforms.directionShadowBias[l].xyz; float bias = shadowUniforms.directionShadowBias[l].w;