From 8b4cc52f2d199b0351ca75029feea1e798c3eb99 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Mon, 1 Aug 2022 15:24:03 -0400 Subject: [PATCH] [impeller] Set binding base for fragment shaders (flutter/engine#35052) Fixes: https://github.com/flutter/flutter/issues/108739 --- .../src/flutter/impeller/compiler/compiler.cc | 15 ++++++++++++ .../src/flutter/impeller/compiler/compiler.h | 2 ++ .../impeller/compiler/compiler_test.cc | 12 ++++++++++ .../flutter/impeller/compiler/compiler_test.h | 3 +++ .../impeller/compiler/compiler_unittests.cc | 23 +++++++++++++++++++ engine/src/flutter/impeller/fixtures/BUILD.gn | 1 + .../src/flutter/impeller/fixtures/sample.frag | 14 +++++++++++ 7 files changed, 70 insertions(+) create mode 100644 engine/src/flutter/impeller/fixtures/sample.frag diff --git a/engine/src/flutter/impeller/compiler/compiler.cc b/engine/src/flutter/impeller/compiler/compiler.cc index e5b2018fd26..e38aa147145 100644 --- a/engine/src/flutter/impeller/compiler/compiler.cc +++ b/engine/src/flutter/impeller/compiler/compiler.cc @@ -13,10 +13,15 @@ #include "impeller/compiler/compiler_backend.h" #include "impeller/compiler/includer.h" #include "impeller/compiler/logger.h" +#include "impeller/compiler/types.h" namespace impeller { namespace compiler { +const uint32_t kFragBindingBase = 128; +const size_t kNumUniformKinds = + int(shaderc_uniform_kind::shaderc_uniform_kind_buffer) + 1; + static CompilerBackend CreateMSLCompiler(const spirv_cross::ParsedIR& ir, const SourceOptions& source_options) { auto sl_compiler = std::make_shared(ir); @@ -104,6 +109,15 @@ static CompilerBackend CreateCompiler(const spirv_cross::ParsedIR& ir, return compiler; } +void Compiler::SetBindingBase(shaderc::CompileOptions& compiler_opts) const { + for (size_t uniform_kind = 0; uniform_kind < kNumUniformKinds; + uniform_kind++) { + compiler_opts.SetBindingBaseForStage( + ToShaderCShaderKind(SourceType::kFragmentShader), + static_cast(uniform_kind), kFragBindingBase); + } +} + Compiler::Compiler(const fml::Mapping& source_mapping, SourceOptions source_options, Reflector::Options reflector_options) @@ -212,6 +226,7 @@ Compiler::Compiler(const fml::Mapping& source_mapping, } spirv_options.SetAutoBindUniforms(true); + SetBindingBase(spirv_options); spirv_options.SetAutoMapLocations(true); std::vector included_file_names; diff --git a/engine/src/flutter/impeller/compiler/compiler.h b/engine/src/flutter/impeller/compiler/compiler.h index 9dd3d901c3d..379f610fd22 100644 --- a/engine/src/flutter/impeller/compiler/compiler.h +++ b/engine/src/flutter/impeller/compiler/compiler.h @@ -57,6 +57,8 @@ class Compiler { std::string GetDependencyNames(std::string separator) const; + void SetBindingBase(shaderc::CompileOptions& compiler_opts) const; + FML_DISALLOW_COPY_AND_ASSIGN(Compiler); }; diff --git a/engine/src/flutter/impeller/compiler/compiler_test.cc b/engine/src/flutter/impeller/compiler/compiler_test.cc index 85b3abaa5f9..402d05fe734 100644 --- a/engine/src/flutter/impeller/compiler/compiler_test.cc +++ b/engine/src/flutter/impeller/compiler/compiler_test.cc @@ -6,6 +6,11 @@ #include +#include "flutter/fml/file.h" +#include "flutter/fml/logging.h" +#include "flutter/fml/mapping.h" +#include "flutter/fml/unique_fd.h" + namespace impeller { namespace compiler { namespace testing { @@ -58,6 +63,13 @@ static std::string SLFileName(const char* fixture_name, return stream.str(); } +std::unique_ptr CompilerTest::GetReflectionJson( + const char* fixture_name) const { + auto filename = ReflectionJSONName(fixture_name); + auto fd = fml::OpenFileReadOnly(intermediates_directory_, filename.c_str()); + return fml::FileMapping::CreateReadOnly(fd); +} + bool CompilerTest::CanCompileAndReflect(const char* fixture_name, SourceType source_type) const { auto fixture = flutter::testing::OpenFixtureAsMapping(fixture_name); diff --git a/engine/src/flutter/impeller/compiler/compiler_test.h b/engine/src/flutter/impeller/compiler/compiler_test.h index a7be1c688b1..0e4fd94fd39 100644 --- a/engine/src/flutter/impeller/compiler/compiler_test.h +++ b/engine/src/flutter/impeller/compiler/compiler_test.h @@ -25,6 +25,9 @@ class CompilerTest : public ::testing::TestWithParam { const char* fixture_name, SourceType source_type = SourceType::kUnknown) const; + std::unique_ptr GetReflectionJson( + const char* fixture_name) const; + private: fml::UniqueFD intermediates_directory_; diff --git a/engine/src/flutter/impeller/compiler/compiler_unittests.cc b/engine/src/flutter/impeller/compiler/compiler_unittests.cc index d19d9f00c98..f46b745c817 100644 --- a/engine/src/flutter/impeller/compiler/compiler_unittests.cc +++ b/engine/src/flutter/impeller/compiler/compiler_unittests.cc @@ -9,6 +9,8 @@ #include "impeller/compiler/source_options.h" #include "impeller/compiler/types.h" +#include "nlohmann/json.hpp" + namespace impeller { namespace compiler { namespace testing { @@ -50,6 +52,27 @@ TEST_P(CompilerTest, CanCompileComputeShader) { ASSERT_TRUE(CanCompileAndReflect("sample.comp", SourceType::kComputeShader)); } +TEST_P(CompilerTest, BindingBaseForFragShader) { + if (GetParam() == TargetPlatform::kFlutterSPIRV) { + // This is a failure of reflection which this target doesn't perform. + GTEST_SKIP(); + } + + ASSERT_TRUE(CanCompileAndReflect("sample.vert", SourceType::kVertexShader)); + ASSERT_TRUE(CanCompileAndReflect("sample.frag", SourceType::kFragmentShader)); + + auto get_binding = [&](const char* fixture) -> uint32_t { + auto json_fd = GetReflectionJson(fixture); + nlohmann::json shader_json = nlohmann::json::parse(json_fd->GetMapping()); + return shader_json["buffers"][0]["binding"].get(); + }; + + auto vert_uniform_binding = get_binding("sample.vert"); + auto frag_uniform_binding = get_binding("sample.frag"); + + ASSERT_GT(frag_uniform_binding, vert_uniform_binding); +} + TEST_P(CompilerTest, MustFailDueToMultipleLocationPerStructMember) { if (GetParam() == TargetPlatform::kFlutterSPIRV) { // This is a failure of reflection which this target doesn't perform. diff --git a/engine/src/flutter/impeller/fixtures/BUILD.gn b/engine/src/flutter/impeller/fixtures/BUILD.gn index d955a90bfa9..635125cc1ec 100644 --- a/engine/src/flutter/impeller/fixtures/BUILD.gn +++ b/engine/src/flutter/impeller/fixtures/BUILD.gn @@ -43,6 +43,7 @@ test_fixtures("file_fixtures") { "embarcadero.jpg", "kalimba.jpg", "sample.comp", + "sample.frag", "sample.tesc", "sample.tese", "sample.vert", diff --git a/engine/src/flutter/impeller/fixtures/sample.frag b/engine/src/flutter/impeller/fixtures/sample.frag new file mode 100644 index 00000000000..5d9c83604dd --- /dev/null +++ b/engine/src/flutter/impeller/fixtures/sample.frag @@ -0,0 +1,14 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +uniform FragInfo { + vec4 color; +} +frag_info; + +out vec4 frag_color; + +void main() { + frag_color = frag_info.color; +}