From 8d484fc4ed6e8173411f35383b5d663d3763ea53 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Tue, 25 Mar 2025 19:46:47 +0000 Subject: [PATCH] Reverts "[iOS] reduce wide gamut memory by 50% (for onscreen surfaces). (#165601)" (#165915) Reverts: flutter/flutter#165601 Initiated by: jonahwilliams Reason for reverting: descriptor mismatch not caught in testing. Maybe missing feature? Original PR Author: jonahwilliams Reviewed By: {gaaclarke} This change reverts the following previous change: If the onscreen surface is opaque, resolve to an alpha-less wide gamut format. This reduces the bits per color from 40 to 32, which allows us to fit in a 32 bit texture size, down from 64. This should reduce memory usage by nearly 50 MB for typical applications. Co-authored-by: auto-submit[bot] --- .../flutter/impeller/display_list/canvas.cc | 18 +++++----------- .../impeller/display_list/canvas_unittests.cc | 18 ++-------------- .../backend/metal/swapchain_transients_mtl.mm | 6 +----- .../swapchain_transients_mtl_unittests.mm | 8 ------- .../ios/framework/Source/FlutterMetalLayer.mm | 3 --- .../framework/Source/FlutterOverlayView.mm | 5 +---- .../ios/framework/Source/FlutterView.mm | 21 ++++++++++++------- .../darwin/ios/ios_surface_metal_impeller.mm | 9 ++------ 8 files changed, 24 insertions(+), 64 deletions(-) diff --git a/engine/src/flutter/impeller/display_list/canvas.cc b/engine/src/flutter/impeller/display_list/canvas.cc index 78f3b7bf05..9c3380db23 100644 --- a/engine/src/flutter/impeller/display_list/canvas.cc +++ b/engine/src/flutter/impeller/display_list/canvas.cc @@ -1723,20 +1723,12 @@ bool Canvas::SupportsBlitToOnscreen() const { } bool Canvas::BlitToOnscreen(bool is_onscreen) { - std::shared_ptr command_buffer = - renderer_.GetContext()->CreateCommandBuffer(); + auto command_buffer = renderer_.GetContext()->CreateCommandBuffer(); command_buffer->SetLabel("EntityPass Root Command Buffer"); - RenderTarget offscreen_target = render_passes_.back() - .inline_pass_context->GetPassTarget() - .GetRenderTarget(); - // If the src and destination format differ (due to wide gamut, alpha-less - // format, et cetera), then a draw must always be performed instead of a blit. - if (SupportsBlitToOnscreen() && - offscreen_target.GetRenderTargetTexture() - ->GetTextureDescriptor() - .format == render_target_.GetRenderTargetTexture() - ->GetTextureDescriptor() - .format) { + auto offscreen_target = render_passes_.back() + .inline_pass_context->GetPassTarget() + .GetRenderTarget(); + if (SupportsBlitToOnscreen()) { auto blit_pass = command_buffer->CreateBlitPass(); blit_pass->AddCopy(offscreen_target.GetRenderTargetTexture(), render_target_.GetRenderTargetTexture()); diff --git a/engine/src/flutter/impeller/display_list/canvas_unittests.cc b/engine/src/flutter/impeller/display_list/canvas_unittests.cc index 2f313a4df9..18382ad750 100644 --- a/engine/src/flutter/impeller/display_list/canvas_unittests.cc +++ b/engine/src/flutter/impeller/display_list/canvas_unittests.cc @@ -24,12 +24,11 @@ namespace testing { std::unique_ptr CreateTestCanvas( ContentContext& context, std::optional cull_rect = std::nullopt, - bool requires_readback = false, - std::optional format = std::nullopt) { + bool requires_readback = false) { TextureDescriptor onscreen_desc; onscreen_desc.size = {100, 100}; onscreen_desc.format = - format.value_or(context.GetDeviceCapabilities().GetDefaultColorFormat()); + context.GetDeviceCapabilities().GetDefaultColorFormat(); onscreen_desc.usage = TextureUsage::kRenderTarget; onscreen_desc.storage_mode = StorageMode::kDevicePrivate; onscreen_desc.sample_count = SampleCount::kCount1; @@ -386,18 +385,5 @@ TEST_P(AiksTest, SupportsBlitToOnscreen) { } } -TEST_P(AiksTest, SupportsBlitToOnscreenWithDifferentFormat) { - if (GetBackend() == PlaygroundBackend::kOpenGLES) { - GTEST_SKIP() << "Not valid on GLES"; - } - // Create an onscreen format which is different than the offscreen format, - // then make the canvas perform a restore to verify a blit is not used. - ContentContext context(GetContext(), nullptr); - auto canvas = CreateTestCanvas(context, Rect::MakeLTRB(0, 0, 100, 100), - /*requires_readback=*/true, - /*format=*/PixelFormat::kB8G8R8A8UNormIntSRGB); - canvas->EndReplay(); -} - } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl.mm index 2e52793298..6527dd588e 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl.mm @@ -52,11 +52,7 @@ std::shared_ptr SwapchainTransientsMTL::GetMSAATexture() { TextureDescriptor desc; desc.size = size_; desc.sample_count = SampleCount::kCount4; - if (format_ == PixelFormat::kB10G10R10XR) { - desc.format = PixelFormat::kB10G10R10A10XR; - } else { - desc.format = format_; - } + desc.format = format_; desc.storage_mode = StorageMode::kDeviceTransient; desc.usage = TextureUsage::kRenderTarget; desc.type = TextureType::kTexture2DMultisample; diff --git a/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl_unittests.mm b/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl_unittests.mm index dea293c61c..40a393084a 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl_unittests.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/swapchain_transients_mtl_unittests.mm @@ -73,14 +73,6 @@ TEST_P(SwapchainTransientsMTLTest, CanAllocateSwapchainTextures) { // Texture cache is invalidated when pixel format changes. transients->SetSizeAndFormat({2, 2}, PixelFormat::kB10G10R10A10XR); EXPECT_NE(resolve, transients->GetResolveTexture()); - - // Transients converts alpha-less format to have alpha for MSAA. - transients->SetSizeAndFormat({2, 2}, PixelFormat::kB10G10R10XR); - - EXPECT_EQ(transients->GetMSAATexture()->GetTextureDescriptor().format, - PixelFormat::kB10G10R10A10XR); - EXPECT_EQ(transients->GetResolveTexture()->GetTextureDescriptor().format, - PixelFormat::kB10G10R10XR); } } // namespace testing diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm index 9933f9a926..f31eda904b 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterMetalLayer.mm @@ -275,9 +275,6 @@ extern CFTimeInterval display_link_target; } else if (self.pixelFormat == MTLPixelFormatBGRA10_XR) { pixelFormat = kCVPixelFormatType_40ARGBLEWideGamut; bytesPerElement = 8; - } else if (self.pixelFormat == MTLPixelFormatBGR10_XR) { - pixelFormat = kCVPixelFormatType_30RGBLEPackedWideGamut; - bytesPerElement = 4; } else { FML_LOG(ERROR) << "Unsupported pixel format: " << self.pixelFormat; return nil; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm index afe969db1c..8000df2830 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm @@ -51,12 +51,9 @@ FLUTTER_ASSERT_ARC CAMetalLayer* layer = (CAMetalLayer*)self.layer; #pragma clang diagnostic pop layer.pixelFormat = pixelFormat; - if (pixelFormat == MTLPixelFormatRGBA16Float || pixelFormat == MTLPixelFormatBGRA10_XR || - pixelFormat == MTLPixelFormatBGR10_XR) { + if (pixelFormat == MTLPixelFormatRGBA16Float || pixelFormat == MTLPixelFormatBGRA10_XR) { self->_colorSpaceRef = fml::CFRef(CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB)); layer.colorspace = self->_colorSpaceRef; - // Overlay layers always need an alpha channel. - layer.pixelFormat = MTLPixelFormatBGRA10_XR; } } return self; diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterView.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterView.mm index b128b90b6c..dd37e8a553 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterView.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterView.mm @@ -78,6 +78,16 @@ FLUTTER_ASSERT_ARC return self; } +static void PrintWideGamutWarningOnce() { + static BOOL did_print = NO; + if (did_print) { + return; + } + FML_DLOG(WARNING) << "Rendering wide gamut colors is turned on but isn't " + "supported, downgrading the color gamut to sRGB."; + did_print = YES; +} + - (void)layoutSubviews { if ([self.layer isKindOfClass:[CAMetalLayer class]]) { // It is a known Apple bug that CAMetalLayer incorrectly reports its supported @@ -86,7 +96,6 @@ FLUTTER_ASSERT_ARC #pragma clang diagnostic ignored "-Wunguarded-availability-new" CAMetalLayer* layer = (CAMetalLayer*)self.layer; #pragma clang diagnostic pop - CGFloat screenScale = self.screen.scale; layer.allowsGroupOpacity = YES; layer.contentsScale = screenScale; @@ -95,13 +104,9 @@ FLUTTER_ASSERT_ARC if (_isWideGamutEnabled && self.isWideGamutSupported) { fml::CFRef srgb(CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB)); layer.colorspace = srgb; - // If the flutter layer is opaque, then use an alpha-less format for the onscreen - // texture. This will reduce wide gamut memory usage by 50%, and Impeller will - // still correctly use alpha for MSAA textures and any offscreen save layer usage. - // For non-wide gamut formats there is no point in removing the alpha channel as - // the textures must align to 32 bits (32 -> 24 = 32) whereas wide gamut is (40 -> 32 = 32) - // instead of 64. - layer.pixelFormat = layer.opaque ? MTLPixelFormatBGR10_XR : MTLPixelFormatBGRA10_XR; + layer.pixelFormat = MTLPixelFormatBGRA10_XR; + } else if (_isWideGamutEnabled && !self.isWideGamutSupported) { + PrintWideGamutWarningOnce(); } } diff --git a/engine/src/flutter/shell/platform/darwin/ios/ios_surface_metal_impeller.mm b/engine/src/flutter/shell/platform/darwin/ios/ios_surface_metal_impeller.mm index f025db100d..8ec9bb8eef 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/ios_surface_metal_impeller.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/ios_surface_metal_impeller.mm @@ -4,7 +4,6 @@ #import "flutter/shell/platform/darwin/ios/ios_surface_metal_impeller.h" -#include "flutter/impeller/core/formats.h" #include "flutter/impeller/renderer/backend/metal/formats_mtl.h" #include "flutter/impeller/renderer/context.h" #include "flutter/shell/gpu/gpu_surface_metal_impeller.h" @@ -44,12 +43,8 @@ void IOSSurfaceMetalImpeller::UpdateStorageSizeIfNecessary() { // |IOSSurface| std::unique_ptr IOSSurfaceMetalImpeller::CreateGPUSurface() { - // Convert alpha-less onscreen format to alpha including format. - impeller::PixelFormat pixel_format = impeller::FromMTLPixelFormat(layer_.pixelFormat); - if (pixel_format == impeller::PixelFormat::kB10G10R10XR) { - pixel_format = impeller::PixelFormat::kB10G10R10A10XR; - } - impeller_context_->UpdateOffscreenLayerPixelFormat(pixel_format); + impeller_context_->UpdateOffscreenLayerPixelFormat( + impeller::FromMTLPixelFormat(layer_.pixelFormat)); return std::make_unique(this, // aiks_context_ // );