Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit fc72934

Browse files
authored
[Impeller] Don't double-convert include path encodings in ImpellerC (#37408)
1 parent 9628070 commit fc72934

File tree

5 files changed

+52
-2
lines changed

5 files changed

+52
-2
lines changed

ci/licenses_golden/licenses_flutter

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,6 +1124,7 @@ FILE: ../../../flutter/impeller/compiler/spirv_sksl.cc
11241124
FILE: ../../../flutter/impeller/compiler/spirv_sksl.h
11251125
FILE: ../../../flutter/impeller/compiler/switches.cc
11261126
FILE: ../../../flutter/impeller/compiler/switches.h
1127+
FILE: ../../../flutter/impeller/compiler/switches_unittests.cc
11271128
FILE: ../../../flutter/impeller/compiler/types.cc
11281129
FILE: ../../../flutter/impeller/compiler/types.h
11291130
FILE: ../../../flutter/impeller/compiler/utilities.cc

impeller/compiler/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ impeller_component("compiler_unittests") {
120120
"compiler_test.cc",
121121
"compiler_test.h",
122122
"compiler_unittests.cc",
123+
"switches_unittests.cc",
123124
]
124125

125126
deps = [

impeller/compiler/switches.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,17 @@ Switches::Switches(const fml::CommandLine& command_line)
127127

128128
// fml::OpenDirectoryReadOnly for Windows doesn't handle relative paths
129129
// beginning with `../` well, so we build an absolute path.
130+
131+
// Get the current working directory as a utf8 encoded string.
132+
// Note that the `include_dir_path` is already utf8 encoded, and so we
133+
// mustn't attempt to double-convert it to utf8 lest multi-byte characters
134+
// will become mangled.
135+
auto cwd = Utf8FromPath(std::filesystem::current_path());
130136
auto include_dir_absolute = std::filesystem::absolute(
131-
std::filesystem::current_path() / include_dir_path);
137+
std::filesystem::path(cwd) / include_dir_path);
132138

133139
auto dir = std::make_shared<fml::UniqueFD>(fml::OpenDirectoryReadOnly(
134-
*working_directory, Utf8FromPath(include_dir_absolute).c_str()));
140+
*working_directory, include_dir_absolute.string().c_str()));
135141
if (!dir || !dir->is_valid()) {
136142
continue;
137143
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/fml/command_line.h"
6+
#include "flutter/fml/file.h"
7+
#include "flutter/testing/testing.h"
8+
#include "impeller/compiler/switches.h"
9+
#include "impeller/compiler/utilities.h"
10+
11+
namespace impeller {
12+
namespace compiler {
13+
namespace testing {
14+
15+
TEST(SwitchesTest, DoesntMangleUnicodeIncludes) {
16+
const char* directory_name = "test_shader_include_�";
17+
fml::CreateDirectory(flutter::testing::OpenFixturesDirectory(),
18+
{directory_name}, fml::FilePermission::kRead);
19+
20+
auto include_path =
21+
std::string(flutter::testing::GetFixturesPath()) + "/" + directory_name;
22+
auto include_option = "--include=" + include_path;
23+
24+
const auto cl = fml::CommandLineFromInitializerList(
25+
{"impellerc", "--opengl-desktop", "--input=input.vert",
26+
"--sl=output.vert", "--spirv=output.spirv", include_option.c_str()});
27+
Switches switches(cl);
28+
ASSERT_TRUE(switches.AreValid(std::cout));
29+
ASSERT_EQ(switches.include_directories.size(), 1u);
30+
ASSERT_NE(switches.include_directories[0].dir, nullptr);
31+
ASSERT_EQ(switches.include_directories[0].name, include_path);
32+
}
33+
34+
} // namespace testing
35+
} // namespace compiler
36+
} // namespace impeller

impeller/compiler/utilities.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@
1313
namespace impeller {
1414
namespace compiler {
1515

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

1824
std::string InferShaderNameFromPath(std::string_view path);

0 commit comments

Comments
 (0)