From 280796e1f4c916934c62dbdd0bb65d8717da1e2f Mon Sep 17 00:00:00 2001
From: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
Date: Fri, 10 Sep 2021 11:59:20 -0500
Subject: [PATCH] Fix GHI 4041 Terrain Shader Crash (#4050)

The terrain feature processor was crashing due to an invalid shader name.  The shader name has been fixed, but the feature processor has also been hardened so that it no longer crashes if a shader fails to load.
Also, while testing, this inadvertently exposed a second crash in EntitySerializer that occurs when components don't serialize in correctly.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
---
 .../AzCore/AzCore/Component/EntitySerializer.cpp   |  5 +++--
 .../TerrainRenderer/TerrainFeatureProcessor.cpp    | 14 +++++++++++---
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/Code/Framework/AzCore/AzCore/Component/EntitySerializer.cpp b/Code/Framework/AzCore/AzCore/Component/EntitySerializer.cpp
index 803daf1373..1dd685f763 100644
--- a/Code/Framework/AzCore/AzCore/Component/EntitySerializer.cpp
+++ b/Code/Framework/AzCore/AzCore/Component/EntitySerializer.cpp
@@ -84,8 +84,9 @@ namespace AZ
             static TypeId genericComponentWrapperTypeId("{68D358CA-89B9-4730-8BA6-E181DEA28FDE}");
             for (auto& [componentKey, component] : componentMap)
             {
-                // if underlying type is genericComponentWrapperTypeId, the template is null and the component should not be addded
-                if (component->GetUnderlyingComponentType() != genericComponentWrapperTypeId)
+                // if the component didn't serialize (i.e. is null) or the underlying type is genericComponentWrapperTypeId, the
+                // template is null and the component should not be addded
+                if (component && (component->GetUnderlyingComponentType() != genericComponentWrapperTypeId))
                 {
                     entityInstance->m_components.emplace_back(component);
                 }
diff --git a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp
index f0743cfb52..511b206b84 100644
--- a/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp
+++ b/Gems/Terrain/Code/Source/TerrainRenderer/TerrainFeatureProcessor.cpp
@@ -66,7 +66,13 @@ namespace Terrain
     }
 
     void TerrainFeatureProcessor::ConfigurePipelineState(ShaderState& shaderState, bool assertOnFail)
-    {   
+    {
+        if (shaderState.m_shader == nullptr)
+        {
+            AZ_Assert(shaderState.m_shader || !assertOnFail, "Terrain shader failed to load correctly.");
+            return;
+        }
+
         bool success = GetParentScene()->ConfigurePipelineState(shaderState.m_shader->GetDrawListTag(), shaderState.m_pipelineStateDescriptor);
         AZ_Assert(success || !assertOnFail, "Couldn't configure the pipeline state.");
         if (success)
@@ -110,7 +116,7 @@ namespace Terrain
             };
             
             LoadShader("Shaders/Terrain/Terrain.azshader", m_shaderStates[ShaderType::Forward]);
-            LoadShader("Shaders/Terrain/Terrain_DepthPass_WithPS.azshader", m_shaderStates[ShaderType::Depth]);
+            LoadShader("Shaders/Terrain/Terrain_DepthPass.azshader", m_shaderStates[ShaderType::Depth]);
 
             // Forward and depth shader use same srg layout.
             AZ::RHI::Ptr<AZ::RHI::ShaderResourceGroupLayout> perObjectSrgLayout =
@@ -248,7 +254,9 @@ namespace Terrain
     {
         AZ_PROFILE_FUNCTION(AzRender);
 
-        if (m_shaderStates[ShaderType::Forward].m_shader->GetDrawListTag().IsNull() ||
+        if ((m_shaderStates[ShaderType::Forward].m_shader == nullptr) ||
+            (m_shaderStates[ShaderType::Depth].m_shader == nullptr) ||
+            m_shaderStates[ShaderType::Forward].m_shader->GetDrawListTag().IsNull() ||
             m_shaderStates[ShaderType::Depth].m_shader->GetDrawListTag().IsNull())
         {
             return;