From fda3a967b07ae91a50db805b2eadf337383969ec Mon Sep 17 00:00:00 2001 From: William Candillon Date: Thu, 9 Jul 2026 10:20:53 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(=F0=9F=8C=8E):=20apply=20the=20paragrap?= =?UTF-8?q?h=20style's=20default=20textStyle=20(#3928)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On web, JsiSkParagraphStyle.toParagraphStyle only ever passed { color: BLACK } as the textStyle, silently dropping the default textStyle provided to ParagraphBuilder.Make (fontFamilies, fontSize, etc.), so text laid out with the CanvasKit defaults (fontSize 14) unless a style was pushed explicitly. The native implementation honors it, so web behaved differently from iOS/Android. Merge the defined properties of the converted textStyle over the black-color default (the ParagraphStyle constructor requires a color). Also includes the node version upgrade from main (#3927). --- .../__tests__/e2e/ParagraphMethods.spec.tsx | 49 +++++++++++++++++++ .../skia/src/skia/web/JsiSkParagraphStyle.ts | 18 ++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/packages/skia/src/renderer/__tests__/e2e/ParagraphMethods.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/ParagraphMethods.spec.tsx index 833ee3971e..03df1274da 100644 --- a/packages/skia/src/renderer/__tests__/e2e/ParagraphMethods.spec.tsx +++ b/packages/skia/src/renderer/__tests__/e2e/ParagraphMethods.spec.tsx @@ -10,6 +10,55 @@ const RobotoRegular = Array.from( ); describe("Paragraph Methods", () => { + describe("paragraph style", () => { + it("should apply the default textStyle from the paragraph style", async () => { + const heights = await surface.eval( + (Skia, ctx) => { + const robotoRegular = Skia.Typeface.MakeFreeTypeFaceFromData( + Skia.Data.fromBytes(new Uint8Array(ctx.RobotoRegular)) + )!; + const provider = Skia.TypefaceFontProvider.Make(); + provider.registerFont(robotoRegular, "Roboto"); + + const textStyle = { + color: Skia.Color("black"), + fontFamilies: ["Roboto"], + fontSize: 24, + }; + + // The default textStyle set on the paragraph style should produce + // the same layout as the same style pushed explicitly. + const fromParagraphStyle = Skia.ParagraphBuilder.Make( + { textStyle }, + provider + ) + .addText("Hello") + .build(); + fromParagraphStyle.layout(512); + + const fromPushStyle = Skia.ParagraphBuilder.Make({}, provider) + .pushStyle(textStyle) + .addText("Hello") + .build(); + fromPushStyle.layout(512); + + return { + fromParagraphStyle: fromParagraphStyle.getLineMetrics()[0].height, + fromPushStyle: fromPushStyle.getLineMetrics()[0].height, + }; + }, + { + RobotoRegular, + } + ); + + expect(heights.fromParagraphStyle).toBeCloseTo(heights.fromPushStyle, 3); + // Roboto at fontSize 24 has a line height of ~28; the default fontSize + // (14) would yield ~16.4. + expect(heights.fromParagraphStyle).toBeGreaterThan(24); + }); + }); + describe("getRectsForPlaceholders", () => { it("should handle multiple placeholders with different alignments", async () => { const placeholderRects = await surface.eval( diff --git a/packages/skia/src/skia/web/JsiSkParagraphStyle.ts b/packages/skia/src/skia/web/JsiSkParagraphStyle.ts index 274e3a2752..f05308e00a 100644 --- a/packages/skia/src/skia/web/JsiSkParagraphStyle.ts +++ b/packages/skia/src/skia/web/JsiSkParagraphStyle.ts @@ -1,8 +1,10 @@ -import type { CanvasKit, ParagraphStyle } from "canvaskit-wasm"; +import type { CanvasKit, ParagraphStyle, TextStyle } from "canvaskit-wasm"; import { TextDirection } from "../types"; import type { SkParagraphStyle } from "../types"; +import { JsiSkTextStyle } from "./JsiSkTextStyle"; + export class JsiSkParagraphStyle { static toParagraphStyle( ck: CanvasKit, @@ -10,7 +12,19 @@ export class JsiSkParagraphStyle { ): ParagraphStyle { // Seems like we need to provide the textStyle.color value, otherwise // the constructor crashes. - const ps = new ck.ParagraphStyle({ textStyle: { color: ck.BLACK } }); + const textStyle: TextStyle = { color: ck.BLACK }; + if (value.textStyle) { + // Only merge the properties that are set; toTextStyle() emits undefined + // for the others and the ParagraphStyle constructor requires a color. + Object.entries(JsiSkTextStyle.toTextStyle(value.textStyle)).forEach( + ([key, v]) => { + if (v !== undefined) { + (textStyle as Record)[key] = v; + } + } + ); + } + const ps = new ck.ParagraphStyle({ textStyle }); ps.disableHinting = value.disableHinting ?? ps.disableHinting; ps.ellipsis = value.ellipsis ?? ps.ellipsis; From 368169ddbf37a5bd5db7df48a2c9e7175c91b6a0 Mon Sep 17 00:00:00 2001 From: William Candillon Date: Thu, 9 Jul 2026 14:46:00 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(=F0=9F=8C=90):=20improve=20WebGL=20cont?= =?UTF-8?q?ext=20management=20in=20SkiaPictureView=20(#3929)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release WebGL contexts and GrDirectContexts on web Canvas unmount, reuse them on relayout, and unregister view handles from SkiaViewApi so unmounted views no longer retain their detached canvas elements. Drop pictures set after unmount and guard snapshot calls on unregistered views. --- .../skia/src/specs/NativeSkiaModule.web.ts | 46 ++++- .../skia/src/views/SkiaPictureView.web.tsx | 166 ++++++++++++++---- 2 files changed, 168 insertions(+), 44 deletions(-) diff --git a/packages/skia/src/specs/NativeSkiaModule.web.ts b/packages/skia/src/specs/NativeSkiaModule.web.ts index bd06158c3d..fe470d540a 100644 --- a/packages/skia/src/specs/NativeSkiaModule.web.ts +++ b/packages/skia/src/specs/NativeSkiaModule.web.ts @@ -6,29 +6,45 @@ import type { SkiaPictureViewHandle } from "../views/SkiaPictureView.web"; export type ISkiaViewApiWeb = ISkiaViewApi & { views: Record; deferedPictures: Record; + unregisteredViews: Set; registerView(nativeId: string, view: SkiaPictureViewHandle): void; + unregisterView(nativeId: string): void; }; global.SkiaViewApi = { views: {}, deferedPictures: {}, + unregisteredViews: new Set(), deferedOnSize: {}, web: true, registerView(nativeId: string, view: SkiaPictureViewHandle) { + this.unregisteredViews.delete(nativeId); // Maybe a picture for this view was already set if (this.deferedPictures[nativeId]) { view.setPicture(this.deferedPictures[nativeId] as SkPicture); + delete this.deferedPictures[nativeId]; } this.views[nativeId] = view; }, + unregisterView(nativeId: string) { + // Views must be removed on unmount: the handle's closures capture the + // canvas element, so a stale entry retains the whole detached DOM tree. + this.unregisteredViews.add(nativeId); + delete this.views[nativeId]; + delete this.deferedPictures[nativeId]; + }, // eslint-disable-next-line @typescript-eslint/no-explicit-any setJsiProperty(nativeId: number, name: string, value: any) { if (name === "picture") { - if (!this.views[`${nativeId}`]) { - this.deferedPictures[`${nativeId}`] = value; - } else { - this.views[`${nativeId}`].setPicture(value); + const id = `${nativeId}`; + if (this.views[id]) { + this.views[id].setPicture(value); + } else if (!this.unregisteredViews.has(id)) { + this.deferedPictures[id] = value; } + // Otherwise the view has unmounted (e.g. a trailing animation frame): + // drop the picture instead of deferring it for an id that will never + // register again, which would retain it forever. } }, size(nativeId: number) { @@ -39,14 +55,30 @@ global.SkiaViewApi = { } }, requestRedraw(nativeId: number) { - this.views[`${nativeId}`].redraw(); + // The view may already have unmounted (e.g. a trailing animation frame). + this.views[`${nativeId}`]?.redraw(); }, makeImageSnapshot(nativeId: number, rect?: SkRect) { - return this.views[`${nativeId}`].makeImageSnapshot(rect); + const view = this.views[`${nativeId}`]; + if (!view) { + throw new Error( + `Cannot make image snapshot: view with nativeID ${nativeId} is not registered (it may have unmounted)` + ); + } + return view.makeImageSnapshot(rect); }, makeImageSnapshotAsync(nativeId: number, rect?: SkRect) { return new Promise((resolve, reject) => { - const result = this.views[`${nativeId}`].makeImageSnapshot(rect); + const view = this.views[`${nativeId}`]; + if (!view) { + reject( + new Error( + `Cannot make image snapshot: view with nativeID ${nativeId} is not registered (it may have unmounted)` + ) + ); + return; + } + const result = view.makeImageSnapshot(rect); if (result) { resolve(result); } else { diff --git a/packages/skia/src/views/SkiaPictureView.web.tsx b/packages/skia/src/views/SkiaPictureView.web.tsx index 919762adb2..c4582f1f0d 100644 --- a/packages/skia/src/views/SkiaPictureView.web.tsx +++ b/packages/skia/src/views/SkiaPictureView.web.tsx @@ -6,6 +6,7 @@ import React, { useImperativeHandle, } from "react"; import type { LayoutChangeEvent } from "react-native"; +import type { GrDirectContext, WebGLContextHandle } from "canvaskit-wasm"; import type { SkRect, SkPicture, SkImage } from "../skia/types"; import { JsiSkSurface } from "../skia/web/JsiSkSurface"; @@ -36,11 +37,27 @@ interface Renderer { class WebGLRenderer implements Renderer { private surface: JsiSkSurface | null = null; + private grContext: GrDirectContext | null = null; + private contextHandle: WebGLContextHandle = 0; constructor( private canvas: HTMLCanvasElement, private pd: number ) { + this.contextHandle = CanvasKit.GetWebGLContext(canvas); + if (!this.contextHandle) { + throw new Error("Could not create a WebGL context"); + } + this.grContext = CanvasKit.MakeWebGLContext(this.contextHandle); + if (!this.grContext) { + CanvasKit.deleteContext(this.contextHandle); + this.contextHandle = 0; + throw new Error("Could not create a graphics context"); + } + const ctx = canvas.getContext("webgl2"); + if (ctx) { + ctx.drawingBufferColorSpace = "display-p3"; + } this.onResize(); } @@ -57,13 +74,21 @@ class WebGLRenderer implements Renderer { onResize() { const { canvas, pd } = this; + if (!this.grContext) { + return; + } canvas.width = canvas.clientWidth * pd; canvas.height = canvas.clientHeight * pd; - const surface = CanvasKit.MakeWebGLCanvasSurface(canvas); - const ctx = canvas.getContext("webgl2"); - if (ctx) { - ctx.drawingBufferColorSpace = "display-p3"; - } + this.surface?.ref.delete(); + this.surface = null; + // Reuse the existing WebGL context and GrDirectContext: only the surface + // needs to be recreated when the canvas is resized. + const surface = CanvasKit.MakeOnScreenGLSurface( + this.grContext, + canvas.width, + canvas.height, + CanvasKit.ColorSpace.SRGB + ); if (!surface) { throw new Error("Could not create surface"); } @@ -83,17 +108,39 @@ class WebGLRenderer implements Renderer { } dispose(): void { - if (this.surface) { - this.canvas - ?.getContext("webgl2") - ?.getExtension("WEBGL_lose_context") - ?.loseContext(); - this.surface.ref.delete(); - this.surface = null; + this.surface?.ref.delete(); + this.surface = null; + if (this.grContext) { + this.grContext.releaseResourcesAndAbandonContext(); + this.grContext.delete(); + this.grContext = null; + } + this.canvas + ?.getContext("webgl2") + ?.getExtension("WEBGL_lose_context") + ?.loseContext(); + if (this.contextHandle) { + // Unregister the context from CanvasKit's internal registry, otherwise + // it retains the canvas element (and its detached DOM tree) forever. + CanvasKit.deleteContext(this.contextHandle); + // Making the now-deleted handle current clears CanvasKit's + // current-context globals (GLctx/Module.ctx), which would otherwise + // keep referencing the context (and the canvas) until another surface + // becomes current. With a deleted handle this is a no-op that returns + // null without creating anything. + CanvasKit.MakeWebGLContext(this.contextHandle); + this.contextHandle = 0; } } } +interface TempRenderResult { + surface: JsiSkSurface; + tempCanvas: OffscreenCanvas; + grContext: GrDirectContext; + contextHandle: WebGLContextHandle; +} + class StaticWebGLRenderer implements Renderer { private cachedImage: SkImage | null = null; @@ -106,22 +153,35 @@ class StaticWebGLRenderer implements Renderer { this.cachedImage = null; } - private renderPictureToSurface( - picture: SkPicture - ): { surface: JsiSkSurface; tempCanvas: OffscreenCanvas } | null { + private renderPictureToSurface(picture: SkPicture): TempRenderResult | null { const tempCanvas = new OffscreenCanvas( this.canvas.clientWidth * this.pd, this.canvas.clientHeight * this.pd ); let surface: JsiSkSurface | null = null; + let grContext: GrDirectContext | null = null; + let contextHandle: WebGLContextHandle = 0; try { - const webglSurface = CanvasKit.MakeWebGLCanvasSurface(tempCanvas); + contextHandle = CanvasKit.GetWebGLContext(tempCanvas); + if (!contextHandle) { + throw new Error("Could not create a WebGL context"); + } + grContext = CanvasKit.MakeWebGLContext(contextHandle); + if (!grContext) { + throw new Error("Could not create a graphics context"); + } const ctx = tempCanvas.getContext("webgl2"); if (ctx) { ctx.drawingBufferColorSpace = "display-p3"; } + const webglSurface = CanvasKit.MakeOnScreenGLSurface( + grContext, + tempCanvas.width, + tempCanvas.height, + CanvasKit.ColorSpace.SRGB + ); if (!webglSurface) { throw new Error("Could not create WebGL surface"); @@ -137,23 +197,39 @@ class StaticWebGLRenderer implements Renderer { skiaCanvas.restore(); surface.ref.flush(); - return { surface, tempCanvas }; + return { surface, tempCanvas, grContext, contextHandle }; } catch (error) { - if (surface) { - surface.ref.delete(); - } - this.cleanupWebGLContext(tempCanvas); + this.cleanupRenderResult({ + surface, + tempCanvas, + grContext, + contextHandle, + }); return null; } } - private cleanupWebGLContext(tempCanvas: OffscreenCanvas): void { - const ctx = tempCanvas.getContext("webgl2"); - if (ctx) { - const loseContext = ctx.getExtension("WEBGL_lose_context"); - if (loseContext) { - loseContext.loseContext(); - } + private cleanupRenderResult(result: { + surface: JsiSkSurface | null; + tempCanvas: OffscreenCanvas; + grContext: GrDirectContext | null; + contextHandle: WebGLContextHandle; + }): void { + result.surface?.ref.delete(); + if (result.grContext) { + result.grContext.releaseResourcesAndAbandonContext(); + result.grContext.delete(); + } + result.tempCanvas + .getContext("webgl2") + ?.getExtension("WEBGL_lose_context") + ?.loseContext(); + if (result.contextHandle) { + // Unregister the context from CanvasKit's internal registry, otherwise + // it retains the OffscreenCanvas forever. + CanvasKit.deleteContext(result.contextHandle); + // Clear CanvasKit's current-context globals (see WebGLRenderer.dispose). + CanvasKit.MakeWebGLContext(result.contextHandle); } } @@ -165,6 +241,7 @@ class StaticWebGLRenderer implements Renderer { const { tempCanvas } = renderResult; const ctx2d = this.canvas.getContext("2d"); if (!ctx2d) { + this.cleanupRenderResult(renderResult); throw new Error("Could not get 2D context"); } @@ -185,7 +262,7 @@ class StaticWebGLRenderer implements Renderer { this.canvas.clientHeight * this.pd ); - this.cleanupWebGLContext(tempCanvas); + this.cleanupRenderResult(renderResult); } makeImageSnapshot(picture: SkPicture, rect?: SkRect): SkImage | null { @@ -195,15 +272,14 @@ class StaticWebGLRenderer implements Renderer { return null; } - const { surface, tempCanvas } = renderResult; - try { - this.cachedImage = surface.makeImageSnapshot(dp2Pixel(this.pd, rect)); + this.cachedImage = renderResult.surface.makeImageSnapshot( + dp2Pixel(this.pd, rect) + ); } catch (error) { console.error("Error creating image snapshot:", error); } finally { - surface.ref.delete(); - this.cleanupWebGLContext(tempCanvas); + this.cleanupRenderResult(renderResult); } } @@ -246,6 +322,7 @@ export const SkiaPictureView = (props: SkiaPictureViewProps) => { const { ref } = props; const canvasRef = useRef(null); const renderer = useRef(null); + const rendererIsStatic = useRef(null); const redrawRequestsRef = useRef(0); const requestIdRef = useRef(0); const pictureRef = useRef(null); @@ -342,10 +419,21 @@ export const SkiaPictureView = (props: SkiaPictureViewProps) => { (evt: LayoutChangeEvent) => { const canvas = canvasRef.current; if (canvas) { - renderer.current = - props.__destroyWebGLContextAfterRender === true + const destroyAfterRender = + props.__destroyWebGLContextAfterRender === true; + if ( + renderer.current === null || + rendererIsStatic.current !== destroyAfterRender + ) { + renderer.current?.dispose(); + renderer.current = destroyAfterRender ? new StaticWebGLRenderer(canvas, pd) : new WebGLRenderer(canvas, pd); + rendererIsStatic.current = destroyAfterRender; + } else { + // Reuse the existing renderer (and its WebGL context) on relayout. + renderer.current.onResize(); + } if (pictureRef.current) { renderer.current.draw(pictureRef.current); } @@ -375,7 +463,8 @@ export const SkiaPictureView = (props: SkiaPictureViewProps) => { useEffect(() => { const nativeID = props.nativeID ?? `${SkiaViewNativeId.current++}`; - (global.SkiaViewApi as ISkiaViewApiWeb).registerView(nativeID, { + const api = global.SkiaViewApi as ISkiaViewApiWeb; + api.registerView(nativeID, { setPicture, getSize, redraw, @@ -383,6 +472,9 @@ export const SkiaPictureView = (props: SkiaPictureViewProps) => { measure, measureInWindow, } as SkiaPictureViewHandle); + return () => { + api.unregisterView(nativeID); + }; }, [ setPicture, getSize,