Fixing shader includes

BansheeEditor/Source/BsBuiltinEditorResources.cpp

@@ -29,6 +29,7 @@
 #include "BsImporter.h"
 #include "BsGpuProgram.h"
 #include "BsShader.h"
+#include "BsShaderInclude.h"
 #include "BsTechnique.h"
 #include "BsPass.h"
 #include "BsMaterial.h"
@@ -1208,16 +1209,42 @@ namespace BansheeEngine
 			Vector<Path> files;
 			FileSystem::getChildren(DefaultShaderFolderRaw, files, directories);
 
+			// Process includes first since shaders reference them
 			for (auto& shaderFile : files)
 			{
-				HShader shader = Importer::instance().import<Shader>(shaderFile);
+				if (shaderFile.getExtension() != ".bslinc")
+					continue;
 
-				if (shader != nullptr)
+				HResource resource = Importer::instance().import(shaderFile);
+
+				if (resource != nullptr)
+				{
+					if (rtti_is_of_type<ShaderInclude>(resource.getInternalPtr()))
+					{
+						Path outputLoc = DefaultShaderFolder;
+						outputLoc.append(shaderFile.getWFilename() + L".asset");
+
+						Resources::instance().save(resource, outputLoc, true);
+					}
+				}
+			}
+
+			for (auto& shaderFile : files)
+			{
+				if (shaderFile.getExtension() != ".bsl")
+					continue;
+
+				HResource resource = Importer::instance().import(shaderFile);
+
+				if (resource != nullptr)
 				{
-					Path gpuProgOutputLoc = DefaultShaderFolder;
-					gpuProgOutputLoc.append(shaderFile.getWFilename() + L".asset");
+					if (rtti_is_of_type<Shader>(resource.getInternalPtr()))
+					{
+						Path outputLoc = DefaultShaderFolder;
+						outputLoc.append(shaderFile.getWFilename() + L".asset");
 
-					Resources::instance().save(shader, gpuProgOutputLoc, true);
+						Resources::instance().save(resource, outputLoc, true);
+					}
 				}
 			}
 		}

BansheeEngine/Source/BsBuiltinResources.cpp

@@ -15,6 +15,7 @@
 #include "BsResources.h"
 #include "BsGpuProgram.h"
 #include "BsShader.h"
+#include "BsShaderInclude.h"
 #include "BsTechnique.h"
 #include "BsPass.h"
 #include "BsMaterial.h"
@@ -683,16 +684,42 @@ namespace BansheeEngine
 			Vector<Path> files;
 			FileSystem::getChildren(DefaultShaderFolderRaw, files, directories);
 
+			// Process includes first since shaders reference them
 			for (auto& shaderFile : files)
 			{
-				HShader shader = Importer::instance().import<Shader>(shaderFile);
+				if (shaderFile.getExtension() != ".bslinc")
+					continue;
 
-				if (shader != nullptr)
+				HResource resource = Importer::instance().import(shaderFile);
+
+				if (resource != nullptr)
+				{
+					if (rtti_is_of_type<ShaderInclude>(resource.getInternalPtr()))
+					{
+						Path outputLoc = DefaultShaderFolder;
+						outputLoc.append(shaderFile.getWFilename() + L".asset");
+
+						Resources::instance().save(resource, outputLoc, true);
+					}
+				}
+			}
+
+			for (auto& shaderFile : files)
+			{
+				if (shaderFile.getExtension() != ".bsl")
+					continue;
+
+				HResource resource = Importer::instance().import(shaderFile);
+
+				if (resource != nullptr)
 				{
-					Path gpuProgOutputLoc = DefaultShaderFolder;
-					gpuProgOutputLoc.append(shaderFile.getWFilename() + L".asset");
+					if (rtti_is_of_type<Shader>(resource.getInternalPtr()))
+					{
+						Path outputLoc = DefaultShaderFolder;
+						outputLoc.append(shaderFile.getWFilename() + L".asset");
 
-					Resources::instance().save(shader, gpuProgOutputLoc, true);
+						Resources::instance().save(resource, outputLoc, true);
+					}
 				}
 			}
 		}

BansheeGLRenderSystem/Source/GLSL/src/BsGLSLGpuProgram.cpp

@@ -201,10 +201,6 @@ namespace BansheeEngine
 				mVertexDeclaration = HardwareBufferCoreManager::instance().createVertexDeclaration(elementList);
 			}
 		}
-		else
-		{
-			LOGWRN("Shader compilation/linking failed: " + mCompileError);
-		}
 
 		BS_INC_RENDER_STAT_CAT(ResCreated, RenderStatObject_GpuProgram);
 		GpuProgramCore::initialize();

