[Impeller] fix remaining Validation errors. (flutter#51692)

Jonah Williams · web-flow · commit dad4edbf4ce6 · 2024-03-27T17:37:51.000Z
Fixes flutter#145039 Some of the errors were due to unconsumed fragment shader inputs. Enabling VK_KHR_maintenance4 relaxes this restriction.
diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc
@@ -2200,7 +2200,7 @@ TEST_P(EntityTest, RuntimeEffect) {
   ASSERT_TRUE(runtime_stage->IsDirty());
 
   bool expect_dirty = true;
-  Pipeline<PipelineDescriptor>* first_pipeline = nullptr;
+  Pipeline<PipelineDescriptor>* first_pipeline;
   auto callback = [&](ContentContext& context, RenderPass& pass) -> bool {
     EXPECT_EQ(runtime_stage->IsDirty(), expect_dirty);
 
@@ -2238,7 +2238,10 @@ TEST_P(EntityTest, RuntimeEffect) {
   // Simulate some renders and hot reloading of the shader.
   auto content_context = GetContentContext();
   {
-    RenderTarget target;
+    RenderTarget target =
+        content_context->GetRenderTargetCache()->CreateOffscreen(
+            *content_context->GetContext(), {1, 1}, 1u);
+
     testing::MockRenderPass mock_pass(GetContext(), target);
     callback(*content_context, mock_pass);
     callback(*content_context, mock_pass);
@@ -2252,10 +2255,7 @@ TEST_P(EntityTest, RuntimeEffect) {
     expect_dirty = true;
 
     callback(*content_context, mock_pass);
-    callback(*content_context, mock_pass);
   }
-
-  ASSERT_TRUE(OpenPlaygroundHere(callback));
 }
 
 TEST_P(EntityTest, RuntimeEffectCanSuccessfullyRender) {
@@ -2319,7 +2319,6 @@ TEST_P(EntityTest, RuntimeEffectSetsRightSizeWhenUniformIsStruct) {
 
   auto contents = std::make_shared<RuntimeEffectContents>();
   contents->SetGeometry(Geometry::MakeCover());
-
   contents->SetRuntimeStage(runtime_stage);
 
   struct FragUniforms {
@@ -2338,7 +2337,9 @@ TEST_P(EntityTest, RuntimeEffectSetsRightSizeWhenUniformIsStruct) {
   entity.SetContents(contents);
 
   auto context = GetContentContext();
-  RenderTarget target;
+  RenderTarget target = context->GetRenderTargetCache()->CreateOffscreen(
+      *context->GetContext(), {1, 1}, 1u);
+
   testing::MockRenderPass pass(GetContext(), target);
   ASSERT_TRUE(contents->Render(*context, entity, pass));
   ASSERT_EQ(pass.GetCommands().size(), 1u);
diff --git a/impeller/fixtures/runtime_stage_example.frag b/impeller/fixtures/runtime_stage_example.frag
@@ -2,13 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-layout(location = 0) uniform vec2 iResolution;
-layout(location = 1) uniform float iTime;
+#include <flutter/runtime_effect.glsl>
 
-layout(location = 0) out vec4 fragColor;
+uniform vec2 iResolution;
+uniform float iTime;
+
+out vec4 fragColor;
 
 void main() {
-  vec2 uv = gl_FragCoord.xy / iResolution;
+  vec2 uv = FlutterFragCoord() / iResolution;
   float t = 4 * iTime;
   vec3 col = 0.5 + 0.5 * cos(t + uv.xyx + vec3(0, 1, 4));
   fragColor = vec4(col, 1.0);
diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc
@@ -104,12 +104,6 @@ bool SaveScreenshot(std::unique_ptr<testing::Screenshot> screenshot) {
   return true;
 }
 
-bool ShouldTestHaveVulkanValidations() {
-  std::string test_name = GetTestName();
-  return std::find(kVulkanDenyValidationTests.begin(),
-                   kVulkanDenyValidationTests.end(),
-                   test_name) == kVulkanDenyValidationTests.end();
-}
 }  // namespace
 
 struct GoldenPlaygroundTest::GoldenPlaygroundTestImpl {
@@ -144,14 +138,13 @@ void GoldenPlaygroundTest::SetUp() {
   std::filesystem::path icd_path = target_path / "vk_swiftshader_icd.json";
   setenv("VK_ICD_FILENAMES", icd_path.c_str(), 1);
 
-  bool enable_vulkan_validations = ShouldTestHaveVulkanValidations();
   switch (GetParam()) {
     case PlaygroundBackend::kMetal:
       pimpl_->screenshotter = std::make_unique<testing::MetalScreenshotter>();
       break;
     case PlaygroundBackend::kVulkan: {
       const std::unique_ptr<PlaygroundImpl>& playground =
-          GetSharedVulkanPlayground(enable_vulkan_validations);
+          GetSharedVulkanPlayground(/*enable_validations=*/true);
       pimpl_->screenshotter =
           std::make_unique<testing::VulkanScreenshotter>(playground);
       break;
@@ -171,7 +164,7 @@ void GoldenPlaygroundTest::SetUp() {
     pimpl_->screenshotter = std::make_unique<testing::MetalScreenshotter>();
   } else if (GetParam() == PlaygroundBackend::kVulkan) {
     const std::unique_ptr<PlaygroundImpl>& playground =
-        GetSharedVulkanPlayground(enable_vulkan_validations);
+        GetSharedVulkanPlayground(/*enable_validations=*/true);
     pimpl_->screenshotter =
         std::make_unique<testing::VulkanScreenshotter>(playground);
   }
@@ -257,7 +250,7 @@ std::shared_ptr<Context> GoldenPlaygroundTest::MakeContext() const {
     /// On Metal we create a context for each test.
     return GetContext();
   } else if (GetParam() == PlaygroundBackend::kVulkan) {
-    bool enable_vulkan_validations = ShouldTestHaveVulkanValidations();
+    bool enable_vulkan_validations = true;
     FML_CHECK(!pimpl_->test_vulkan_playground)
         << "We don't support creating multiple contexts for one test";
     pimpl_->test_vulkan_playground =
diff --git a/impeller/playground/playground.h b/impeller/playground/playground.h
@@ -32,22 +32,6 @@ enum class PlaygroundBackend {
   kVulkan,
 };
 
-// TODO(https://github.com/flutter/flutter/issues/145039)
-// clang-format off
-static const std::vector<std::string> kVulkanDenyValidationTests = {
-  "impeller_Play_SceneTest_FlutterLogo_Vulkan",
-  "impeller_Play_SceneTest_CuboidUnlit_Vulkan",
-  "impeller_Play_RuntimeStageTest_CanCreatePipelineFromRuntimeStage_Vulkan",
-  "impeller_Play_RuntimeEffectSetsRightSizeWhenUniformIsStruct_Vulkan",
-  "impeller_Play_RuntimeEffectCanSuccessfullyRender_Vulkan",
-  "impeller_Play_RuntimeEffect_Vulkan",
-  "impeller_Play_EntityTest_RuntimeStageTest_CanCreatePipelineFromRuntimeStage_Vulkan",
-  "impeller_Play_EntityTest_RuntimeEffectSetsRightSizeWhenUniformIsStruct_Vulkan",
-  "impeller_Play_EntityTest_RuntimeEffectCanSuccessfullyRender_Vulkan",
-  "impeller_Play_EntityTest_RuntimeEffect_Vulkan",
-};
-// clang-format on
-
 constexpr inline RuntimeStageBackend PlaygroundBackendToRuntimeStageBackend(
     PlaygroundBackend backend) {
   switch (backend) {
diff --git a/impeller/playground/playground_impl.cc b/impeller/playground/playground_impl.cc
@@ -20,29 +20,6 @@
 #include "impeller/playground/backend/vulkan/playground_impl_vk.h"
 #endif  // IMPELLER_ENABLE_VULKAN
 
-namespace {
-
-std::string GetTestName() {
-  std::string suite_name =
-      ::testing::UnitTest::GetInstance()->current_test_suite()->name();
-  std::string test_name =
-      ::testing::UnitTest::GetInstance()->current_test_info()->name();
-  std::stringstream ss;
-  ss << "impeller_" << suite_name << "_" << test_name;
-  std::string result = ss.str();
-  // Make sure there are no slashes in the test name.
-  std::replace(result.begin(), result.end(), '/', '_');
-  return result;
-}
-
-bool ShouldTestHaveVulkanValidations() {
-  std::string test_name = GetTestName();
-  return std::find(impeller::kVulkanDenyValidationTests.begin(),
-                   impeller::kVulkanDenyValidationTests.end(),
-                   test_name) == impeller::kVulkanDenyValidationTests.end();
-}
-}  // namespace
-
 namespace impeller {
 
 std::unique_ptr<PlaygroundImpl> PlaygroundImpl::Create(
@@ -64,7 +41,7 @@ std::unique_ptr<PlaygroundImpl> PlaygroundImpl::Create(
                             "isn't available or was disabled on this platform: "
                          << PlaygroundBackendToString(backend);
       }
-      switches.enable_vulkan_validation = ShouldTestHaveVulkanValidations();
+      switches.enable_vulkan_validation = true;
       return std::make_unique<PlaygroundImplVK>(switches);
 #endif  // IMPELLER_ENABLE_VULKAN
     default:
diff --git a/impeller/runtime_stage/runtime_stage_unittests.cc b/impeller/runtime_stage/runtime_stage_unittests.cc
@@ -277,7 +277,16 @@ TEST_P(RuntimeStageTest, CanCreatePipelineFromRuntimeStage) {
   auto vertex_descriptor = std::make_shared<VertexDescriptor>();
   vertex_descriptor->SetStageInputs(VS::kAllShaderStageInputs,
                                     VS::kInterleavedBufferLayout);
-  vertex_descriptor->RegisterDescriptorSetLayouts(VS::kDescriptorSetLayouts);
+
+  std::array<DescriptorSetLayout, 2> descriptor_set_layouts = {
+      VS::kDescriptorSetLayouts[0],
+      DescriptorSetLayout{
+          .binding = 64u,
+          .descriptor_type = DescriptorType::kUniformBuffer,
+          .shader_stage = ShaderStage::kFragment,
+      },
+  };
+  vertex_descriptor->RegisterDescriptorSetLayouts(descriptor_set_layouts);
 
   desc.SetVertexDescriptor(std::move(vertex_descriptor));
   ColorAttachmentDescriptor color0;
diff --git a/impeller/scene/shaders/unlit.frag b/impeller/scene/shaders/unlit.frag
@@ -17,8 +17,13 @@ in vec4 v_color;
 
 out vec4 frag_color;
 
+float consume_unused() {
+  return (v_position.x + v_tangent_space[0][0]) * 0.001;
+}
+
 void main() {
   vec4 vertex_color = mix(vec4(1), v_color, frag_info.vertex_color_weight);
   frag_color = texture(base_color_texture, v_texture_coords) * vertex_color *
-               frag_info.color;
+                   frag_info.color +
+               consume_unused();
 }
