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

Commit e993a5c

Browse files
author
Jonah Williams
authored
[Impeller] Ensure that overlay surfaces are constructed with wide gamut settings. (#48190)
Fixes flutter/flutter#138373 When constructing overlay layers when there are platform views, make sure the same pixel format and color space as the main view is used. Add validation to impeller HAL about blitting different pixel formats.
1 parent 3179986 commit e993a5c

File tree

11 files changed

+154
-22
lines changed

11 files changed

+154
-22
lines changed

ci/licenses_golden/excluded_files

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,7 @@
176176
../../../flutter/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc
177177
../../../flutter/impeller/renderer/backend/vulkan/resource_manager_vk_unittests.cc
178178
../../../flutter/impeller/renderer/backend/vulkan/test
179+
../../../flutter/impeller/renderer/blit_pass_unittests.cc
179180
../../../flutter/impeller/renderer/capabilities_unittests.cc
180181
../../../flutter/impeller/renderer/compute_subgroup_unittests.cc
181182
../../../flutter/impeller/renderer/compute_unittests.cc

impeller/renderer/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ impeller_component("renderer_unittests") {
122122
testonly = true
123123

124124
sources = [
125+
"blit_pass_unittests.cc",
125126
"capabilities_unittests.cc",
126127
"device_buffer_unittests.cc",
127128
"host_buffer_unittests.cc",

impeller/renderer/backend/metal/context_mtl.mm

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "flutter/fml/logging.h"
1313
#include "flutter/fml/paths.h"
1414
#include "flutter/fml/synchronization/sync_switch.h"
15+
#include "impeller/core/formats.h"
1516
#include "impeller/core/sampler_descriptor.h"
1617
#include "impeller/renderer/backend/metal/gpu_tracer_mtl.h"
1718
#include "impeller/renderer/backend/metal/sampler_library_mtl.h"

impeller/renderer/blit_pass.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "impeller/base/strings.h"
1010
#include "impeller/base/validation.h"
11+
#include "impeller/core/formats.h"
1112
#include "impeller/core/host_buffer.h"
1213
#include "impeller/renderer/blit_command.h"
1314

@@ -52,6 +53,16 @@ bool BlitPass::AddCopy(std::shared_ptr<Texture> source,
5253
static_cast<int>(destination->GetTextureDescriptor().sample_count));
5354
return false;
5455
}
56+
if (source->GetTextureDescriptor().format !=
57+
destination->GetTextureDescriptor().format) {
58+
VALIDATION_LOG << SPrintF(
59+
"The source pixel format (%s) must match the destination pixel format "
60+
"(%s) "
61+
"for blits.",
62+
PixelFormatToString(source->GetTextureDescriptor().format),
63+
PixelFormatToString(destination->GetTextureDescriptor().format));
64+
return false;
65+
}
5566

5667
if (!source_region.has_value()) {
5768
source_region = IRect::MakeSize(source->GetSize());
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
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 "gtest/gtest.h"
6+
#include "impeller/base/validation.h"
7+
#include "impeller/core/formats.h"
8+
#include "impeller/core/texture_descriptor.h"
9+
#include "impeller/playground/playground_test.h"
10+
#include "impeller/renderer/command_buffer.h"
11+
12+
namespace impeller {
13+
namespace testing {
14+
15+
using BlitPassTest = PlaygroundTest;
16+
INSTANTIATE_PLAYGROUND_SUITE(BlitPassTest);
17+
18+
TEST_P(BlitPassTest, BlitAcrossDifferentPixelFormatsFails) {
19+
ScopedValidationDisable scope; // avoid noise in output.
20+
auto context = GetContext();
21+
auto cmd_buffer = context->CreateCommandBuffer();
22+
auto blit_pass = cmd_buffer->CreateBlitPass();
23+
24+
TextureDescriptor src_desc;
25+
src_desc.format = PixelFormat::kA8UNormInt;
26+
src_desc.size = {100, 100};
27+
auto src = context->GetResourceAllocator()->CreateTexture(src_desc);
28+
29+
TextureDescriptor dst_format;
30+
dst_format.format = PixelFormat::kR8G8B8A8UNormInt;
31+
dst_format.size = {100, 100};
32+
auto dst = context->GetResourceAllocator()->CreateTexture(dst_format);
33+
34+
EXPECT_FALSE(blit_pass->AddCopy(src, dst));
35+
}
36+
37+
TEST_P(BlitPassTest, BlitAcrossDifferentSampleCountsFails) {
38+
ScopedValidationDisable scope; // avoid noise in output.
39+
auto context = GetContext();
40+
auto cmd_buffer = context->CreateCommandBuffer();
41+
auto blit_pass = cmd_buffer->CreateBlitPass();
42+
43+
TextureDescriptor src_desc;
44+
src_desc.format = PixelFormat::kR8G8B8A8UNormInt;
45+
src_desc.sample_count = SampleCount::kCount4;
46+
src_desc.size = {100, 100};
47+
auto src = context->GetResourceAllocator()->CreateTexture(src_desc);
48+
49+
TextureDescriptor dst_format;
50+
dst_format.format = PixelFormat::kR8G8B8A8UNormInt;
51+
dst_format.size = {100, 100};
52+
auto dst = context->GetResourceAllocator()->CreateTexture(dst_format);
53+
54+
EXPECT_FALSE(blit_pass->AddCopy(src, dst));
55+
}
56+
57+
TEST_P(BlitPassTest, BlitPassesForMatchingFormats) {
58+
ScopedValidationDisable scope; // avoid noise in output.
59+
auto context = GetContext();
60+
auto cmd_buffer = context->CreateCommandBuffer();
61+
auto blit_pass = cmd_buffer->CreateBlitPass();
62+
63+
TextureDescriptor src_desc;
64+
src_desc.format = PixelFormat::kR8G8B8A8UNormInt;
65+
src_desc.size = {100, 100};
66+
auto src = context->GetResourceAllocator()->CreateTexture(src_desc);
67+
68+
TextureDescriptor dst_format;
69+
dst_format.format = PixelFormat::kR8G8B8A8UNormInt;
70+
dst_format.size = {100, 100};
71+
auto dst = context->GetResourceAllocator()->CreateTexture(dst_format);
72+
73+
EXPECT_TRUE(blit_pass->AddCopy(src, dst));
74+
}
75+
76+
} // namespace testing
77+
} // namespace impeller

shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,15 @@
55
#ifndef SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_FLUTTER_OVERLAY_VIEW_H_
66
#define SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_FLUTTER_OVERLAY_VIEW_H_
77

8+
#include <Metal/Metal.h>
89
#include <UIKit/UIKit.h>
910

1011
#include <memory>
1112

1213
#include "flutter/fml/memory/weak_ptr.h"
1314
#include "flutter/shell/common/shell.h"
1415
#import "flutter/shell/platform/darwin/ios/ios_surface.h"
16+
#include "fml/platform/darwin/cf_utils.h"
1517

1618
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h"
1719

@@ -27,12 +29,12 @@
2729
/// views also handle touch propagation and the like for touches that occurs
2830
/// either on overlays or otherwise may be intercepted by the platform views.
2931
@interface FlutterOverlayView : UIView
30-
3132
- (instancetype)initWithFrame:(CGRect)frame NS_UNAVAILABLE;
3233
- (instancetype)initWithCoder:(NSCoder*)aDecoder NS_UNAVAILABLE;
3334

3435
- (instancetype)init NS_DESIGNATED_INITIALIZER;
35-
- (instancetype)initWithContentsScale:(CGFloat)contentsScale;
36+
- (instancetype)initWithContentsScale:(CGFloat)contentsScale
37+
pixelFormat:(MTLPixelFormat)pixelFormat;
3638

3739
@end
3840

shell/platform/darwin/ios/framework/Source/FlutterOverlayView.mm

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// found in the LICENSE file.
44

55
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h"
6+
#include <CoreGraphics/CGColorSpace.h>
7+
#include <Metal/Metal.h>
68

79
#include "flutter/common/settings.h"
810
#include "flutter/common/task_runners.h"
@@ -18,7 +20,9 @@
1820

1921
// This is mostly a duplication of FlutterView.
2022
// TODO(amirh): once GL support is in evaluate if we can merge this with FlutterView.
21-
@implementation FlutterOverlayView
23+
@implementation FlutterOverlayView {
24+
fml::CFRef<CGColorSpaceRef> _colorSpaceRef;
25+
}
2226

2327
- (instancetype)initWithFrame:(CGRect)frame {
2428
NSAssert(NO, @"FlutterOverlayView must init or initWithContentsScale");
@@ -42,15 +46,24 @@ - (instancetype)init {
4246
return self;
4347
}
4448

45-
- (instancetype)initWithContentsScale:(CGFloat)contentsScale {
49+
- (instancetype)initWithContentsScale:(CGFloat)contentsScale
50+
pixelFormat:(MTLPixelFormat)pixelFormat {
4651
self = [self init];
4752

4853
if ([self.layer isKindOfClass:NSClassFromString(@"CAMetalLayer")]) {
4954
self.layer.allowsGroupOpacity = NO;
5055
self.layer.contentsScale = contentsScale;
5156
self.layer.rasterizationScale = contentsScale;
57+
#pragma clang diagnostic push
58+
#pragma clang diagnostic ignored "-Wunguarded-availability-new"
59+
CAMetalLayer* layer = (CAMetalLayer*)self.layer;
60+
#pragma clang diagnostic pop
61+
layer.pixelFormat = pixelFormat;
62+
if (pixelFormat == MTLPixelFormatRGBA16Float) {
63+
self->_colorSpaceRef = fml::CFRef(CGColorSpaceCreateWithName(kCGColorSpaceExtendedSRGB));
64+
layer.colorspace = self->_colorSpaceRef;
65+
}
5266
}
53-
5467
return self;
5568
}
5669

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
#include <Metal/Metal.h>
56
#import <UIKit/UIGestureRecognizerSubclass.h>
67

78
#include <list>
@@ -14,6 +15,7 @@
1415
#import "flutter/shell/platform/darwin/common/framework/Headers/FlutterChannels.h"
1516
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterOverlayView.h"
1617
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h"
18+
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterView.h"
1719
#import "flutter/shell/platform/darwin/ios/framework/Source/FlutterViewController_Internal.h"
1820
#import "flutter/shell/platform/darwin/ios/ios_surface.h"
1921

@@ -85,13 +87,15 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
8587

8688
std::shared_ptr<FlutterPlatformViewLayer> FlutterPlatformViewLayerPool::GetLayer(
8789
GrDirectContext* gr_context,
88-
const std::shared_ptr<IOSContext>& ios_context) {
90+
const std::shared_ptr<IOSContext>& ios_context,
91+
MTLPixelFormat pixel_format) {
8992
if (available_layer_index_ >= layers_.size()) {
9093
std::shared_ptr<FlutterPlatformViewLayer> layer;
9194
fml::scoped_nsobject<UIView> overlay_view;
9295
fml::scoped_nsobject<UIView> overlay_view_wrapper;
9396

94-
if (!gr_context) {
97+
bool impeller_enabled = !!ios_context->GetImpellerContext();
98+
if (!gr_context && !impeller_enabled) {
9599
overlay_view.reset([[FlutterOverlayView alloc] init]);
96100
overlay_view_wrapper.reset([[FlutterOverlayView alloc] init]);
97101

@@ -104,8 +108,10 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
104108
std::move(surface));
105109
} else {
106110
CGFloat screenScale = [UIScreen mainScreen].scale;
107-
overlay_view.reset([[FlutterOverlayView alloc] initWithContentsScale:screenScale]);
108-
overlay_view_wrapper.reset([[FlutterOverlayView alloc] initWithContentsScale:screenScale]);
111+
overlay_view.reset([[FlutterOverlayView alloc] initWithContentsScale:screenScale
112+
pixelFormat:pixel_format]);
113+
overlay_view_wrapper.reset([[FlutterOverlayView alloc] initWithContentsScale:screenScale
114+
pixelFormat:pixel_format]);
109115

110116
auto ca_layer = fml::scoped_nsobject<CALayer>{[[overlay_view.get() layer] retain]};
111117
std::unique_ptr<IOSSurface> ios_surface = IOSSurface::Create(ios_context, ca_layer);
@@ -735,13 +741,15 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
735741
// on the overlay layer.
736742
background_canvas->ClipRect(joined_rect, DlCanvas::ClipOp::kDifference);
737743
// Get a new host layer.
738-
std::shared_ptr<FlutterPlatformViewLayer> layer = GetLayer(gr_context, //
739-
ios_context, //
740-
slice, //
741-
joined_rect, //
742-
current_platform_view_id, //
743-
overlay_id //
744-
);
744+
std::shared_ptr<FlutterPlatformViewLayer> layer =
745+
GetLayer(gr_context, //
746+
ios_context, //
747+
slice, //
748+
joined_rect, //
749+
current_platform_view_id, //
750+
overlay_id, //
751+
((FlutterView*)flutter_view_.get()).pixelFormat //
752+
);
745753
did_submit &= layer->did_submit_last_frame;
746754
platform_view_layers[current_platform_view_id].push_back(layer);
747755
overlay_id++;
@@ -811,9 +819,11 @@ static bool ClipRRectContainsPlatformViewBoundingRect(const SkRRect& clip_rrect,
811819
EmbedderViewSlice* slice,
812820
SkRect rect,
813821
int64_t view_id,
814-
int64_t overlay_id) {
822+
int64_t overlay_id,
823+
MTLPixelFormat pixel_format) {
815824
FML_DCHECK(flutter_view_);
816-
std::shared_ptr<FlutterPlatformViewLayer> layer = layer_pool_->GetLayer(gr_context, ios_context);
825+
std::shared_ptr<FlutterPlatformViewLayer> layer =
826+
layer_pool_->GetLayer(gr_context, ios_context, pixel_format);
817827

818828
UIView* overlay_view_wrapper = layer->overlay_view_wrapper.get();
819829
auto screenScale = [UIScreen mainScreen].scale;

shell/platform/darwin/ios/framework/Source/FlutterPlatformViews_Internal.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_FLUTTERPLATFORMVIEWS_INTERNAL_H_
66
#define FLUTTER_SHELL_PLATFORM_DARWIN_IOS_FRAMEWORK_SOURCE_FLUTTERPLATFORMVIEWS_INTERNAL_H_
77

8+
#include <Metal/Metal.h>
89
#include "flutter/flow/embedded_views.h"
910
#include "flutter/fml/platform/darwin/scoped_nsobject.h"
1011
#include "flutter/shell/common/shell.h"
@@ -167,9 +168,9 @@ class FlutterPlatformViewLayerPool {
167168

168169
// Gets a layer from the pool if available, or allocates a new one.
169170
// Finally, it marks the layer as used. That is, it increments `available_layer_index_`.
170-
std::shared_ptr<FlutterPlatformViewLayer> GetLayer(
171-
GrDirectContext* gr_context,
172-
const std::shared_ptr<IOSContext>& ios_context);
171+
std::shared_ptr<FlutterPlatformViewLayer> GetLayer(GrDirectContext* gr_context,
172+
const std::shared_ptr<IOSContext>& ios_context,
173+
MTLPixelFormat pixel_format);
173174

174175
// Gets the layers in the pool that aren't currently used.
175176
// This method doesn't mark the layers as unused.
@@ -320,7 +321,8 @@ class FlutterPlatformViewsController {
320321
EmbedderViewSlice* slice,
321322
SkRect rect,
322323
int64_t view_id,
323-
int64_t overlay_id);
324+
int64_t overlay_id,
325+
MTLPixelFormat pixel_format);
324326
// Removes overlay views and platform views that aren't needed in the current frame.
325327
// Must run on the platform thread.
326328
void RemoveUnusedLayers();

shell/platform/darwin/ios/framework/Source/FlutterView.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_FLUTTER_VIEW_H_
66
#define SHELL_PLATFORM_IOS_FRAMEWORK_SOURCE_FLUTTER_VIEW_H_
77

8+
#include <Metal/Metal.h>
89
#import <UIKit/UIKit.h>
910

1011
#include <memory>
@@ -47,6 +48,7 @@
4748
enableWideGamut:(BOOL)isWideGamutEnabled NS_DESIGNATED_INITIALIZER;
4849

4950
- (UIScreen*)screen;
51+
- (MTLPixelFormat)pixelFormat;
5052

5153
// Set by FlutterEngine or FlutterViewController to override software rendering.
5254
@property(class, nonatomic) BOOL forceSoftwareRendering;

0 commit comments

Comments
 (0)