BansheeSL/Source/BsSLFXCompiler.cpp

@@ -75,9 +75,14 @@ namespace BansheeEngine
 		{
 			LOGERR("Error while parsing shader FX code: " + String(parseState->errorMessage) + ". Location: " +
 				toString(parseState->errorLine) + " (" + toString(parseState->errorColumn) + ")");
+
+			parseStateDelete(parseState);
 		}
 		else
 		{
+			// Only enable for debug purposes
+			//SLFXDebugPrint(parseState->rootNode, "");
+
 			output = parseShader("Shader", parseState, codeBlocks);
 
 			StringStream gpuProgError;
@@ -120,13 +125,8 @@ namespace BansheeEngine
 
 			if (hasError)
 				LOGERR("Failed compiling GPU program(s): " + gpuProgError.str());
-
-			// Only enable for debug purposes
-			//SLFXDebugPrint(parseState->rootNode, "");
 		}
 
-		parseStateDelete(parseState);
-
 		return output;
 	}
 
@@ -489,6 +489,7 @@ namespace BansheeEngine
 		if (techniqueNode == nullptr || techniqueNode->type != NT_Technique)
 			return;
 
+		bool anyMerged = false;
 		for (int i = 0; i < mergeInto->rootNode->options->count; i++)
 		{
 			NodeOption* option = &mergeInto->rootNode->options->entries[i];
@@ -496,10 +497,23 @@ namespace BansheeEngine
 			if (option->type == OT_Technique)
 			{
 				if (isTechniqueMergeValid(option->value.nodePtr, techniqueNode))
+				{
 					mergeTechnique(mergeInto, option->value.nodePtr, techniqueNode, codeBlockOffset);
+					anyMerged = true;
+				}
 				break;
 			}
 		}
+
+		// If this is a language specific technique and it wasn't merged with anything, create a new technique node
+		if (!anyMerged)
+		{
+			ASTFXNode* techniqueNode = nodeCreate(mergeInto->memContext, NT_Technique);
+			NodeOption techniqueOption; techniqueOption.type = OT_Technique; techniqueOption.value.nodePtr = techniqueNode;
+			nodeOptionsAdd(mergeInto->memContext, mergeInto->rootNode->options, &techniqueOption);
+
+			mergeTechnique(mergeInto, techniqueNode, techniqueNode, codeBlockOffset);
+		}
 	}
 
 	void BSLFXCompiler::mergeAST(ParseState* mergeInto, ASTFXNode* mergeFrom, UINT32 codeBlockOffset)
@@ -1455,15 +1469,15 @@ namespace BansheeEngine
 
 	ShaderPtr BSLFXCompiler::parseShader(const String& name, ParseState* parseState, Vector<CodeBlock>& codeBlocks)
 	{
-		ASTFXNode* rootNode = parseState->rootNode;
-		if (rootNode == nullptr || rootNode->type != NT_Shader)
+		if (parseState->rootNode == nullptr || parseState->rootNode->type != NT_Shader)
 			return nullptr;
 
 		Vector<String> includeDependencies;
 
 		// Merge all include ASTs
-		std::function<void(ASTFXNode*, Vector<CodeBlock>&)> parseIncludes = [&](ASTFXNode* node, Vector<CodeBlock>& parentCodeBlocks)
+		std::function<ParseState*(ParseState*, Vector<CodeBlock>&)> parseIncludes = [&](ParseState* parentParseState, Vector<CodeBlock>& parentCodeBlocks)
 		{
+			ASTFXNode* node = parentParseState->rootNode;
 			Vector<tuple<ParseState*, Vector<CodeBlock>>> toMerge;
 
 			for (int i = 0; i < node->options->count; i++)
@@ -1485,22 +1499,22 @@ namespace BansheeEngine
 
 						ParseState* includeParseState = parseStateCreate();
 						Vector<CodeBlock> includeCodeBlocks = parseCodeBlocks(includeSource);
-						parseFX(parseState, includeSource.c_str());
+						parseFX(includeParseState, includeSource.c_str());
 
 						ShaderPtr output;
-						if (parseState->hasError > 0)
+						if (includeParseState->hasError > 0)
 						{
-							LOGERR("Error while parsing shader FX code: " + String(parseState->errorMessage) + ". Location: " +
-								toString(parseState->errorLine) + " (" + toString(parseState->errorColumn) + ")");
+							LOGERR("Error while parsing shader FX code: " + String(includeParseState->errorMessage) + ". Location: " +
+								toString(includeParseState->errorLine) + " (" + toString(includeParseState->errorColumn) + ")");
+
+							parseStateDelete(includeParseState);
 						}
 						else
 						{
-							parseIncludes(includeParseState->rootNode, includeCodeBlocks);
+							ParseState* combinedIncludeParseState = parseIncludes(includeParseState, includeCodeBlocks);
 
-							toMerge.push_back(make_tuple(includeParseState, includeCodeBlocks));
+							toMerge.push_back(make_tuple(combinedIncludeParseState, includeCodeBlocks));
 						}
-
-						parseStateDelete(parseState);
 					}
 					else
 					{
@@ -1520,24 +1534,31 @@ namespace BansheeEngine
 					const Vector<CodeBlock>& curCodeBlocks = get<1>(toMerge[i]);
 					for (auto& codeBlock : curCodeBlocks)
 						outputCodeBlocks.push_back(codeBlock);
+
+					parseStateDelete(get<0>(toMerge[i]));
 				}
 
-				mergeAST(get<0>(toMerge[0]), parseState->rootNode, (UINT32)outputCodeBlocks.size());
+				mergeAST(get<0>(toMerge[0]), node, (UINT32)outputCodeBlocks.size());
 				for (auto& codeBlock : parentCodeBlocks)
 					outputCodeBlocks.push_back(codeBlock);
 
+				parseStateDelete(parentParseState);
+
 				parentCodeBlocks = outputCodeBlocks;
+				return get<0>(toMerge[0]);
 			}
