From 38e0f7c152f25208d0f5bf1ea16e315a2fb4eec0 Mon Sep 17 00:00:00 2001 From: William Candillon Date: Tue, 27 May 2025 09:53:16 +0200 Subject: [PATCH 1/2] Support for 16 bits textures --- apps/example/ios/Podfile.lock | 2 +- .../cpp/rnskia-android/OpenGLContext.h | 32 ++++- .../RNSkAndroidPlatformContext.h | 11 +- packages/skia/apple/MetalContext.h | 36 +++++- .../skia/apple/RNSkApplePlatformContext.h | 1 + .../skia/apple/RNSkApplePlatformContext.mm | 12 +- packages/skia/cpp/api/JsiSkSurfaceFactory.h | 12 +- .../skia/cpp/rnskia/RNSkPlatformContext.h | 9 ++ .../renderer/__tests__/e2e/Offscreen.spec.tsx | 117 ++++++++++++++++++ .../src/skia/types/Surface/SurfaceFactory.ts | 4 +- .../skia/src/skia/web/JsiSkSurfaceFactory.ts | 29 ++++- 11 files changed, 243 insertions(+), 22 deletions(-) diff --git a/apps/example/ios/Podfile.lock b/apps/example/ios/Podfile.lock index 510c3e7673..2ae035ebe5 100644 --- a/apps/example/ios/Podfile.lock +++ b/apps/example/ios/Podfile.lock @@ -2193,7 +2193,7 @@ SPEC CHECKSUMS: React-Mapbuffer: 3c11cee7737609275c7b66bd0b1de475f094cedf React-microtasksnativemodule: 843f352b32aacbe13a9c750190d34df44c3e6c2c react-native-safe-area-context: 0f14bce545abcdfbff79ce2e3c78c109f0be283e - react-native-skia: cde44b3bdb773204750a79d01ee74b85480287b1 + react-native-skia: 8ea98bfb08c756d04fa37803fcc4ff049d47ab42 react-native-slider: bb7eb4732940fab78217e1c096bb647d8b0d1cf3 React-NativeModulesApple: 88433b6946778bea9c153e27b671de15411bf225 React-perflogger: 9e8d3c0dc0194eb932162812a168aa5dc662f418 diff --git a/packages/skia/android/cpp/rnskia-android/OpenGLContext.h b/packages/skia/android/cpp/rnskia-android/OpenGLContext.h index a8ca8b7434..0a4a33ad60 100644 --- a/packages/skia/android/cpp/rnskia-android/OpenGLContext.h +++ b/packages/skia/android/cpp/rnskia-android/OpenGLContext.h @@ -58,9 +58,7 @@ class OpenGLContext { return instance; } - sk_sp MakeOffscreen(int width, int height) { - auto colorType = kRGBA_8888_SkColorType; - + sk_sp MakeOffscreen(int width, int height, SkColorType colorType) { SkSurfaceProps props(0, kUnknown_SkPixelGeometry); auto result = _glContext->makeCurrent(_glSurface.get()); @@ -68,9 +66,9 @@ class OpenGLContext { return nullptr; } - // Create texture - auto GL_RGBA8 = 0x8058; - auto format = GrBackendFormats::MakeGL(GL_RGBA8, GL_TEXTURE_2D); + // Create texture with appropriate format based on colorType + GrGLenum glInternalFormat = skColorTypeToGLFormat(colorType); + auto format = GrBackendFormats::MakeGL(glInternalFormat, GL_TEXTURE_2D); auto texture = _directContext->createBackendTexture( width, height, format, SkColors::kTransparent, skgpu::Mipmapped::kNo, GrRenderable::kYes); @@ -181,6 +179,28 @@ class OpenGLContext { std::unique_ptr _glSurface; sk_sp _directContext; + GrGLenum skColorTypeToGLFormat(SkColorType colorType) { + switch (colorType) { + case kRGBA_8888_SkColorType: + return 0x8058; // GL_RGBA8 + case kBGRA_8888_SkColorType: + return 0x8058; // GL_RGBA8 (will handle swizzling in shader) + case kRGB_565_SkColorType: + return 0x8D62; // GL_RGB565 + case kARGB_4444_SkColorType: + return 0x8033; // GL_RGBA4 + case kRGBA_F16_SkColorType: + case kRGBA_F16Norm_SkColorType: + return 0x881A; // GL_RGBA16F + case kGray_8_SkColorType: + return 0x8229; // GL_R8 + case kRGBA_1010102_SkColorType: + return 0x8059; // GL_RGB10_A2 + default: + return 0x8058; // GL_RGBA8 fallback + } + } + OpenGLContext() { auto display = OpenGLSharedContext::getInstance().getDisplay(); auto sharedContext = OpenGLSharedContext::getInstance().getContext(); diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h b/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h index 4793dc447d..ddde26d519 100644 --- a/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h +++ b/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h @@ -49,14 +49,19 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext { _jniPlatformContext->raiseError(err); } - sk_sp makeOffscreenSurface(int width, int height) override { + sk_sp makeOffscreenSurface(int width, int height, SkColorType colorType) override { #if defined(SK_GRAPHITE) - return DawnContext::getInstance().MakeOffscreen(width, height); + return DawnContext::getInstance().MakeOffscreen(width, height, colorType); #else - return OpenGLContext::getInstance().MakeOffscreen(width, height); + return OpenGLContext::getInstance().MakeOffscreen(width, height, colorType); #endif } + sk_sp makeOffscreenSurface(int width, int height) override { + // Android/OpenGL default is RGBA_8888 + return makeOffscreenSurface(width, height, kRGBA_8888_SkColorType); + } + std::shared_ptr makeContextFromNativeSurface(void *surface, int width, int height) override { #if defined(SK_GRAPHITE) diff --git a/packages/skia/apple/MetalContext.h b/packages/skia/apple/MetalContext.h index 04485c29a7..0b8821de77 100644 --- a/packages/skia/apple/MetalContext.h +++ b/packages/skia/apple/MetalContext.h @@ -43,10 +43,13 @@ struct OffscreenRenderContext { OffscreenRenderContext(id device, sk_sp skiaContext, id commandQueue, int width, - int height) { + int height, SkColorType colorType) { + // Convert SkColorType to Metal pixel format + MTLPixelFormat pixelFormat = skColorTypeToMTLPixelFormat(colorType); + // Create a Metal texture descriptor MTLTextureDescriptor *textureDescriptor = [MTLTextureDescriptor - texture2DDescriptorWithPixelFormat:MTLPixelFormatBGRA8Unorm + texture2DDescriptorWithPixelFormat:pixelFormat width:width height:height mipmapped:NO]; @@ -54,6 +57,29 @@ struct OffscreenRenderContext { MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead; texture = [device newTextureWithDescriptor:textureDescriptor]; } + +private: + MTLPixelFormat skColorTypeToMTLPixelFormat(SkColorType colorType) { + switch (colorType) { + case kRGBA_8888_SkColorType: + return MTLPixelFormatRGBA8Unorm; + case kBGRA_8888_SkColorType: + return MTLPixelFormatBGRA8Unorm; + case kRGB_565_SkColorType: + return MTLPixelFormatB5G6R5Unorm; + case kARGB_4444_SkColorType: + return MTLPixelFormatABGR4Unorm; + case kRGBA_F16_SkColorType: + case kRGBA_F16Norm_SkColorType: + return MTLPixelFormatRGBA16Float; + case kGray_8_SkColorType: + return MTLPixelFormatR8Unorm; + case kRGBA_1010102_SkColorType: + return MTLPixelFormatRGB10A2Unorm; + default: + return MTLPixelFormatBGRA8Unorm; // fallback to default + } + } }; class MetalContext { @@ -67,10 +93,10 @@ class MetalContext { return instance; } - sk_sp MakeOffscreen(int width, int height) { + sk_sp MakeOffscreen(int width, int height, SkColorType colorType) { auto device = MetalSharedContext::getInstance().getDevice(); auto ctx = new OffscreenRenderContext(device, _directContext, _commandQueue, - width, height); + width, height, colorType); // Create a GrBackendTexture from the Metal texture GrMtlTextureInfo info; @@ -81,7 +107,7 @@ class MetalContext { // Create a SkSurface from the GrBackendTexture auto surface = SkSurfaces::WrapBackendTexture( _directContext.get(), backendTexture, kTopLeft_GrSurfaceOrigin, 0, - kBGRA_8888_SkColorType, nullptr, nullptr, + colorType, nullptr, nullptr, [](void *addr) { delete (OffscreenRenderContext *)addr; }, ctx); return surface; diff --git a/packages/skia/apple/RNSkApplePlatformContext.h b/packages/skia/apple/RNSkApplePlatformContext.h index f1c6f5a83c..3cd7eb4ab1 100644 --- a/packages/skia/apple/RNSkApplePlatformContext.h +++ b/packages/skia/apple/RNSkApplePlatformContext.h @@ -69,6 +69,7 @@ class RNSkApplePlatformContext : public RNSkPlatformContext { const std::function)> &op) override; void raiseError(const std::exception &err) override; + sk_sp makeOffscreenSurface(int width, int height, SkColorType colorType) override; sk_sp makeOffscreenSurface(int width, int height) override; sk_sp createFontMgr() override; diff --git a/packages/skia/apple/RNSkApplePlatformContext.mm b/packages/skia/apple/RNSkApplePlatformContext.mm index ec66aa9d3c..ba0cea3475 100644 --- a/packages/skia/apple/RNSkApplePlatformContext.mm +++ b/packages/skia/apple/RNSkApplePlatformContext.mm @@ -238,14 +238,20 @@ } sk_sp RNSkApplePlatformContext::makeOffscreenSurface(int width, - int height) { + int height, SkColorType colorType) { #if defined(SK_GRAPHITE) - return DawnContext::getInstance().MakeOffscreen(width, height); + return DawnContext::getInstance().MakeOffscreen(width, height, colorType); #else - return MetalContext::getInstance().MakeOffscreen(width, height); + return MetalContext::getInstance().MakeOffscreen(width, height, colorType); #endif } +sk_sp RNSkApplePlatformContext::makeOffscreenSurface(int width, + int height) { + // Apple/Metal default is BGRA_8888 + return makeOffscreenSurface(width, height, kBGRA_8888_SkColorType); +} + sk_sp RNSkApplePlatformContext::makeImageFromNativeBuffer(void *buffer) { #if defined(SK_GRAPHITE) diff --git a/packages/skia/cpp/api/JsiSkSurfaceFactory.h b/packages/skia/cpp/api/JsiSkSurfaceFactory.h index 66d356b833..8eb167f77d 100644 --- a/packages/skia/cpp/api/JsiSkSurfaceFactory.h +++ b/packages/skia/cpp/api/JsiSkSurfaceFactory.h @@ -39,7 +39,17 @@ class JsiSkSurfaceFactory : public JsiSkHostObject { auto width = static_cast(arguments[0].asNumber()); auto height = static_cast(arguments[1].asNumber()); auto context = getContext(); - auto surface = context->makeOffscreenSurface(width, height); + + sk_sp surface; + if (count > 2 && !arguments[2].isUndefined()) { + auto colorTypeValue = static_cast(arguments[2].asNumber()); + auto colorType = static_cast(colorTypeValue); + surface = context->makeOffscreenSurface(width, height, colorType); + } else { + // Use platform-specific default color type + surface = context->makeOffscreenSurface(width, height); + } + if (surface == nullptr) { return jsi::Value::null(); } diff --git a/packages/skia/cpp/rnskia/RNSkPlatformContext.h b/packages/skia/cpp/rnskia/RNSkPlatformContext.h index dd2cc18860..a09b142d03 100644 --- a/packages/skia/cpp/rnskia/RNSkPlatformContext.h +++ b/packages/skia/cpp/rnskia/RNSkPlatformContext.h @@ -89,6 +89,15 @@ class RNSkPlatformContext { * Creates an offscreen surface * @param width Width of the offscreen surface * @param height Height of the offscreen surface + * @param colorType Color type for the surface + * @return sk_sp + */ + virtual sk_sp makeOffscreenSurface(int width, int height, SkColorType colorType) = 0; + + /** + * Creates an offscreen surface with platform-specific default color type + * @param width Width of the offscreen surface + * @param height Height of the offscreen surface * @return sk_sp */ virtual sk_sp makeOffscreenSurface(int width, int height) = 0; diff --git a/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx index 8737a29c76..ee108ae793 100644 --- a/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx +++ b/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx @@ -3,6 +3,7 @@ import React from "react"; import { checkImage, docPath } from "../../../__tests__/setup"; import { Circle } from "../../components"; import { surface, importSkia } from "../setup"; +import { ColorType } from "../../../skia/types"; describe("Offscreen Drawings", () => { it("Should use the canvas API to build an image", async () => { @@ -106,4 +107,120 @@ describe("Offscreen Drawings", () => { ); checkImage(image, docPath("offscreen/circle.png")); }); + + it("Should support 16-bit texture formats", async () => { + const result = await surface.eval( + (Skia, ctx) => { + // Create a small 2x2 surface with 16-bit float format + const offscreen16bit = Skia.Surface.MakeOffscreen(2, 2, ctx.colorType); + if (!offscreen16bit) { + throw new Error("Could not create 16-bit offscreen surface"); + } + + const canvas = offscreen16bit.getCanvas(); + const paint = Skia.Paint(); + + // Draw with a high dynamic range color value (> 1.0) + // This tests if 16-bit float format can handle values beyond [0,1] range + paint.setColor(Float32Array.of(2.0, 0.5, 0.25, 1.0)); + canvas.drawRect(Skia.XYWHRect(0, 0, 1, 1), paint); + + paint.setColor(Float32Array.of(0.125, 1.5, 0.75, 1.0)); + canvas.drawRect(Skia.XYWHRect(1, 0, 1, 1), paint); + + paint.setColor(Float32Array.of(0.75, 0.25, 3.0, 1.0)); + canvas.drawRect(Skia.XYWHRect(0, 1, 1, 1), paint); + + paint.setColor(Float32Array.of(1.25, 2.5, 0.5, 1.0)); + canvas.drawRect(Skia.XYWHRect(1, 1, 1, 1), paint); + + offscreen16bit.flush(); + + // Read pixels to verify we can handle 16-bit data + const image = offscreen16bit.makeImageSnapshot(); + const pixelData = image.readPixels(); + + // Verify we got some pixel data back + if (!pixelData) { + throw new Error("Failed to read pixels from 16-bit surface"); + } + + // For 16-bit surfaces, we expect different behavior than 8-bit + // The pixel data length should match the format + const expectedLength = 2 * 2 * 4; // 2x2 pixels, 4 channels (RGBA) + + return { + success: true, + pixelDataLength: pixelData.length, + expectedLength, + // For verification, return first few pixel values + firstPixels: Array.from(pixelData.slice(0, 16)), + colorType: ctx.colorType, + surfaceWidth: image.width(), + surfaceHeight: image.height(), + }; + }, + { colorType: ColorType.RGBA_F16 } + ); + + // Verify the test completed successfully + expect(result.success).toBe(true); + expect(result.surfaceWidth).toBe(2); + expect(result.surfaceHeight).toBe(2); + expect(result.pixelDataLength).toBeGreaterThan(0); + expect(result.colorType).toBe(ColorType.RGBA_F16); + }); + + it("Should use platform-specific default color types", async () => { + const result = await surface.eval( + (Skia, ctx) => { + // Create surface without specifying color type (uses platform default) + const defaultSurface = Skia.Surface.MakeOffscreen(2, 2); + if (!defaultSurface) { + throw new Error("Could not create default offscreen surface"); + } + + const canvas = defaultSurface.getCanvas(); + const paint = Skia.Paint(); + paint.setColor(Skia.Color("red")); + canvas.drawRect(Skia.XYWHRect(0, 0, 2, 2), paint); + defaultSurface.flush(); + + // Create surface with explicit RGBA_8888 (ColorType enum value 4) + const rgba8888Surface = Skia.Surface.MakeOffscreen( + 2, + 2, + ctx.rgba8888ColorType + ); + if (!rgba8888Surface) { + throw new Error("Could not create RGBA_8888 offscreen surface"); + } + + const canvas2 = rgba8888Surface.getCanvas(); + const paint2 = Skia.Paint(); + paint2.setColor(Skia.Color("red")); + canvas2.drawRect(Skia.XYWHRect(0, 0, 2, 2), paint2); + rgba8888Surface.flush(); + + // Both should work and produce similar results + const defaultImage = defaultSurface.makeImageSnapshot(); + const rgba8888Image = rgba8888Surface.makeImageSnapshot(); + + return { + success: true, + defaultImageWidth: defaultImage.width(), + defaultImageHeight: defaultImage.height(), + rgba8888ImageWidth: rgba8888Image.width(), + rgba8888ImageHeight: rgba8888Image.height(), + }; + }, + { rgba8888ColorType: ColorType.RGBA_8888 } + ); + + expect(result.success).toBe(true); + expect(result.defaultImageWidth).toBe(2); + expect(result.defaultImageHeight).toBe(2); + expect(result.rgba8888ImageWidth).toBe(2); + expect(result.rgba8888ImageHeight).toBe(2); + }); }); diff --git a/packages/skia/src/skia/types/Surface/SurfaceFactory.ts b/packages/skia/src/skia/types/Surface/SurfaceFactory.ts index 688d738404..27c1f684ae 100644 --- a/packages/skia/src/skia/types/Surface/SurfaceFactory.ts +++ b/packages/skia/src/skia/types/Surface/SurfaceFactory.ts @@ -1,4 +1,5 @@ import type { SkSurface } from "./Surface"; +import type { ColorType } from "../Image/ColorType"; export interface SurfaceFactory { /** @@ -14,6 +15,7 @@ export interface SurfaceFactory { * Creates a GPU backed surface. * @param width - number of pixels of the width of the drawable area. * @param height - number of pixels of the height of the drawable area. + * @param colorType - color type for the surface (optional, defaults to RGBA_8888) */ - MakeOffscreen: (width: number, height: number) => SkSurface | null; + MakeOffscreen: (width: number, height: number, colorType?: ColorType) => SkSurface | null; } diff --git a/packages/skia/src/skia/web/JsiSkSurfaceFactory.ts b/packages/skia/src/skia/web/JsiSkSurfaceFactory.ts index 80ac0dd642..35cbcda5f7 100644 --- a/packages/skia/src/skia/web/JsiSkSurfaceFactory.ts +++ b/packages/skia/src/skia/web/JsiSkSurfaceFactory.ts @@ -1,6 +1,6 @@ import type { CanvasKit, Surface } from "canvaskit-wasm"; -import type { SurfaceFactory } from "../types"; +import type { ColorType, SurfaceFactory } from "../types"; import { Host } from "./Host"; import { JsiSkSurface } from "./JsiSkSurface"; @@ -17,7 +17,7 @@ export class JsiSkSurfaceFactory extends Host implements SurfaceFactory { ); } - MakeOffscreen(width: number, height: number) { + MakeOffscreen(width: number, height: number, _colorType?: ColorType) { // OffscreenCanvas may be unvailable in some environments. // eslint-disable-next-line @typescript-eslint/no-explicit-any const OC = (globalThis as any).OffscreenCanvas; @@ -31,11 +31,36 @@ export class JsiSkSurfaceFactory extends Host implements SurfaceFactory { if (!grContext) { throw new Error("Could not make a graphics context"); } + + // TODO: if a 16bit color type is provided, we can work around it to provide it via CanvasKit + surface = this.CanvasKit.MakeRenderTarget(grContext, width, height); + // Note: CanvasKit doesn't directly support specifying ColorType in MakeRenderTarget + // This is a limitation of the web implementation } if (!surface) { return null; } return new JsiSkSurface(this.CanvasKit, surface); } + + // On Web at the moment we only support RGBA_8888 or RGBA_F16 + // private convertColorType(colorType: ColorType) { + // switch (colorType) { + // case ColorType.RGBA_8888: + // return this.CanvasKit.ColorType.RGBA_8888; + // case ColorType.BGRA_8888: + // return this.CanvasKit.ColorType.BGRA_8888; + // case ColorType.RGB_565: + // return this.CanvasKit.ColorType.RGB_565; + // case ColorType.RGBA_F16: + // return this.CanvasKit.ColorType.RGBA_F16; + // case ColorType.Gray_8: + // return this.CanvasKit.ColorType.Gray_8; + // case ColorType.RGBA_1010102: + // return this.CanvasKit.ColorType.RGBA_1010102; + // default: + // return this.CanvasKit.ColorType.RGBA_8888; + // } + // } } From a25cf9cfadefa62f3eb152523a6875dda443a4ea Mon Sep 17 00:00:00 2001 From: William Candillon Date: Tue, 27 May 2025 17:08:20 +0200 Subject: [PATCH 2/2] :wrench: --- .../cpp/rnskia-android/OpenGLContext.h | 34 +++++++------- .../RNSkAndroidPlatformContext.h | 3 +- packages/skia/apple/MetalContext.h | 46 +++++++++---------- packages/skia/apple/MetalWindowContext.mm | 6 ++- .../skia/apple/RNSkApplePlatformContext.h | 3 +- .../skia/apple/RNSkApplePlatformContext.mm | 7 +-- packages/skia/cpp/api/JsiSkImage.h | 5 +- packages/skia/cpp/api/JsiSkImageFactory.h | 4 +- packages/skia/cpp/api/JsiSkSurface.h | 3 +- packages/skia/cpp/api/JsiSkSurfaceFactory.h | 4 +- .../skia/cpp/rnskia/RNSkPlatformContext.h | 3 +- packages/skia/cpp/utils/RNSkTypedArray.h | 18 ++++++-- .../renderer/__tests__/e2e/Offscreen.spec.tsx | 46 +++++++++++++++++-- 13 files changed, 119 insertions(+), 63 deletions(-) diff --git a/packages/skia/android/cpp/rnskia-android/OpenGLContext.h b/packages/skia/android/cpp/rnskia-android/OpenGLContext.h index 0a4a33ad60..c810269a6d 100644 --- a/packages/skia/android/cpp/rnskia-android/OpenGLContext.h +++ b/packages/skia/android/cpp/rnskia-android/OpenGLContext.h @@ -181,23 +181,23 @@ class OpenGLContext { GrGLenum skColorTypeToGLFormat(SkColorType colorType) { switch (colorType) { - case kRGBA_8888_SkColorType: - return 0x8058; // GL_RGBA8 - case kBGRA_8888_SkColorType: - return 0x8058; // GL_RGBA8 (will handle swizzling in shader) - case kRGB_565_SkColorType: - return 0x8D62; // GL_RGB565 - case kARGB_4444_SkColorType: - return 0x8033; // GL_RGBA4 - case kRGBA_F16_SkColorType: - case kRGBA_F16Norm_SkColorType: - return 0x881A; // GL_RGBA16F - case kGray_8_SkColorType: - return 0x8229; // GL_R8 - case kRGBA_1010102_SkColorType: - return 0x8059; // GL_RGB10_A2 - default: - return 0x8058; // GL_RGBA8 fallback + case kRGBA_8888_SkColorType: + return 0x8058; // GL_RGBA8 + case kBGRA_8888_SkColorType: + return 0x8058; // GL_RGBA8 (will handle swizzling in shader) + case kRGB_565_SkColorType: + return 0x8D62; // GL_RGB565 + case kARGB_4444_SkColorType: + return 0x8033; // GL_RGBA4 + case kRGBA_F16_SkColorType: + case kRGBA_F16Norm_SkColorType: + return 0x881A; // GL_RGBA16F + case kGray_8_SkColorType: + return 0x8229; // GL_R8 + case kRGBA_1010102_SkColorType: + return 0x8059; // GL_RGB10_A2 + default: + return 0x8058; // GL_RGBA8 fallback } } diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h b/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h index ddde26d519..13c1d7e01f 100644 --- a/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h +++ b/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h @@ -49,7 +49,8 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext { _jniPlatformContext->raiseError(err); } - sk_sp makeOffscreenSurface(int width, int height, SkColorType colorType) override { + sk_sp makeOffscreenSurface(int width, int height, + SkColorType colorType) override { #if defined(SK_GRAPHITE) return DawnContext::getInstance().MakeOffscreen(width, height, colorType); #else diff --git a/packages/skia/apple/MetalContext.h b/packages/skia/apple/MetalContext.h index 0b8821de77..77da8bfae3 100644 --- a/packages/skia/apple/MetalContext.h +++ b/packages/skia/apple/MetalContext.h @@ -46,13 +46,13 @@ struct OffscreenRenderContext { int height, SkColorType colorType) { // Convert SkColorType to Metal pixel format MTLPixelFormat pixelFormat = skColorTypeToMTLPixelFormat(colorType); - + // Create a Metal texture descriptor - MTLTextureDescriptor *textureDescriptor = [MTLTextureDescriptor - texture2DDescriptorWithPixelFormat:pixelFormat - width:width - height:height - mipmapped:NO]; + MTLTextureDescriptor *textureDescriptor = + [MTLTextureDescriptor texture2DDescriptorWithPixelFormat:pixelFormat + width:width + height:height + mipmapped:NO]; textureDescriptor.usage = MTLTextureUsageRenderTarget | MTLTextureUsageShaderRead; texture = [device newTextureWithDescriptor:textureDescriptor]; @@ -61,23 +61,23 @@ struct OffscreenRenderContext { private: MTLPixelFormat skColorTypeToMTLPixelFormat(SkColorType colorType) { switch (colorType) { - case kRGBA_8888_SkColorType: - return MTLPixelFormatRGBA8Unorm; - case kBGRA_8888_SkColorType: - return MTLPixelFormatBGRA8Unorm; - case kRGB_565_SkColorType: - return MTLPixelFormatB5G6R5Unorm; - case kARGB_4444_SkColorType: - return MTLPixelFormatABGR4Unorm; - case kRGBA_F16_SkColorType: - case kRGBA_F16Norm_SkColorType: - return MTLPixelFormatRGBA16Float; - case kGray_8_SkColorType: - return MTLPixelFormatR8Unorm; - case kRGBA_1010102_SkColorType: - return MTLPixelFormatRGB10A2Unorm; - default: - return MTLPixelFormatBGRA8Unorm; // fallback to default + case kRGBA_8888_SkColorType: + return MTLPixelFormatRGBA8Unorm; + case kBGRA_8888_SkColorType: + return MTLPixelFormatBGRA8Unorm; + case kRGB_565_SkColorType: + return MTLPixelFormatB5G6R5Unorm; + case kARGB_4444_SkColorType: + return MTLPixelFormatABGR4Unorm; + case kRGBA_F16_SkColorType: + case kRGBA_F16Norm_SkColorType: + return MTLPixelFormatRGBA16Float; + case kGray_8_SkColorType: + return MTLPixelFormatR8Unorm; + case kRGBA_1010102_SkColorType: + return MTLPixelFormatRGB10A2Unorm; + default: + return MTLPixelFormatBGRA8Unorm; // fallback to default } } }; diff --git a/packages/skia/apple/MetalWindowContext.mm b/packages/skia/apple/MetalWindowContext.mm index 8f8b597903..c1fc02142b 100644 --- a/packages/skia/apple/MetalWindowContext.mm +++ b/packages/skia/apple/MetalWindowContext.mm @@ -25,10 +25,12 @@ _layer.drawableSize = CGSizeMake(width, height); BOOL supportsWideColor = NO; if (@available(iOS 10.0, *)) { - supportsWideColor = [UIScreen mainScreen].traitCollection.displayGamut == UIDisplayGamutP3; + supportsWideColor = + [UIScreen mainScreen].traitCollection.displayGamut == UIDisplayGamutP3; } if (supportsWideColor) { - CGColorSpaceRef colorSpace = CGColorSpaceCreateWithName(kCGColorSpaceDisplayP3); + CGColorSpaceRef colorSpace = + CGColorSpaceCreateWithName(kCGColorSpaceDisplayP3); _layer.colorspace = colorSpace; CGColorSpaceRelease(colorSpace); } diff --git a/packages/skia/apple/RNSkApplePlatformContext.h b/packages/skia/apple/RNSkApplePlatformContext.h index 3cd7eb4ab1..b194857623 100644 --- a/packages/skia/apple/RNSkApplePlatformContext.h +++ b/packages/skia/apple/RNSkApplePlatformContext.h @@ -69,7 +69,8 @@ class RNSkApplePlatformContext : public RNSkPlatformContext { const std::function)> &op) override; void raiseError(const std::exception &err) override; - sk_sp makeOffscreenSurface(int width, int height, SkColorType colorType) override; + sk_sp makeOffscreenSurface(int width, int height, + SkColorType colorType) override; sk_sp makeOffscreenSurface(int width, int height) override; sk_sp createFontMgr() override; diff --git a/packages/skia/apple/RNSkApplePlatformContext.mm b/packages/skia/apple/RNSkApplePlatformContext.mm index ba0cea3475..3d52bb7a2e 100644 --- a/packages/skia/apple/RNSkApplePlatformContext.mm +++ b/packages/skia/apple/RNSkApplePlatformContext.mm @@ -190,7 +190,7 @@ if (!GrBackendTextures::GetMtlTextureInfo(texture, &textureInfo)) { throw std::runtime_error("Couldn't get Metal texture info"); } - result.mtlTexture = textureInfo.fTexture.get(); + result.mtlTexture = textureInfo.fTexture.get(); return result; } @@ -237,8 +237,9 @@ RCTFatal(RCTErrorWithMessage([NSString stringWithUTF8String:err.what()])); } -sk_sp RNSkApplePlatformContext::makeOffscreenSurface(int width, - int height, SkColorType colorType) { +sk_sp +RNSkApplePlatformContext::makeOffscreenSurface(int width, int height, + SkColorType colorType) { #if defined(SK_GRAPHITE) return DawnContext::getInstance().MakeOffscreen(width, height, colorType); #else diff --git a/packages/skia/cpp/api/JsiSkImage.h b/packages/skia/cpp/api/JsiSkImage.h index 8c949b3b22..95a9048840 100644 --- a/packages/skia/cpp/api/JsiSkImage.h +++ b/packages/skia/cpp/api/JsiSkImage.h @@ -196,8 +196,9 @@ class JsiSkImage : public JsiSkWrappingSkPtrHostObject { SkImageInfo info = (count > 2 && !arguments[2].isUndefined()) ? *JsiSkImageInfo::fromValue(runtime, arguments[2]) - : SkImageInfo::MakeN32(getObject()->width(), getObject()->height(), - getObject()->imageInfo().alphaType()); + : SkImageInfo::Make(getObject()->width(), getObject()->height(), + getObject()->imageInfo().colorType(), + getObject()->imageInfo().alphaType()); size_t bytesPerRow = 0; if (count > 4 && !arguments[4].isUndefined()) { bytesPerRow = static_cast(arguments[4].asNumber()); diff --git a/packages/skia/cpp/api/JsiSkImageFactory.h b/packages/skia/cpp/api/JsiSkImageFactory.h index 7cc3d3acb0..12c525db4f 100644 --- a/packages/skia/cpp/api/JsiSkImageFactory.h +++ b/packages/skia/cpp/api/JsiSkImageFactory.h @@ -93,8 +93,8 @@ class JsiSkImageFactory : public JsiSkHostObject { } if (count > 4 && arguments[4].isObject() && arguments[4].asObject(runtime).isHostObject(runtime)) { - auto jsiImage = arguments[4].asObject(runtime).asHostObject( - runtime); + auto jsiImage = + arguments[4].asObject(runtime).asHostObject(runtime); jsiImage->setObject(image); return jsi::Value(runtime, arguments[4]); } diff --git a/packages/skia/cpp/api/JsiSkSurface.h b/packages/skia/cpp/api/JsiSkSurface.h index 2dbe27e8fc..43bbdf5572 100644 --- a/packages/skia/cpp/api/JsiSkSurface.h +++ b/packages/skia/cpp/api/JsiSkSurface.h @@ -76,7 +76,8 @@ class JsiSkSurface : public JsiSkWrappingSkPtrHostObject { DawnContext::getInstance().submitRecording(recording.get()); #endif if (count > 1 && arguments[1].isObject()) { - auto jsiImage = arguments[1].asObject(runtime).asHostObject(runtime); + auto jsiImage = + arguments[1].asObject(runtime).asHostObject(runtime); jsiImage->setObject(image); return jsi::Value(runtime, arguments[1]); } diff --git a/packages/skia/cpp/api/JsiSkSurfaceFactory.h b/packages/skia/cpp/api/JsiSkSurfaceFactory.h index 8eb167f77d..8a6c5229d5 100644 --- a/packages/skia/cpp/api/JsiSkSurfaceFactory.h +++ b/packages/skia/cpp/api/JsiSkSurfaceFactory.h @@ -39,7 +39,7 @@ class JsiSkSurfaceFactory : public JsiSkHostObject { auto width = static_cast(arguments[0].asNumber()); auto height = static_cast(arguments[1].asNumber()); auto context = getContext(); - + sk_sp surface; if (count > 2 && !arguments[2].isUndefined()) { auto colorTypeValue = static_cast(arguments[2].asNumber()); @@ -49,7 +49,7 @@ class JsiSkSurfaceFactory : public JsiSkHostObject { // Use platform-specific default color type surface = context->makeOffscreenSurface(width, height); } - + if (surface == nullptr) { return jsi::Value::null(); } diff --git a/packages/skia/cpp/rnskia/RNSkPlatformContext.h b/packages/skia/cpp/rnskia/RNSkPlatformContext.h index a09b142d03..3d4df45444 100644 --- a/packages/skia/cpp/rnskia/RNSkPlatformContext.h +++ b/packages/skia/cpp/rnskia/RNSkPlatformContext.h @@ -92,7 +92,8 @@ class RNSkPlatformContext { * @param colorType Color type for the surface * @return sk_sp */ - virtual sk_sp makeOffscreenSurface(int width, int height, SkColorType colorType) = 0; + virtual sk_sp makeOffscreenSurface(int width, int height, + SkColorType colorType) = 0; /** * Creates an offscreen surface with platform-specific default color type diff --git a/packages/skia/cpp/utils/RNSkTypedArray.h b/packages/skia/cpp/utils/RNSkTypedArray.h index 5a0f57e1ba..b739f9a9fe 100644 --- a/packages/skia/cpp/utils/RNSkTypedArray.h +++ b/packages/skia/cpp/utils/RNSkTypedArray.h @@ -21,16 +21,24 @@ class RNSkTypedArray { return typedArray; } } else { - if (info.colorType() == kRGBA_F32_SkColorType) { + if (info.colorType() == kAlpha_8_SkColorType || + info.colorType() == kRGBA_8888_SkColorType || + info.colorType() == kRGB_888x_SkColorType || + info.colorType() == kBGRA_8888_SkColorType || + info.colorType() == kGray_8_SkColorType || + info.colorType() == kR8G8_unorm_SkColorType || + info.colorType() == kSRGBA_8888_SkColorType || + info.colorType() == kR8_unorm_SkColorType) { + auto arrayCtor = - runtime.global().getPropertyAsFunction(runtime, "Float32Array"); + runtime.global().getPropertyAsFunction(runtime, "Uint8Array"); return arrayCtor.callAsConstructor(runtime, - static_cast(reqSize / 4)); + static_cast(reqSize)); } else { auto arrayCtor = - runtime.global().getPropertyAsFunction(runtime, "Uint8Array"); + runtime.global().getPropertyAsFunction(runtime, "Float32Array"); return arrayCtor.callAsConstructor(runtime, - static_cast(reqSize)); + static_cast(reqSize / 4)); } } } diff --git a/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx index ee108ae793..ac6d58560f 100644 --- a/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx +++ b/packages/skia/src/renderer/__tests__/e2e/Offscreen.spec.tsx @@ -146,16 +146,22 @@ describe("Offscreen Drawings", () => { } // For 16-bit surfaces, we expect different behavior than 8-bit - // The pixel data length should match the format + // The pixel data should be a Float32Array for 16-bit formats + const isFloat32Array = pixelData instanceof Float32Array; + const isUint8Array = pixelData instanceof Uint8Array; const expectedLength = 2 * 2 * 4; // 2x2 pixels, 4 channels (RGBA) return { success: true, pixelDataLength: pixelData.length, expectedLength, + isFloat32Array, + isUint8Array, + arrayConstructorName: pixelData.constructor.name, // For verification, return first few pixel values firstPixels: Array.from(pixelData.slice(0, 16)), - colorType: ctx.colorType, + actualColorType: image.getImageInfo().colorType, + expectedColorType: ctx.colorType, surfaceWidth: image.width(), surfaceHeight: image.height(), }; @@ -168,7 +174,13 @@ describe("Offscreen Drawings", () => { expect(result.surfaceWidth).toBe(2); expect(result.surfaceHeight).toBe(2); expect(result.pixelDataLength).toBeGreaterThan(0); - expect(result.colorType).toBe(ColorType.RGBA_F16); + expect(result.actualColorType).toBe(ColorType.RGBA_F16); + expect(result.actualColorType).toBe(result.expectedColorType); + + // Verify that 16-bit formats return Float32Array instead of Uint8Array + expect(result.isFloat32Array).toBe(true); + expect(result.isUint8Array).toBe(false); + expect(result.arrayConstructorName).toBe("Float32Array"); }); it("Should use platform-specific default color types", async () => { @@ -206,12 +218,26 @@ describe("Offscreen Drawings", () => { const defaultImage = defaultSurface.makeImageSnapshot(); const rgba8888Image = rgba8888Surface.makeImageSnapshot(); + // Test pixel data types for 8-bit vs 16-bit formats + const rgba8888PixelData = rgba8888Image.readPixels(); + const isRgba8888Uint8Array = rgba8888PixelData instanceof Uint8Array; + const isRgba8888Float32Array = + rgba8888PixelData instanceof Float32Array; + return { success: true, defaultImageWidth: defaultImage.width(), defaultImageHeight: defaultImage.height(), rgba8888ImageWidth: rgba8888Image.width(), rgba8888ImageHeight: rgba8888Image.height(), + defaultImageColorType: defaultImage.getImageInfo().colorType, + rgba8888ImageColorType: rgba8888Image.getImageInfo().colorType, + // Verify 8-bit format returns Uint8Array + isRgba8888Uint8Array, + isRgba8888Float32Array, + rgba8888ArrayConstructorName: rgba8888PixelData + ? rgba8888PixelData.constructor.name + : null, }; }, { rgba8888ColorType: ColorType.RGBA_8888 } @@ -222,5 +248,19 @@ describe("Offscreen Drawings", () => { expect(result.defaultImageHeight).toBe(2); expect(result.rgba8888ImageWidth).toBe(2); expect(result.rgba8888ImageHeight).toBe(2); + + // Verify that the explicit RGBA_8888 surface has the correct color type + expect(result.defaultImageColorType).toBe( + surface.OS === "ios" ? ColorType.BGRA_8888 : ColorType.RGBA_8888 + ); + + // Verify that the default surface has a valid color type (platform-specific) + expect(result.defaultImageColorType).toBeDefined(); + expect(typeof result.defaultImageColorType).toBe("number"); + + // Verify that 8-bit formats return Uint8Array (not Float32Array) + expect(result.isRgba8888Uint8Array).toBe(true); + expect(result.isRgba8888Float32Array).toBe(false); + expect(result.rgba8888ArrayConstructorName).toBe("Uint8Array"); }); });