Skip to content

Commit

Permalink
[Impeller] Don't double-convert include path encodings in ImpellerC (f…
Browse files Browse the repository at this point in the history
  • Loading branch information
bdero authored and naudzghebre committed Nov 9, 2022
1 parent 5bccf58 commit 22593f9
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,7 @@ FILE: ../../../flutter/impeller/compiler/spirv_sksl.cc
FILE: ../../../flutter/impeller/compiler/spirv_sksl.h
FILE: ../../../flutter/impeller/compiler/switches.cc
FILE: ../../../flutter/impeller/compiler/switches.h
FILE: ../../../flutter/impeller/compiler/switches_unittests.cc
FILE: ../../../flutter/impeller/compiler/types.cc
FILE: ../../../flutter/impeller/compiler/types.h
FILE: ../../../flutter/impeller/compiler/utilities.cc
Expand Down
1 change: 1 addition & 0 deletions impeller/compiler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ impeller_component("compiler_unittests") {
"compiler_test.cc",
"compiler_test.h",
"compiler_unittests.cc",
"switches_unittests.cc",
]

deps = [
Expand Down
10 changes: 8 additions & 2 deletions impeller/compiler/switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,17 @@ Switches::Switches(const fml::CommandLine& command_line)

// fml::OpenDirectoryReadOnly for Windows doesn't handle relative paths
// beginning with `../` well, so we build an absolute path.

// Get the current working directory as a utf8 encoded string.
// Note that the `include_dir_path` is already utf8 encoded, and so we
// mustn't attempt to double-convert it to utf8 lest multi-byte characters
// will become mangled.
auto cwd = Utf8FromPath(std::filesystem::current_path());
auto include_dir_absolute = std::filesystem::absolute(
std::filesystem::current_path() / include_dir_path);
std::filesystem::path(cwd) / include_dir_path);

auto dir = std::make_shared<fml::UniqueFD>(fml::OpenDirectoryReadOnly(
*working_directory, Utf8FromPath(include_dir_absolute).c_str()));
*working_directory, include_dir_absolute.string().c_str()));
if (!dir || !dir->is_valid()) {
continue;
}
Expand Down
36 changes: 36 additions & 0 deletions impeller/compiler/switches_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// 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.

#include "flutter/fml/command_line.h"
#include "flutter/fml/file.h"
#include "flutter/testing/testing.h"
#include "impeller/compiler/switches.h"
#include "impeller/compiler/utilities.h"

namespace impeller {
namespace compiler {
namespace testing {

TEST(SwitchesTest, DoesntMangleUnicodeIncludes) {
const char* directory_name = "test_shader_include_�";
fml::CreateDirectory(flutter::testing::OpenFixturesDirectory(),
{directory_name}, fml::FilePermission::kRead);

auto include_path =
std::string(flutter::testing::GetFixturesPath()) + "/" + directory_name;
auto include_option = "--include=" + include_path;

const auto cl = fml::CommandLineFromInitializerList(
{"impellerc", "--opengl-desktop", "--input=input.vert",
"--sl=output.vert", "--spirv=output.spirv", include_option.c_str()});
Switches switches(cl);
ASSERT_TRUE(switches.AreValid(std::cout));
ASSERT_EQ(switches.include_directories.size(), 1u);
ASSERT_NE(switches.include_directories[0].dir, nullptr);
ASSERT_EQ(switches.include_directories[0].name, include_path);
}

} // namespace testing
} // namespace compiler
} // namespace impeller
6 changes: 6 additions & 0 deletions impeller/compiler/utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
namespace impeller {
namespace compiler {

/// @brief Converts a native format path to a utf8 string.
///
/// This utility uses `path::u8string()` to convert native paths to
/// utf8. If the given path doesn't match the underlying native path
/// format, and the native path format isn't utf8 (i.e. Windows, which
/// has utf16 paths), the path will get mangled.
std::string Utf8FromPath(const std::filesystem::path& path);

std::string InferShaderNameFromPath(std::string_view path);
Expand Down

0 comments on commit 22593f9

Please sign in to comment.