+			else
+				return parentParseState;
 		};
 
-		parseIncludes(parseState->rootNode, codeBlocks);
+		parseState = parseIncludes(parseState, codeBlocks);
 		
 		SHADER_DESC shaderDesc;
 		Vector<TechniquePtr> techniques;
 
-		for (int i = 0; i < rootNode->options->count; i++)
+		for (int i = 0; i < parseState->rootNode->options->count; i++)
 		{
-			NodeOption* option = &rootNode->options->entries[i];
+			NodeOption* option = &parseState->rootNode->options->entries[i];
 
 			switch (option->type)
 			{
@@ -1567,6 +1588,8 @@ namespace BansheeEngine
 			}
 		}
 
+		parseStateDelete(parseState);
+
 		return Shader::_createPtr(name, shaderDesc, techniques);
 	}
 

TODO.txt

@@ -96,21 +96,13 @@ Later:
 ----------------------------------------------------------------------
 Include files:
 
-Include merging:
- - Finish up merging: Missing recursive parsing and loading of includes + code block array merging
- - ShaderManager
-   - Do not use ResourceListener for shader updates as it cannot handle missing includes (i.e. if include gets imported after its shader)
-   - Specify includes by asset path
-     - By default they're searched relative to the working directory
-     - Higher layers can override the path resolver to ShaderManager
-       - e.g. EditorApplication can resolve paths that exist in Project Library (i.e. map from source asset -> actual asset)
-   - ShaderManager needs to be notified when an include is deleted/moved/renamed
-     - Job for project libary. Outside of editor do nothing. i.e. add method ShaderManager::recompileDependencies(include)
-	   - Recompile should just rebuild the entire Shader (not calling GpuProgram::recompile or similar)
-
-Once include works modify existing shaders so they use it.
+Modify all engine/editor shaders so they use includes
+Add support for shader automatic recompilation if includes change
+ - This duty falls on ProjectLibrary. It needs to track shader dependencies, and track
+   include file move/rename/delete/reimport.
 
 Test:
+ - Test include merging and ensure new techniques/passes are created if matching ones cannot be found
  - If errors are reported properly from FX compiler and GPU program compilers
  - Try preprocessing using one RenderAPi and then load the shaders using another and see if they're created normally
 
@@ -119,16 +111,6 @@ Test:
 Add support for registering default parameters (add a set of addParameter methods to SHADER_DESC). Keep sampler states and textures in a dynamic array, and all other data in a dynamic byte array.
  - Make sure initializing the material properly sets up default parameters
 
-----
-
-TODO LATER:
- - Add GpuProgram caching (e.g. save compiled programs somewhere for later use)
- - Consider adding restrictions on parameter ranges (probably via an additional qualifier)
- - Parameter definitions don't support arrays (Leave this for a later iteration)
- - Parameter definitions don't support structs
- - Better error reporting instead of just "syntax error" that better informs the user what the error is
- - Ensure #defines work properly
-
 ----------------------------------------------------------------------
 Scene View