From b2fe2de11d8186132017071a41211fbc61224942 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Wed, 24 Apr 2024 01:32:43 +0900 Subject: [PATCH 1/2] [darwin] Update pixel format handling in FlutterTexture CVPixelBuffers from the application can have arbitrary pixel formats. However, current implementation doesn't support all pixel formats. To address this, add a comment to the FlutterTexture class to clarify which pixel formats are supported. Also, reject unsupported pixel formats so that the application can handle them properly. Fixes #147242. --- .../common/framework/Headers/FlutterTexture.h | 9 +++- .../FlutterDarwinExternalTextureMetal.mm | 5 +- .../FlutterEmbedderExternalTextureTest.mm | 46 +++++++++++++++++++ .../Source/FlutterExternalTexture.mm | 5 +- 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/common/framework/Headers/FlutterTexture.h b/shell/platform/darwin/common/framework/Headers/FlutterTexture.h index 003b8650bb775..1fa7a0ef6c205 100644 --- a/shell/platform/darwin/common/framework/Headers/FlutterTexture.h +++ b/shell/platform/darwin/common/framework/Headers/FlutterTexture.h @@ -19,7 +19,14 @@ FLUTTER_DARWIN_EXPORT * See also: https://github.com/flutter/plugins/tree/master/packages/camera */ @protocol FlutterTexture -/** Copy the contents of the texture into a `CVPixelBuffer`. */ +/** + * Copy the contents of the texture into a `CVPixelBuffer`. + * + * The type of the pixel buffer is one of the following: + * - `kCVPixelFormatType_32BGRA` + * - `kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange` + * - `kCVPixelFormatType_420YpCbCr8BiPlanarFullRange` + */ - (CVPixelBufferRef _Nullable)copyPixelBuffer; /** diff --git a/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm b/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm index 514b7e67b5830..6dadde93ce4a6 100644 --- a/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm +++ b/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm @@ -133,8 +133,11 @@ - (void)onTextureUnregistered { if (_pixelFormat == kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange || _pixelFormat == kCVPixelFormatType_420YpCbCr8BiPlanarFullRange) { image = [self wrapNV12ExternalPixelBuffer:pixelBuffer context:context]; - } else { + } else if (_pixelFormat == kCVPixelFormatType_32BGRA) { image = [self wrapRGBAExternalPixelBuffer:pixelBuffer context:context]; + } else { + FML_LOG(ERROR) << "Unsupported pixel format: " << _pixelFormat; + return nullptr; } if (!image) { diff --git a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm index 47ba5ee32328a..75d1b16627fc8 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterEmbedderExternalTextureTest.mm @@ -288,4 +288,50 @@ - (CVPixelBufferRef)pixelBuffer { gpuSurface->makeImageSnapshot(); } +TEST_F(FlutterEmbedderExternalTextureTest, TestPopulateUnsupportedExternalTexture) { + // Constants. + const size_t width = 100; + const size_t height = 100; + const int64_t texture_id = 1; + + // Set up the surface. + FlutterDarwinContextMetalSkia* darwinContextMetal = + [[FlutterDarwinContextMetalSkia alloc] initWithDefaultMTLDevice]; + SkImageInfo info = SkImageInfo::MakeN32Premul(width, height); + GrDirectContext* grContext = darwinContextMetal.mainContext.get(); + sk_sp gpuSurface(SkSurfaces::RenderTarget(grContext, skgpu::Budgeted::kNo, info)); + + // Create a texture. + TestExternalTexture* testExternalTexture = + [[TestExternalTexture alloc] initWidth:width + height:height + pixelFormatType:kCVPixelFormatType_420YpCbCr8PlanarFullRange]; + FlutterExternalTexture* textureHolder = + [[FlutterExternalTexture alloc] initWithFlutterTexture:testExternalTexture + darwinMetalContext:darwinContextMetal]; + + // Callback to resolve the texture. + EmbedderExternalTextureMetal::ExternalTextureCallback callback = [&](int64_t texture_id, size_t w, + size_t h) { + EXPECT_TRUE(w == width); + EXPECT_TRUE(h == height); + + auto texture = std::make_unique(); + EXPECT_FALSE([textureHolder populateTexture:texture.get()]); + return nullptr; + }; + + // Render the texture. + std::unique_ptr texture = + std::make_unique(texture_id, callback); + SkRect bounds = SkRect::MakeWH(info.width(), info.height()); + DlImageSampling sampling = DlImageSampling::kNearestNeighbor; + DlSkCanvasAdapter canvas(gpuSurface->getCanvas()); + flutter::Texture::PaintContext context{ + .canvas = &canvas, + .gr_context = grContext, + }; + texture->Paint(context, bounds, /*freeze=*/false, sampling); +} + } // namespace flutter::testing diff --git a/shell/platform/darwin/macos/framework/Source/FlutterExternalTexture.mm b/shell/platform/darwin/macos/framework/Source/FlutterExternalTexture.mm index 0d95de6814f50..e15cd4e1e2d36 100644 --- a/shell/platform/darwin/macos/framework/Source/FlutterExternalTexture.mm +++ b/shell/platform/darwin/macos/framework/Source/FlutterExternalTexture.mm @@ -43,8 +43,11 @@ - (BOOL)populateTexture:(FlutterMetalExternalTexture*)textureOut { if (pixel_format == kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange || pixel_format == kCVPixelFormatType_420YpCbCr8BiPlanarFullRange) { return [self populateTextureFromYUVAPixelBuffer:pixelBuffer textureOut:textureOut]; - } else { + } else if (pixel_format == kCVPixelFormatType_32BGRA) { return [self populateTextureFromRGBAPixelBuffer:pixelBuffer textureOut:textureOut]; + } else { + NSLog(@"Unsupported pixel format: %d", pixel_format); + return NO; } } From 267b011c3977b96c05f80ccb9b8a716ad9ea4cd5 Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Wed, 24 Apr 2024 20:35:36 +0900 Subject: [PATCH 2/2] [darwin] Update pixel buffer wrapper method to use BGRA It was requested to rename during the review, so I did it. --- .../darwin/graphics/FlutterDarwinExternalTextureMetal.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm b/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm index 6dadde93ce4a6..578d9e9fd27db 100644 --- a/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm +++ b/shell/platform/darwin/graphics/FlutterDarwinExternalTextureMetal.mm @@ -134,7 +134,7 @@ - (void)onTextureUnregistered { _pixelFormat == kCVPixelFormatType_420YpCbCr8BiPlanarFullRange) { image = [self wrapNV12ExternalPixelBuffer:pixelBuffer context:context]; } else if (_pixelFormat == kCVPixelFormatType_32BGRA) { - image = [self wrapRGBAExternalPixelBuffer:pixelBuffer context:context]; + image = [self wrapBGRAExternalPixelBuffer:pixelBuffer context:context]; } else { FML_LOG(ERROR) << "Unsupported pixel format: " << _pixelFormat; return nullptr; @@ -238,7 +238,7 @@ - (void)onTextureUnregistered { return flutter::DlImage::Make(skImage); } -- (sk_sp)wrapRGBAExternalPixelBuffer:(CVPixelBufferRef)pixelBuffer +- (sk_sp)wrapBGRAExternalPixelBuffer:(CVPixelBufferRef)pixelBuffer context:(flutter::Texture::PaintContext&)context { SkISize textureSize = SkISize::Make(CVPixelBufferGetWidth(pixelBuffer), CVPixelBufferGetHeight(pixelBuffer));