From 679be5fa6ea8344c4d09f280cd3bec23b50d4e16 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 11 Aug 2022 18:17:38 -0700 Subject: [PATCH 1/2] Revert special character escaping --- impeller/compiler/compiler.cc | 13 +------ impeller/compiler/compiler_test.cc | 38 ------------------- impeller/compiler/compiler_test.h | 2 - impeller/compiler/compiler_unittests.cc | 5 --- .../fixtures/shaders/general_shaders/BUILD.gn | 1 - .../functions%20with_space.frag | 34 ----------------- 6 files changed, 1 insertion(+), 92 deletions(-) delete mode 100644 lib/ui/fixtures/shaders/general_shaders/functions%20with_space.frag diff --git a/impeller/compiler/compiler.cc b/impeller/compiler/compiler.cc index 795f9962a7479..b5fb91100b64a 100644 --- a/impeller/compiler/compiler.cc +++ b/impeller/compiler/compiler.cc @@ -464,24 +464,13 @@ const std::vector& Compiler::GetIncludedFileNames() const { return included_file_names_; } -// Escape `%` and `#` characters according to doc comment at -// https://github.com/ninja-build/ninja/blob/master/src/depfile_parser.cc#L28 -static void EscapeString(std::string& str, std::stringstream& stream) { - for (auto it = str.begin(); it != str.end(); it++) { - if (*it == '%' || *it == '#') { - stream << '\\'; - } - stream << *it; - } -} - static std::string JoinStrings(std::vector items, std::string separator) { std::stringstream stream; for (size_t i = 0, count = items.size(); i < count; i++) { const auto is_last = (i == count - 1); - EscapeString(items[i], stream); + stream << items[i]; if (!is_last) { stream << separator; } diff --git a/impeller/compiler/compiler_test.cc b/impeller/compiler/compiler_test.cc index 79285051f1f81..b3aece6b5cb9b 100644 --- a/impeller/compiler/compiler_test.cc +++ b/impeller/compiler/compiler_test.cc @@ -58,37 +58,6 @@ static std::string SLFileName(const char* fixture_name, return stream.str(); } -static std::string DepfileName(const char* fixture_name) { - std::stringstream stream; - stream << fixture_name << ".d"; - return stream.str(); -} - -bool CompilerTest::ValidateDepfileEscaped(const char* fixture_name) const { - auto depfile_name = DepfileName(fixture_name); - auto mapping = std::make_unique( - fml::OpenFile(intermediates_directory_, depfile_name.c_str(), false, - fml::FilePermission::kRead)); - - std::string contents(reinterpret_cast(mapping->GetMapping()), - mapping->GetSize()); - bool escaped = false; - for (auto it = contents.begin(); it != contents.end(); it++) { - if (*it == '\\') { - escaped = true; - } else if (*it == '%' || *it == '#') { - if (!escaped) { - VALIDATION_LOG << "Unescaped character " << *it << " in depfile."; - return false; - } - escaped = false; - } else { - escaped = false; - } - } - return true; -} - bool CompilerTest::CanCompileAndReflect(const char* fixture_name, SourceType source_type) const { auto fixture = flutter::testing::OpenFixtureAsMapping(fixture_name); @@ -190,13 +159,6 @@ bool CompilerTest::CanCompileAndReflect(const char* fixture_name, return false; } } - - auto mapping = compiler.CreateDepfileContents({fixture_name}); - if (!fml::WriteAtomically(intermediates_directory_, - DepfileName(fixture_name).c_str(), *mapping)) { - VALIDATION_LOG << "Could not write depfile."; - return false; - } return true; } diff --git a/impeller/compiler/compiler_test.h b/impeller/compiler/compiler_test.h index be9504de15dac..a7be1c688b198 100644 --- a/impeller/compiler/compiler_test.h +++ b/impeller/compiler/compiler_test.h @@ -25,8 +25,6 @@ class CompilerTest : public ::testing::TestWithParam { const char* fixture_name, SourceType source_type = SourceType::kUnknown) const; - bool ValidateDepfileEscaped(const char* fixture_name) const; - private: fml::UniqueFD intermediates_directory_; diff --git a/impeller/compiler/compiler_unittests.cc b/impeller/compiler/compiler_unittests.cc index 68084d62d35ca..23101e313f90a 100644 --- a/impeller/compiler/compiler_unittests.cc +++ b/impeller/compiler/compiler_unittests.cc @@ -65,11 +65,6 @@ TEST_P(CompilerTest, MustFailDueToMultipleLocationPerStructMember) { ASSERT_FALSE(CanCompileAndReflect("struct_def_bug.vert")); } -TEST_P(CompilerTest, ShaderWithSpecialCharactersHasEscapedDepfile) { - ASSERT_TRUE(CanCompileAndReflect("sa\%m#ple.vert")); - ASSERT_TRUE(ValidateDepfileEscaped("sa\%m#ple.vert")); -} - #define INSTANTIATE_TARGET_PLATFORM_TEST_SUITE_P(suite_name) \ INSTANTIATE_TEST_SUITE_P( \ suite_name, CompilerTest, \ diff --git a/lib/ui/fixtures/shaders/general_shaders/BUILD.gn b/lib/ui/fixtures/shaders/general_shaders/BUILD.gn index eb14c9ca2f859..640823715e8f2 100644 --- a/lib/ui/fixtures/shaders/general_shaders/BUILD.gn +++ b/lib/ui/fixtures/shaders/general_shaders/BUILD.gn @@ -11,7 +11,6 @@ if (enable_unittests) { "blue_green_sampler.frag", "children_and_uniforms.frag", "functions.frag", - "functions%20with_space.frag", "no_builtin_redefinition.frag", "no_uniforms.frag", "simple.frag", diff --git a/lib/ui/fixtures/shaders/general_shaders/functions%20with_space.frag b/lib/ui/fixtures/shaders/general_shaders/functions%20with_space.frag deleted file mode 100644 index 52bf92dbcc1cb..0000000000000 --- a/lib/ui/fixtures/shaders/general_shaders/functions%20with_space.frag +++ /dev/null @@ -1,34 +0,0 @@ -#version 320 es - -// 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. - -precision highp float; - -layout ( location = 0 ) out vec4 oColor; - -layout ( location = 0 ) uniform float a; // should be 1.0 - -float addA(float x) { - return x + a; -} - -vec2 pairWithA(float x) { - return vec2(x, a); -} - -vec3 composedFunction(float x) { - return vec3(addA(x), pairWithA(x)); -} - -float multiParam(float x, float y, float z) { - return x * y * z * a; -} - -void main() { - float x = addA(0.0); // x = 0 + 1; - vec3 v3 = composedFunction(x); // v3 = vec3(2, 1, 1); - x = multiParam(v3.x, v3.y, v3.z); // x = 2 * 1 * 1 * 1; - oColor = vec4(0.0, x / 2.0, 0.0, 1.0); // vec4(0, 1, 0, 1); -} From 9493723e2e92c324486a2527880816175358196b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Aug 2022 15:50:44 -0700 Subject: [PATCH 2/2] ++ --- testing/dart/fragment_shader_test.dart | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/testing/dart/fragment_shader_test.dart b/testing/dart/fragment_shader_test.dart index d3b1b33521145..7fdcdf2742654 100644 --- a/testing/dart/fragment_shader_test.dart +++ b/testing/dart/fragment_shader_test.dart @@ -24,16 +24,6 @@ void main() async { _expectShaderRendersGreen(shader); }); - test('simple shader with space in key renders correctly', () async { - final FragmentProgram program = await FragmentProgram.fromAsset( - 'functions with_space.frag.iplr', - ); - final Shader shader = program.shader( - floatUniforms: Float32List.fromList([1]), - ); - _expectShaderRendersGreen(shader); - }); - test('blue-green image renders green', () async { final FragmentProgram program = await FragmentProgram.fromAsset( 'blue_green_sampler.frag.iplr',