diff --git a/NEW_RELEASE_NOTES.md b/NEW_RELEASE_NOTES.md index 4a1a9c7fa7..0d5e893e7c 100644 --- a/NEW_RELEASE_NOTES.md +++ b/NEW_RELEASE_NOTES.md @@ -7,3 +7,6 @@ for next branch cut* header. appropriate header in [RELEASE_NOTES.md](./RELEASE_NOTES.md). ## Release notes for next branch cut + +- materials: five custom variables (varyings) are now available on the condition that the `color` attribute is not requested (b/404930099). [⚠️ **New Material Version**] + diff --git a/docs/Materials.md.html b/docs/Materials.md.html index 7671c8f97c..20f133e12f 100644 --- a/docs/Materials.md.html +++ b/docs/Materials.md.html @@ -13,7 +13,7 @@ This document is part of the [Filament project](https://github.com/google/filame ## Authors - [Romain Guy](https://github.com/romainguy), [@romainguy](https://twitter.com/romainguy) -- [Mathias Agopian](https://github.com/pixelflinger), [@darthmoosious](https://twitter.com/darthmoosious) +- [Mathias Agopian](https://github.com/pixelflinger), [@pixelflinger](https://bsky.app/profile/pixelflinger.bsky.social) # Overview @@ -1273,6 +1273,9 @@ Description when selecting any shading model that is not `unlit`. See the shader sections of this document for more information on how to access these attributes from the shaders. +!!! Note: Interaction with custom variables + When the `color` attribute is specified, only four custom variables are available instead of five. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ JSON material { parameters : [ @@ -1303,7 +1306,7 @@ Type : array of `string` Value -: Up to 4 strings, each must be a valid GLSL identifier. +: Up to 5 strings, each must be a valid GLSL identifier. Description : Defines custom interpolants (or variables) that are output by the material's vertex shader. @@ -1319,6 +1322,10 @@ Description particular if `default` is specified the default precision is used is the fragment shader (`mediump`) and in the vertex shader (`highp`). +!!! Warning: Interaction with required attributes + If the `color` attribute is specified in the `required` list, then only four variables can be used + instead of five. + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ JSON material { name : Skybox, diff --git a/libs/filamat/include/filamat/MaterialBuilder.h b/libs/filamat/include/filamat/MaterialBuilder.h index 2d868b281e..cea4a338ed 100644 --- a/libs/filamat/include/filamat/MaterialBuilder.h +++ b/libs/filamat/include/filamat/MaterialBuilder.h @@ -217,12 +217,13 @@ public: MaterialBuilder(MaterialBuilder&& rhs) noexcept = default; MaterialBuilder& operator=(MaterialBuilder&& rhs) noexcept = default; - static constexpr size_t MATERIAL_VARIABLES_COUNT = 4; + static constexpr size_t MATERIAL_VARIABLES_COUNT = 5; enum class Variable : uint8_t { CUSTOM0, CUSTOM1, CUSTOM2, - CUSTOM3 + CUSTOM3, + CUSTOM4, // CUSTOM4 is only available if the vertex attribute `color` is not required. // when adding more variables, make sure to update MATERIAL_VARIABLES_COUNT }; diff --git a/libs/filamat/src/MaterialBuilder.cpp b/libs/filamat/src/MaterialBuilder.cpp index 926a094a29..d1e145aa31 100644 --- a/libs/filamat/src/MaterialBuilder.cpp +++ b/libs/filamat/src/MaterialBuilder.cpp @@ -252,6 +252,7 @@ MaterialBuilder& MaterialBuilder::variable(Variable v, const char* name) noexcep case Variable::CUSTOM1: case Variable::CUSTOM2: case Variable::CUSTOM3: + case Variable::CUSTOM4: assert(size_t(v) < MATERIAL_VARIABLES_COUNT); mVariables[size_t(v)] = { CString(name), Precision::DEFAULT, false }; break; @@ -266,6 +267,7 @@ MaterialBuilder& MaterialBuilder::variable(Variable v, case Variable::CUSTOM1: case Variable::CUSTOM2: case Variable::CUSTOM3: + case Variable::CUSTOM4: assert(size_t(v) < MATERIAL_VARIABLES_COUNT); mVariables[size_t(v)] = { CString(name), precision, true }; break; @@ -1254,6 +1256,15 @@ error: OutputTarget::COLOR, Precision::DEFAULT, OutputType::FLOAT4, "color"); } + if (mMaterialDomain == MaterialDomain::SURFACE) { + if (mRequiredAttributes[VertexAttribute::COLOR] && + !mVariables[int(Variable::CUSTOM4)].name.empty()) { + // both the color attribute and the custom4 variable are present, that's not supported + slog.e << "Error: when the 'color' attribute is required 'Variable::CUSTOM4' is not supported." << io::endl; + goto error; + } + } + // TODO: maybe check MaterialDomain::COMPUTE has outputs // Resolve all the #include directives within user code. diff --git a/libs/filamat/tests/test_filamat.cpp b/libs/filamat/tests/test_filamat.cpp index 37baf3e820..d362503032 100644 --- a/libs/filamat/tests/test_filamat.cpp +++ b/libs/filamat/tests/test_filamat.cpp @@ -778,6 +778,48 @@ TEST_F(MaterialCompiler, Uv0AndUv1) { EXPECT_TRUE(result.isValid()); } +TEST_F(MaterialCompiler, FiveCustomVariables) { + filamat::MaterialBuilder builder; + builder.variable(MaterialBuilder::Variable::CUSTOM0, "custom0"); + builder.variable(MaterialBuilder::Variable::CUSTOM1, "custom1"); + builder.variable(MaterialBuilder::Variable::CUSTOM2, "custom2"); + builder.variable(MaterialBuilder::Variable::CUSTOM3, "custom3"); + builder.variable(MaterialBuilder::Variable::CUSTOM4, "custom4"); + filamat::Package result = builder.build(*jobSystem); + EXPECT_TRUE(result.isValid()); +} + +TEST_F(MaterialCompiler, FourCustomVariablesAndColorAttribute) { + filamat::MaterialBuilder builder; + builder.require(filament::VertexAttribute::COLOR); + builder.variable(MaterialBuilder::Variable::CUSTOM0, "custom0"); + builder.variable(MaterialBuilder::Variable::CUSTOM1, "custom1"); + builder.variable(MaterialBuilder::Variable::CUSTOM2, "custom2"); + builder.variable(MaterialBuilder::Variable::CUSTOM3, "custom3"); + filamat::Package result = builder.build(*jobSystem); + EXPECT_TRUE(result.isValid()); +} + +TEST_F(MaterialCompiler, FiveCustomVariablesAndColorAttributeFails) { + filamat::MaterialBuilder builder; + builder.require(filament::VertexAttribute::COLOR); + builder.variable(MaterialBuilder::Variable::CUSTOM0, "custom0"); + builder.variable(MaterialBuilder::Variable::CUSTOM1, "custom1"); + builder.variable(MaterialBuilder::Variable::CUSTOM2, "custom2"); + builder.variable(MaterialBuilder::Variable::CUSTOM3, "custom3"); + builder.variable(MaterialBuilder::Variable::CUSTOM4, "custom4"); + filamat::Package result = builder.build(*jobSystem); + EXPECT_FALSE(result.isValid()); +} + +TEST_F(MaterialCompiler, CustomVariable4AndColorAttributeFails) { + filamat::MaterialBuilder builder; + builder.require(filament::VertexAttribute::COLOR); + builder.variable(MaterialBuilder::Variable::CUSTOM4, "custom4"); + filamat::Package result = builder.build(*jobSystem); + EXPECT_FALSE(result.isValid()); +} + TEST_F(MaterialCompiler, Arrays) { filamat::MaterialBuilder builder; diff --git a/shaders/src/post_process_inputs.vs b/shaders/src/post_process_inputs.vs index eb2af8ab08..a4e6991a1a 100644 --- a/shaders/src/post_process_inputs.vs +++ b/shaders/src/post_process_inputs.vs @@ -20,6 +20,9 @@ struct PostProcessVertexInputs { #ifdef VARIABLE_CUSTOM3 vec4 VARIABLE_CUSTOM3; #endif +#ifdef VARIABLE_CUSTOM4 + vec4 VARIABLE_CUSTOM4; +#endif }; void initPostProcessMaterialVertex(out PostProcessVertexInputs inputs) { @@ -35,4 +38,7 @@ void initPostProcessMaterialVertex(out PostProcessVertexInputs inputs) { #ifdef VARIABLE_CUSTOM3 inputs.VARIABLE_CUSTOM3 = vec4(0.0); #endif +#ifdef VARIABLE_CUSTOM4 + inputs.VARIABLE_CUSTOM4 = vec4(0.0); +#endif } diff --git a/shaders/src/post_process_main.vs b/shaders/src/post_process_main.vs index c9b80e244e..6c3ddb2dea 100644 --- a/shaders/src/post_process_main.vs +++ b/shaders/src/post_process_main.vs @@ -39,6 +39,9 @@ void main() { #if defined(VARIABLE_CUSTOM3) VARIABLE_CUSTOM_AT3 = inputs.VARIABLE_CUSTOM3; #endif +#if defined(VARIABLE_CUSTOM4) + VARIABLE_CUSTOM_AT4 = inputs.VARIABLE_CUSTOM4; +#endif // some PowerVR drivers crash when gl_Position is written more than once gl_Position = inputs.position; diff --git a/shaders/src/surface_main.vs b/shaders/src/surface_main.vs index c8882de005..b957d78a85 100644 --- a/shaders/src/surface_main.vs +++ b/shaders/src/surface_main.vs @@ -151,6 +151,9 @@ void main() { #if defined(VARIABLE_CUSTOM3) VARIABLE_CUSTOM_AT3 = material.VARIABLE_CUSTOM3; #endif +#if defined(VARIABLE_CUSTOM4) && !defined(HAS_ATTRIBUTE_COLOR) + VARIABLE_CUSTOM_AT4 = material.VARIABLE_CUSTOM4; +#endif // The world position can be changed by the user in materialVertex() vertex_worldPosition.xyz = material.worldPosition.xyz; diff --git a/shaders/src/surface_material_inputs.vs b/shaders/src/surface_material_inputs.vs index 2124b7896c..e7a15339a8 100644 --- a/shaders/src/surface_material_inputs.vs +++ b/shaders/src/surface_material_inputs.vs @@ -20,6 +20,9 @@ struct MaterialVertexInputs { #ifdef VARIABLE_CUSTOM3 vec4 VARIABLE_CUSTOM3; #endif +#if defined(VARIABLE_CUSTOM4) && !defined(HAS_ATTRIBUTE_COLOR) + vec4 VARIABLE_CUSTOM4; +#endif #ifdef HAS_ATTRIBUTE_TANGENTS vec3 worldNormal; #endif @@ -74,6 +77,9 @@ void initMaterialVertex(out MaterialVertexInputs material) { #endif #ifdef VARIABLE_CUSTOM3 material.VARIABLE_CUSTOM3 = vec4(0.0); +#endif +#if defined(VARIABLE_CUSTOM4) && !defined(HAS_ATTRIBUTE_COLOR) + material.VARIABLE_CUSTOM4 = vec4(0.0); #endif material.worldPosition = computeWorldPosition(); #ifdef VERTEX_DOMAIN_DEVICE diff --git a/shaders/src/surface_varyings.glsl b/shaders/src/surface_varyings.glsl index 7c1432ef98..c290e87474 100644 --- a/shaders/src/surface_varyings.glsl +++ b/shaders/src/surface_varyings.glsl @@ -2,30 +2,30 @@ // Varyings //------------------------------------------------------------------------------ -LAYOUT_LOCATION(4) VARYING highp vec4 vertex_worldPosition; - -#if defined(HAS_ATTRIBUTE_TANGENTS) -LAYOUT_LOCATION(5) SHADING_INTERPOLATION VARYING mediump vec3 vertex_worldNormal; -#if defined(MATERIAL_NEEDS_TBN) -LAYOUT_LOCATION(6) SHADING_INTERPOLATION VARYING mediump vec4 vertex_worldTangent; -#endif -#endif - -LAYOUT_LOCATION(7) VARYING highp vec4 vertex_position; - -#if defined(FILAMENT_HAS_FEATURE_INSTANCING) -LAYOUT_LOCATION(8) flat VARYING highp int instance_index; -highp int logical_instance_index; -#endif - #if defined(HAS_ATTRIBUTE_COLOR) -LAYOUT_LOCATION(9) VARYING mediump vec4 vertex_color; +LAYOUT_LOCATION(4) VARYING mediump vec4 vertex_color; #endif #if defined(HAS_ATTRIBUTE_UV0) && !defined(HAS_ATTRIBUTE_UV1) -LAYOUT_LOCATION(10) VARYING highp vec2 vertex_uv01; +LAYOUT_LOCATION(5) VARYING highp vec2 vertex_uv01; #elif defined(HAS_ATTRIBUTE_UV1) -LAYOUT_LOCATION(10) VARYING highp vec4 vertex_uv01; +LAYOUT_LOCATION(5) VARYING highp vec4 vertex_uv01; +#endif + +LAYOUT_LOCATION(6) VARYING highp vec4 vertex_worldPosition; + +#if defined(HAS_ATTRIBUTE_TANGENTS) +LAYOUT_LOCATION(7) SHADING_INTERPOLATION VARYING mediump vec3 vertex_worldNormal; +#if defined(MATERIAL_NEEDS_TBN) +LAYOUT_LOCATION(8) SHADING_INTERPOLATION VARYING mediump vec4 vertex_worldTangent; +#endif +#endif + +LAYOUT_LOCATION(9) VARYING highp vec4 vertex_position; + +#if defined(FILAMENT_HAS_FEATURE_INSTANCING) +LAYOUT_LOCATION(10) flat VARYING highp int instance_index; +highp int logical_instance_index; #endif #if defined(VARIANT_HAS_SHADOWING) && defined(VARIANT_HAS_DIRECTIONAL_LIGHTING) diff --git a/tools/matc/src/matc/ParametersProcessor.cpp b/tools/matc/src/matc/ParametersProcessor.cpp index c9b0cfbb10..21f3420b61 100644 --- a/tools/matc/src/matc/ParametersProcessor.cpp +++ b/tools/matc/src/matc/ParametersProcessor.cpp @@ -66,6 +66,7 @@ static MaterialBuilder::Variable intToVariable(size_t i) noexcept { case 1: return MaterialBuilder::Variable::CUSTOM1; case 2: return MaterialBuilder::Variable::CUSTOM2; case 3: return MaterialBuilder::Variable::CUSTOM3; + case 4: return MaterialBuilder::Variable::CUSTOM4; default: return MaterialBuilder::Variable::CUSTOM0; } } @@ -627,8 +628,8 @@ static bool processVariables(MaterialBuilder& builder, const JsonishValue& value const JsonishArray* jsonArray = value.toJsonArray(); const auto& elements = jsonArray->getElements(); - if (elements.size() > 4) { - std::cerr << "variables: Max array size is 4." << std::endl; + if (elements.size() > MaterialBuilder::MATERIAL_VARIABLES_COUNT) { + std::cerr << "variables: Max array size is " << MaterialBuilder::MATERIAL_VARIABLES_COUNT << "." << std::endl; return false; }