[Impeller] moved to bgra10_xr (flutter/engine#52019)

fixes https://github.com/flutter/flutter/issues/145933

This required that we moved the golden image tests to arm64 since the wide gamut tests would now require BGRA10_XR and that's only available to arm64.

tests: in framework repo https://github.com/flutter/flutter/tree/master/dev/integration_tests/wide_gamut_test. There was a test added to that suite specifically for this case when we turned off BGRA10_XR the first time.

This has a dependency on https://github.com/flutter/engine/pull/51998 which includes the necessary skia change.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
This commit is contained in:
gaaclarke 2024-04-15 11:03:51 -07:00 committed by GitHub
parent 6f2a94ded6
commit 47ad73b01c
10 changed files with 76 additions and 34 deletions

View File

@ -215,7 +215,6 @@
"flutter/build/archives:archive_gen_snapshot",
"flutter/build/archives:artifacts",
"flutter/build/dart:copy_dart_sdk",
"flutter/impeller/golden_tests:impeller_golden_tests",
"flutter/shell/platform/darwin/macos:zip_macos_flutter_framework",
"flutter/tools/font_subset",
"flutter:unittests"
@ -249,7 +248,7 @@
"--variant",
"ci/host_release",
"--type",
"dart,dart-host,engine,impeller-golden"
"dart,dart-host,engine"
]
}
]
@ -405,7 +404,7 @@
"drone_dimensions": [
"device_type=none",
"os=Mac-13",
"cpu=x86"
"cpu=arm64"
],
"gclient_variables": {
"download_android_deps": false,
@ -423,16 +422,21 @@
"--prebuilt-dart-sdk",
"--rbe",
"--no-goma",
"--xcode-symlinks"
"--xcode-symlinks",
"--use-glfw-swiftshader"
],
"name": "ci/mac_release_arm64",
"description": "Produces release mode arm64 macOS host-side tooling.",
"ninja": {
"config": "ci/mac_release_arm64",
"targets": [
"flutter/tools/font_subset",
"flutter:unittests",
"flutter/build/archives:archive_gen_snapshot",
"flutter/build/archives:artifacts",
"flutter/shell/platform/darwin/macos:zip_macos_flutter_framework"
"flutter/build/dart:copy_dart_sdk",
"flutter/impeller/golden_tests:impeller_golden_tests",
"flutter/shell/platform/darwin/macos:zip_macos_flutter_framework",
"flutter/tools/font_subset"
]
},
"postsubmit_overrides": {
@ -454,7 +458,20 @@
"$flutter/osx_sdk": {
"sdk_version": "15a240d"
}
}
},
"tests": [
{
"language": "python3",
"name": "Impeller-golden for host_release",
"script": "flutter/testing/run_tests.py",
"parameters": [
"--variant",
"ci/mac_release_arm64",
"--type",
"impeller-golden"
]
}
]
}
],
"generators": {

View File

@ -132,11 +132,8 @@ TEST_P(AiksTest, PaintBlendModeIsRespected) {
// Bug: https://github.com/flutter/flutter/issues/142549
TEST_P(AiksTest, BlendModePlusAlphaWideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
PixelFormat::kB10G10R10A10XR);
auto texture = CreateTextureForFixture("airplane.jpg",
/*enable_mipmapping=*/true);
@ -158,11 +155,8 @@ TEST_P(AiksTest, BlendModePlusAlphaWideGamut) {
// Bug: https://github.com/flutter/flutter/issues/142549
TEST_P(AiksTest, BlendModePlusAlphaColorFilterWideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
PixelFormat::kB10G10R10A10XR);
auto texture = CreateTextureForFixture("airplane.jpg",
/*enable_mipmapping=*/true);

View File

@ -938,17 +938,14 @@ TEST_P(AiksTest, CanDrawPaintMultipleTimes) {
}
// This makes sure the WideGamut named tests use 16bit float pixel format.
TEST_P(AiksTest, F16WideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
TEST_P(AiksTest, FormatWideGamut) {
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
EXPECT_FALSE(IsAlphaClampedToOne(
PixelFormat::kB10G10R10A10XR);
EXPECT_TRUE(IsAlphaClampedToOne(
GetContext()->GetCapabilities()->GetDefaultColorFormat()));
}
TEST_P(AiksTest, NotF16) {
TEST_P(AiksTest, FormatSRGB) {
EXPECT_TRUE(IsAlphaClampedToOne(
GetContext()->GetCapabilities()->GetDefaultColorFormat()));
}
@ -3107,12 +3104,8 @@ TEST_P(AiksTest, MipmapGenerationWorksCorrectly) {
}
TEST_P(AiksTest, DrawAtlasPlusWideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
PixelFormat::kB10G10R10A10XR);
// Draws the image as four squares stiched together.
auto atlas =

View File

@ -128,6 +128,16 @@ void GoldenPlaygroundTest::TearDown() {
ASSERT_FALSE(dlopen("/usr/local/lib/libMoltenVK.dylib", RTLD_NOLOAD));
}
namespace {
bool DoesSupportWideGamutTests() {
#ifdef __arm64__
return true;
#else
return false;
#endif
}
} // namespace
void GoldenPlaygroundTest::SetUp() {
std::filesystem::path testing_assets_path =
flutter::testing::GetTestingAssetsPath();
@ -142,10 +152,17 @@ void GoldenPlaygroundTest::SetUp() {
bool enable_wide_gamut = test_name.find("WideGamut_") != std::string::npos;
switch (GetParam()) {
case PlaygroundBackend::kMetal:
if (!DoesSupportWideGamutTests()) {
GTEST_SKIP_(
"This metal device doesn't support wide gamut golden tests.");
}
pimpl_->screenshotter =
std::make_unique<testing::MetalScreenshotter>(enable_wide_gamut);
break;
case PlaygroundBackend::kVulkan: {
if (enable_wide_gamut) {
GTEST_SKIP_("Vulkan doesn't support wide gamut golden tests.");
}
const std::unique_ptr<PlaygroundImpl>& playground =
GetSharedVulkanPlayground(/*enable_validations=*/true);
pimpl_->screenshotter =
@ -153,6 +170,9 @@ void GoldenPlaygroundTest::SetUp() {
break;
}
case PlaygroundBackend::kOpenGLES: {
if (enable_wide_gamut) {
GTEST_SKIP_("OpenGLES doesn't support wide gamut golden tests.");
}
FML_CHECK(::glfwInit() == GLFW_TRUE);
PlaygroundSwitches playground_switches;
playground_switches.use_angle = true;

View File

@ -73,11 +73,12 @@ PlaygroundImplMTL::PlaygroundImplMTL(PlaygroundSwitches switches)
if (!window) {
return;
}
auto context = ContextMTL::Create(
ShaderLibraryMappingsForPlayground(), is_gpu_disabled_sync_switch_,
"Playground Library",
switches.enable_wide_gamut
? std::optional<PixelFormat>(PixelFormat::kR16G16B16A16Float)
? std::optional<PixelFormat>(PixelFormat::kB10G10R10A10XR)
: std::nullopt);
if (!context) {
return;

View File

@ -15,6 +15,16 @@ PlaygroundTest::PlaygroundTest()
PlaygroundTest::~PlaygroundTest() = default;
namespace {
bool DoesSupportWideGamutTests() {
#ifdef __arm64__
return true;
#else
return false;
#endif
}
} // namespace
void PlaygroundTest::SetUp() {
if (!Playground::SupportsBackend(GetParam())) {
GTEST_SKIP_("Playground doesn't support this backend type.");
@ -34,6 +44,12 @@ void PlaygroundTest::SetUp() {
switches.enable_wide_gamut =
test_name.find("WideGamut/") != std::string::npos;
if (switches.enable_wide_gamut && (GetParam() != PlaygroundBackend::kMetal ||
!DoesSupportWideGamutTests())) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
return;
}
SetupContext(GetParam(), switches);
SetupWindow();
}

View File

@ -25,6 +25,8 @@ std::optional<SkColorType> ToSkColorType(impeller::PixelFormat format) {
return SkColorType::kBGRA_8888_SkColorType;
case impeller::PixelFormat::kB10G10R10XR:
return SkColorType::kBGR_101010x_XR_SkColorType;
case impeller::PixelFormat::kB10G10R10A10XR:
return SkColorType::kBGRA_10101010_XR_SkColorType;
default:
return std::nullopt;
}

View File

@ -259,6 +259,9 @@ extern CFTimeInterval display_link_target;
} else if (self.pixelFormat == MTLPixelFormatBGRA8Unorm) {
pixelFormat = kCVPixelFormatType_32BGRA;
bytesPerElement = 4;
} else if (self.pixelFormat == MTLPixelFormatBGRA10_XR) {
pixelFormat = kCVPixelFormatType_40ARGBLEWideGamut;
bytesPerElement = 8;
} else {
FML_LOG(ERROR) << "Unsupported pixel format: " << self.pixelFormat;
return nil;

View File

@ -59,7 +59,7 @@
CAMetalLayer* layer = (CAMetalLayer*)self.layer;
#pragma clang diagnostic pop
layer.pixelFormat = pixelFormat;
if (pixelFormat == MTLPixelFormatRGBA16Float) {
if (pixelFormat == MTLPixelFormatRGBA16Float || pixelFormat == MTLPixelFormatBGRA10_XR) {
self->_colorSpaceRef = fml::CFRef(CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB));
layer.colorspace = self->_colorSpaceRef;
}

View File

@ -122,11 +122,7 @@ static void PrintWideGamutWarningOnce() {
CGColorSpaceRef srgb = CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB);
layer.colorspace = srgb;
CFRelease(srgb);
// MTLPixelFormatRGBA16Float was chosen since it is compatible with
// impeller's offscreen buffers which need to have transparency. Also,
// F16 was chosen over BGRA10_XR since Skia does not support decoding
// BGRA10_XR.
layer.pixelFormat = MTLPixelFormatRGBA16Float;
layer.pixelFormat = MTLPixelFormatBGRA10_XR;
} else if (_isWideGamutEnabled && !isWideGamutSupported) {
PrintWideGamutWarningOnce();
}