From 70bac08754929b1208220d06f05fab88410c6677 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Tue, 16 Apr 2024 19:55:22 +0200 Subject: [PATCH 1/4] fix: Fix Metal NativeBuffer flickering issue --- .../ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h | 14 +++++- .../ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm | 44 ++++++++++++------- .../ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm | 12 ++--- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h index 7610d8ca94..6adb4fe1a8 100644 --- a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h +++ b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h @@ -16,6 +16,16 @@ #import #pragma clang diagnostic pop +class TextureHolder { +public: + explicit TextureHolder(CVMetalTextureRef texture); + ~TextureHolder(); + + GrBackendTexture toGrBackendTexture(); +private: + CVMetalTextureRef _texture; +}; + class SkiaCVPixelBufferUtils { public: enum class CVPixelBufferBaseFormat { rgb }; @@ -37,13 +47,13 @@ class SkiaCVPixelBufferUtils { /** Gets a GPU-backed Skia Texture for the given RGB CVPixelBuffer. */ - static GrBackendTexture + static TextureHolder* getSkiaTextureForCVPixelBuffer(CVPixelBufferRef pixelBuffer); }; private: static CVMetalTextureCacheRef getTextureCache(); - static GrBackendTexture + static TextureHolder* getSkiaTextureForCVPixelBufferPlane(CVPixelBufferRef pixelBuffer, size_t planeIndex); static MTLPixelFormat diff --git a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm index 15e7309e72..676fb66bf1 100644 --- a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm +++ b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm @@ -29,6 +29,30 @@ } #endif +// pragma MARK: TextureHolder + +TextureHolder::TextureHolder(CVMetalTextureRef texture): _texture(texture) { } +TextureHolder::~TextureHolder() { + CFRelease(_texture); +} + +GrBackendTexture TextureHolder::toGrBackendTexture() { + // Unwrap the underlying MTLTexture + id mtlTexture = CVMetalTextureGetTexture(_texture); + if (mtlTexture == nil) [[unlikely]] { + throw std::runtime_error( + "Failed to get MTLTexture from CVMetalTextureRef!"); + } + + // Wrap MTLTexture in Skia's GrBackendTexture + GrMtlTextureInfo textureInfo; + textureInfo.fTexture.retain((__bridge void *)mtlTexture); + GrBackendTexture texture = + GrBackendTexture((int)mtlTexture.width, (int)mtlTexture.height, + skgpu::Mipmapped::kNo, textureInfo); + return texture; +} + // pragma MARK: Base SkiaCVPixelBufferUtils::CVPixelBufferBaseFormat @@ -67,14 +91,14 @@ } } -GrBackendTexture SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer( +TextureHolder* SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer( CVPixelBufferRef pixelBuffer) { return getSkiaTextureForCVPixelBufferPlane(pixelBuffer, /* planeIndex */ 0); } // pragma MARK: CVPixelBuffer -> Skia Texture -GrBackendTexture SkiaCVPixelBufferUtils::getSkiaTextureForCVPixelBufferPlane( +TextureHolder* SkiaCVPixelBufferUtils::getSkiaTextureForCVPixelBufferPlane( CVPixelBufferRef pixelBuffer, size_t planeIndex) { // 1. Get cache CVMetalTextureCacheRef textureCache = getTextureCache(); @@ -94,21 +118,7 @@ std::to_string(result)); } - // 2. Unwrap the underlying MTLTexture - id mtlTexture = CVMetalTextureGetTexture(textureHolder); - if (mtlTexture == nil) [[unlikely]] { - throw std::runtime_error( - "Failed to get MTLTexture from CVMetalTextureRef!"); - } - - // 3. Wrap MTLTexture in Skia's GrBackendTexture - GrMtlTextureInfo textureInfo; - textureInfo.fTexture.retain((__bridge void *)mtlTexture); - GrBackendTexture texture = - GrBackendTexture((int)mtlTexture.width, (int)mtlTexture.height, - skgpu::Mipmapped::kNo, textureInfo); - CFRelease(textureHolder); - return texture; + return new TextureHolder(textureHolder); } // pragma MARK: getTextureCache() diff --git a/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm b/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm index 00e12e97f8..c67edb4639 100644 --- a/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm +++ b/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm @@ -128,12 +128,12 @@ // CVPixelBuffer is in any RGB format. SkColorType colorType = SkiaCVPixelBufferUtils::RGB::getCVPixelBufferColorType(pixelBuffer); - GrBackendTexture texture = - SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer( - pixelBuffer); - return SkImages::AdoptTextureFrom(context.skContext.get(), texture, - kTopLeft_GrSurfaceOrigin, colorType, - kOpaque_SkAlphaType); + TextureHolder* texture = SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer(pixelBuffer); + return SkImages::BorrowTextureFrom(context.skContext.get(), texture->toGrBackendTexture(), + kTopLeft_GrSurfaceOrigin, colorType, + kOpaque_SkAlphaType, nullptr, [](void* texture) { + delete (TextureHolder*)texture; + }, (void*)texture); } default: [[unlikely]] { From 9bce84004991db5f90b5e45e5e27360ddab28435 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Tue, 16 Apr 2024 19:59:38 +0200 Subject: [PATCH 2/4] chore: Format --- package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h | 7 ++++--- package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm | 14 +++++++------- package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm | 14 ++++++++------ 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h index 6adb4fe1a8..bfbb845d38 100644 --- a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h +++ b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h @@ -20,8 +20,9 @@ class TextureHolder { public: explicit TextureHolder(CVMetalTextureRef texture); ~TextureHolder(); - + GrBackendTexture toGrBackendTexture(); + private: CVMetalTextureRef _texture; }; @@ -47,13 +48,13 @@ class SkiaCVPixelBufferUtils { /** Gets a GPU-backed Skia Texture for the given RGB CVPixelBuffer. */ - static TextureHolder* + static TextureHolder * getSkiaTextureForCVPixelBuffer(CVPixelBufferRef pixelBuffer); }; private: static CVMetalTextureCacheRef getTextureCache(); - static TextureHolder* + static TextureHolder * getSkiaTextureForCVPixelBufferPlane(CVPixelBufferRef pixelBuffer, size_t planeIndex); static MTLPixelFormat diff --git a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm index 676fb66bf1..5fd62ee942 100644 --- a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm +++ b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm @@ -31,10 +31,8 @@ // pragma MARK: TextureHolder -TextureHolder::TextureHolder(CVMetalTextureRef texture): _texture(texture) { } -TextureHolder::~TextureHolder() { - CFRelease(_texture); -} +TextureHolder::TextureHolder(CVMetalTextureRef texture) : _texture(texture) {} +TextureHolder::~TextureHolder() { CFRelease(_texture); } GrBackendTexture TextureHolder::toGrBackendTexture() { // Unwrap the underlying MTLTexture @@ -91,27 +89,29 @@ } } -TextureHolder* SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer( +TextureHolder *SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer( CVPixelBufferRef pixelBuffer) { return getSkiaTextureForCVPixelBufferPlane(pixelBuffer, /* planeIndex */ 0); } // pragma MARK: CVPixelBuffer -> Skia Texture -TextureHolder* SkiaCVPixelBufferUtils::getSkiaTextureForCVPixelBufferPlane( +TextureHolder *SkiaCVPixelBufferUtils::getSkiaTextureForCVPixelBufferPlane( CVPixelBufferRef pixelBuffer, size_t planeIndex) { // 1. Get cache CVMetalTextureCacheRef textureCache = getTextureCache(); // 2. Get MetalTexture from CMSampleBuffer - CVMetalTextureRef textureHolder; size_t width = CVPixelBufferGetWidthOfPlane(pixelBuffer, planeIndex); size_t height = CVPixelBufferGetHeightOfPlane(pixelBuffer, planeIndex); MTLPixelFormat pixelFormat = getMTLPixelFormatForCVPixelBufferPlane(pixelBuffer, planeIndex); + + CVMetalTextureRef textureHolder; CVReturn result = CVMetalTextureCacheCreateTextureFromImage( kCFAllocatorDefault, textureCache, pixelBuffer, nil, pixelFormat, width, height, planeIndex, &textureHolder); + if (result != kCVReturnSuccess) [[unlikely]] { throw std::runtime_error( "Failed to create Metal Texture from CMSampleBuffer! Result: " + diff --git a/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm b/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm index c67edb4639..180e6d46a5 100644 --- a/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm +++ b/package/ios/RNSkia-iOS/SkiaMetalSurfaceFactory.mm @@ -128,12 +128,14 @@ // CVPixelBuffer is in any RGB format. SkColorType colorType = SkiaCVPixelBufferUtils::RGB::getCVPixelBufferColorType(pixelBuffer); - TextureHolder* texture = SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer(pixelBuffer); - return SkImages::BorrowTextureFrom(context.skContext.get(), texture->toGrBackendTexture(), - kTopLeft_GrSurfaceOrigin, colorType, - kOpaque_SkAlphaType, nullptr, [](void* texture) { - delete (TextureHolder*)texture; - }, (void*)texture); + TextureHolder *texture = + SkiaCVPixelBufferUtils::RGB::getSkiaTextureForCVPixelBuffer( + pixelBuffer); + return SkImages::BorrowTextureFrom( + context.skContext.get(), texture->toGrBackendTexture(), + kTopLeft_GrSurfaceOrigin, colorType, kOpaque_SkAlphaType, nullptr, + [](void *texture) { delete (TextureHolder *)texture; }, + (void *)texture); } default: [[unlikely]] { From 9d4e75147e735cc63c4620ce1e9ac0c17b0b3413 Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Tue, 16 Apr 2024 20:03:11 +0200 Subject: [PATCH 3/4] chore: Add comments --- .../ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h index bfbb845d38..191b6be452 100644 --- a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h +++ b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.h @@ -16,11 +16,32 @@ #import #pragma clang diagnostic pop +/** + Holds a Metal Texture. + When the `TextureHolder` is destroyed, the underlying Metal Texture + is marked as invalid and might be overwritten by the Texture Cache, + so make sure to delete the `TextureHolder` only when the `SkImage` + has been deleted. + + For example, use the `releaseProc` parameter in `BorrowImageFromTexture` + to delete the `TextureHolder`. + */ class TextureHolder { public: + /** + Create a new instance of TextureHolder which holds + the given `CVMetalTextureRef`. + + The given `CVMetalTextureRef` is assumed to already be + retained with `CFRetain`, and will later be manually + released with `CFRelease`. + */ explicit TextureHolder(CVMetalTextureRef texture); ~TextureHolder(); + /** + Converts this Texture to a Skia GrBackendTexture. + */ GrBackendTexture toGrBackendTexture(); private: From f361a98ddcf0014604491dea5d446132229c5afb Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Tue, 16 Apr 2024 20:42:50 +0200 Subject: [PATCH 4/4] fix: Don't make texture cache `thread_local` --- package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm index 5fd62ee942..5dc306bfdf 100644 --- a/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm +++ b/package/ios/RNSkia-iOS/SkiaCVPixelBufferUtils.mm @@ -124,7 +124,7 @@ // pragma MARK: getTextureCache() CVMetalTextureCacheRef SkiaCVPixelBufferUtils::getTextureCache() { - static thread_local CVMetalTextureCacheRef textureCache = nil; + static CVMetalTextureCacheRef textureCache = nil; if (textureCache == nil) { // Create a new Texture Cache auto result = CVMetalTextureCacheCreate(kCFAllocatorDefault, nil,