From 96212be2014c5013712e07eed5f2404e7dbaa4ca Mon Sep 17 00:00:00 2001
From: Chinmay Garde <chinmaygarde@google.com>
Date: Wed, 7 Aug 2024 12:38:38 -0700
Subject: [PATCH] [Impeller] Make stage compatibility checker work with stages
 that have no inputs or outputs.

Also removes the workarounds and updates the docstring to talk about the PowerVR hack.

Fixes https://github.com/flutter/flutter/issues/146852
---
 impeller/compiler/code_gen_template.h         |  5 --
 .../shader_stage_compatibility_checker.h      | 46 +++++++++++--------
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h
index 785043493a4bb..6c6b90e763fc2 100644
--- a/impeller/compiler/code_gen_template.h
+++ b/impeller/compiler/code_gen_template.h
@@ -75,9 +75,7 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader {
   // ===========================================================================
   // Stage Inputs ==============================================================
   // ===========================================================================
-{% if length(stage_inputs) > 0 %}
 {% for stage_input in stage_inputs %}
-
   static constexpr auto kInput{{camel_case(stage_input.name)}} = ShaderStageIOSlot { // {{stage_input.name}}
     "{{stage_input.name}}",             // name
     {{stage_input.location}}u,          // attribute location
@@ -91,7 +89,6 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader {
     {{stage_input.relaxed_precision}},  // relaxed precision
   };
 {% endfor %}
-{% endif %}
 
   static constexpr std::array<const ShaderStageIOSlot*, {{length(stage_inputs)}}> kAllShaderStageInputs = {
 {% for stage_input in stage_inputs %}
@@ -131,7 +128,6 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader {
   // ===========================================================================
   // Stage Outputs =============================================================
   // ===========================================================================
-{% if length(stage_outputs) > 0 %}
 {% for stage_output in stage_outputs %}
   static constexpr auto kOutput{{camel_case(stage_output.name)}} = ShaderStageIOSlot { // {{stage_output.name}}
     "{{stage_output.name}}",             // name
@@ -151,7 +147,6 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader {
     &kOutput{{camel_case(stage_output.name)}}, // {{stage_output.name}}
 {% endfor %}
   };
-{% endif %}
 
   // ===========================================================================
   // Resource Binding Utilities ================================================
diff --git a/impeller/renderer/shader_stage_compatibility_checker.h b/impeller/renderer/shader_stage_compatibility_checker.h
index f8fc35336fc05..06871473d7c44 100644
--- a/impeller/renderer/shader_stage_compatibility_checker.h
+++ b/impeller/renderer/shader_stage_compatibility_checker.h
@@ -10,10 +10,31 @@
 #include "impeller/core/shader_types.h"
 
 namespace impeller {
-/// This is a classed use to check that the input slots of fragment shaders
-/// match the output slots of the vertex shaders.
-/// If they don't match it will result in linker errors when creating the
-/// pipeline.  It's not used at runtime.
+
+//------------------------------------------------------------------------------
+/// @brief      Checks, at C++ compile-time, if the two pipeline stages are
+///             compatible.
+///
+///             Stages may be incompatible if the outputs declared in the vertex
+///             stage don't line up with the inputs declared in the fragment
+///             stage. Additionally, the types of the inputs and outputs need to
+///             be identical. Some drivers like the one on the PowerVR GE8320
+///             also have bugs the require the precision qualifier of the stage
+///             interfaces to match exactly.
+///
+///             Not ensuring stage compatibility will cause pipeline creation
+///             errors that will only be caught at runtime. In addition to the
+///             bugs discovered related to precision qualifier, some errors may
+///             only manifest at runtime on some devices.
+///
+///             This static compile-time C++ check ensures that all the possible
+///             runtime errors will be caught at build time.
+///
+///             There is no runtime overhead to using this class.
+///
+/// @tparam     VertexShaderT    The vertex shader stage metadata.
+/// @tparam     FragmentShaderT  The fragment shader stage metadata.
+///
 template <typename VertexShaderT, typename FragmentShaderT>
 class ShaderStageCompatibilityChecker {
  public:
@@ -59,21 +80,6 @@ class ShaderStageCompatibilityChecker {
   }
 };
 
-// The following shaders don't define output slots.
-// TODO(https://github.com/flutter/flutter/issues/146852): Make impellerc emit
-// an empty array for output slots.
-struct ClipVertexShader;
-struct SolidFillVertexShader;
-
-template <typename FragmentShaderT>
-class ShaderStageCompatibilityChecker<ClipVertexShader, FragmentShaderT> {
- public:
-  static constexpr bool Check() { return true; }
-};
-template <typename FragmentShaderT>
-class ShaderStageCompatibilityChecker<SolidFillVertexShader, FragmentShaderT> {
- public:
-  static constexpr bool Check() { return true; }
-};
 }  // namespace impeller
+
 #endif  // FLUTTER_IMPELLER_RENDERER_SHADER_STAGE_COMPATIBILITY_CHECKER_H_