#6052: ShaderTemplate is re-evaluating macro usage on certain changes

codereader · Aug 5, 2022 · 42cb7ae · 42cb7ae
1 parent d726870
commit 42cb7ae
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 5 deletions.
diff --git a/radiantcore/shaders/ShaderTemplate.cpp b/radiantcore/shaders/ShaderTemplate.cpp
@@ -1499,6 +1499,27 @@ std::string ShaderTemplate::getRenderBumpFlatArguments()
     return _renderBumpFlatArguments;
 }
 
+bool ShaderTemplate::evaluateMacroUsage()
+{
+    ensureParsed();
+
+    auto oldDecalMacroFlag = _parseFlags & Material::PF_HasDecalMacro;
+
+    // Reset the decal_macro flag and evaluate
+    _parseFlags &= ~Material::PF_HasDecalMacro;
+
+    if (getPolygonOffset() == 1.0f &&
+        getSortRequest() == Material::SORT_DECAL &&
+        (getSurfaceFlags() & Material::SURF_DISCRETE) != 0 &&
+        (getMaterialFlags() & Material::FLAG_NOSHADOWS) != 0)
+    {
+        _parseFlags |= Material::PF_HasDecalMacro;
+    }
+
+    // If the flag changed => return true
+    return oldDecalMacroFlag != (_parseFlags & Material::PF_HasDecalMacro);
+}
+
 std::string ShaderTemplate::generateSyntax()
 {
     return MaterialSourceGenerator::GenerateDefinitionBlock(*this);

diff --git a/radiantcore/shaders/ShaderTemplate.h b/radiantcore/shaders/ShaderTemplate.h
@@ -189,13 +189,15 @@ class ShaderTemplate final :
     {
         ensureParsed();
         _materialFlags |= flag;
+        evaluateMacroUsage(); // material flags influence macro usage
         onTemplateChanged();
     }
 
     void clearMaterialFlag(Material::Flags flag)
     {
         ensureParsed();
         _materialFlags &= ~flag;
+        evaluateMacroUsage(); // material flags influence macro usage
         onTemplateChanged();
     }
 
@@ -237,15 +239,15 @@ class ShaderTemplate final :
     {
         ensureParsed();
         _surfaceFlags |= flag;
-
+        evaluateMacroUsage(); // surface flags influence macro usage
         onTemplateChanged();
     }
 
     void clearSurfaceFlag(Material::SurfaceFlags flag)
     {
         ensureParsed();
         _surfaceFlags &= ~flag;
-
+        evaluateMacroUsage(); // surface flags influence macro usage
         onTemplateChanged();
     }
 
@@ -384,6 +386,7 @@ class ShaderTemplate final :
 
         _materialFlags |= Material::FLAG_HAS_SORT_DEFINED;
         _sortReq = sortRequest;
+        evaluateMacroUsage(); // sort request influences macro usage
 
         onTemplateChanged();
     }
@@ -404,6 +407,8 @@ class ShaderTemplate final :
             _sortReq = Material::SORT_OPAQUE;
         }
 
+        evaluateMacroUsage(); // sort request influences macro usage
+
         onTemplateChanged();
     }
 
@@ -419,6 +424,8 @@ class ShaderTemplate final :
         setMaterialFlag(Material::FLAG_POLYGONOFFSET);
         _polygonOffset = offset;
 
+        evaluateMacroUsage(); // polygon offset influences macro usage
+
         onTemplateChanged();
     }
 
@@ -536,8 +543,7 @@ class ShaderTemplate final :
     std::string generateSyntax() override;
 
 private:
-
-	// Add the given layer and assigns editor preview layer if applicable
+    // Add the given layer and assigns editor preview layer if applicable
 	void addLayer(const Doom3ShaderLayer::Ptr& layer);
 
     // Parse helpers. These scan for possible matches, this is not a
@@ -557,6 +563,9 @@ class ShaderTemplate final :
 
 	bool saveLayer();
     void determineCoverage();
+    // Checks if the settings of this material justify the use of any macro keywords like DECAL_MACRO
+    // Returns true if a macro flag changed, in which case a changed signal should be emitted
+    bool evaluateMacroUsage();
 };
 
 }
diff --git a/test/MaterialExport.cpp b/test/MaterialExport.cpp
@@ -342,21 +342,27 @@ TEST_F(MaterialExportTest, DecalMacroUsage)
     auto material = GlobalMaterialManager().getMaterial("textures/exporttest/empty");
     EXPECT_EQ(string::trim_copy(material->getDefinition()), "");
 
+    // Set the 4 decal macro properties one after the other, the last one should cut it
     // polygonOffset 1 | discrete | sort decal | noShadows
 
-    // Set the 4 decal macro properties one after the other, the last one should cut it
+    // Clear the noshadows flag, an empty material is translucent and implicitly set to noshadows
+    material->clearMaterialFlag(Material::FLAG_NOSHADOWS);
 
     material->setPolygonOffset(1.0f);
     expectDefinitionDoesNotContain(material, "DECAL_MACRO");
+    EXPECT_FALSE(material->getParseFlags() & Material::PF_HasDecalMacro);
 
     material->setSurfaceFlag(Material::SURF_DISCRETE);
     expectDefinitionDoesNotContain(material, "DECAL_MACRO");
+    EXPECT_FALSE(material->getParseFlags() & Material::PF_HasDecalMacro);
 
     material->setSortRequest(Material::SORT_DECAL);
     expectDefinitionDoesNotContain(material, "DECAL_MACRO");
+    EXPECT_FALSE(material->getParseFlags() & Material::PF_HasDecalMacro);
 
     material->setMaterialFlag(Material::FLAG_NOSHADOWS);
     expectDefinitionContains(material, "DECAL_MACRO");
+    EXPECT_TRUE(material->getParseFlags() & Material::PF_HasDecalMacro);
 
     // Setting decalInfo doesn't influence DECAL_MACRO
     Material::DecalInfo info;
@@ -367,10 +373,12 @@ TEST_F(MaterialExportTest, DecalMacroUsage)
     material->setDecalInfo(info);
 
     expectDefinitionContains(material, "DECAL_MACRO");
+    EXPECT_TRUE(material->getParseFlags() & Material::PF_HasDecalMacro);
 
     // Set the polygonOffset to a mismatching value, this breaks the spell
     material->setPolygonOffset(1.1f);
     expectDefinitionDoesNotContain(material, "DECAL_MACRO");
+    EXPECT_FALSE(material->getParseFlags() & Material::PF_HasDecalMacro);
 }
 
 TEST_F(MaterialExportTest, RenderBump)