From ccae9b0bbf929ce11cd15b46d897f06e29ad0408 Mon Sep 17 00:00:00 2001 From: William Candillon Date: Sun, 23 Aug 2026 19:13:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(=F0=9F=90=9B):=20fix=20paint=20AA=20default?= =?UTF-8?q?=20(#4024)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/skia/cpp/api/recorder/Paint.h | 8 +- .../skia/scripts/install-skia-graphite.ts | 10 +-- .../__tests__/e2e/InnerShadow.spec.tsx | 9 +-- .../src/renderer/__tests__/e2e/Paint.spec.tsx | 79 ++++++++++++++++++- .../skia/src/sksg/Recorder/DrawingContext.ts | 15 ++-- 5 files changed, 100 insertions(+), 21 deletions(-) diff --git a/packages/skia/cpp/api/recorder/Paint.h b/packages/skia/cpp/api/recorder/Paint.h index 5e3dec216a..0e2ba995c2 100644 --- a/packages/skia/cpp/api/recorder/Paint.h +++ b/packages/skia/cpp/api/recorder/Paint.h @@ -157,8 +157,12 @@ class SavePaintCmd : public Command { // Reset the paint that savePaint() just pushed instead of pushing a // second one: the matching RestorePaintDeclaration pops a single frame, // so an extra push would leave the enclosing group's opacity on the - // stack and leak it onto every sibling drawn afterwards. - ctx->getPaint() = SkPaint(); + // stack and leak it onto every sibling drawn afterwards. The fresh + // paint is anti-aliased to match the TS player, which resets with + // Skia.Paint(). + SkPaint freshPaint; + freshPaint.setAntiAlias(true); + ctx->getPaint() = freshPaint; } auto &paint = ctx->getPaint(); if (props.opacity.has_value()) { diff --git a/packages/skia/scripts/install-skia-graphite.ts b/packages/skia/scripts/install-skia-graphite.ts index d177a95b01..6d2b0264a0 100644 --- a/packages/skia/scripts/install-skia-graphite.ts +++ b/packages/skia/scripts/install-skia-graphite.ts @@ -325,7 +325,9 @@ const downloadDawnLibs = async (): Promise => { "libwebgpu_dawn.so" ); if (!existsSync(src)) { - throw new Error(`Missing libwebgpu_dawn.so for ${abi} in ${androidAsset}`); + throw new Error( + `Missing libwebgpu_dawn.so for ${abi} in ${androidAsset}` + ); } fileOps.cp(src, path.join(LIBS_DIR, "android", abi, "libwebgpu_dawn.so")); } @@ -381,8 +383,7 @@ const downloadAppleLibs = async (): Promise => { const extractedIosDir = path.join(iosTempDir, "ios"); if (existsSync(extractedIosDir)) { const xcframeworks = readdirSync(extractedIosDir).filter( - (f) => - f.endsWith(".xcframework") && f !== "libdawn_combined.xcframework" + (f) => f.endsWith(".xcframework") && f !== "libdawn_combined.xcframework" ); for (const xcf of xcframeworks) { fileOps.cp(path.join(extractedIosDir, xcf), path.join(iosDir, xcf)); @@ -402,8 +403,7 @@ const downloadAppleLibs = async (): Promise => { const extractedMacosDir = path.join(macosTempDir, "macos"); if (existsSync(extractedMacosDir)) { const xcframeworks = readdirSync(extractedMacosDir).filter( - (f) => - f.endsWith(".xcframework") && f !== "libdawn_combined.xcframework" + (f) => f.endsWith(".xcframework") && f !== "libdawn_combined.xcframework" ); for (const xcf of xcframeworks) { fileOps.cp(path.join(extractedMacosDir, xcf), path.join(macosDir, xcf)); diff --git a/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx index 2a2fd1e1c4..1ca6ba9fe8 100644 --- a/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx +++ b/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx @@ -75,14 +75,7 @@ describe("Inner shadow", () => { const image = await surface.draw( <> - + diff --git a/packages/skia/src/renderer/__tests__/e2e/Paint.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/Paint.spec.tsx index 44e145499a..e7e4d70780 100644 --- a/packages/skia/src/renderer/__tests__/e2e/Paint.spec.tsx +++ b/packages/skia/src/renderer/__tests__/e2e/Paint.spec.tsx @@ -1,17 +1,32 @@ import React from "react"; -import { surface, importSkia } from "../setup"; +import { surface, importSkia, PIXEL_RATIO } from "../setup"; import { + Blur, Circle, Fill, Group, LinearGradient, Paint, Path, + Rect, SweepGradient, } from "../../components"; import { checkImage, docPath } from "../../../__tests__/setup"; import { fitbox } from "../../components/shapes/FitBox"; +import { createDrawingContext } from "../../../sksg/Recorder/DrawingContext"; +import type { SkImage, SkPaint } from "../../../skia/types"; +import { AlphaType, ColorType } from "../../../skia/types"; + +const readPixel = (image: SkImage, x: number, y: number) => + Array.from( + image.readPixels(x, y, { + width: 1, + height: 1, + colorType: ColorType.RGBA_8888, + alphaType: AlphaType.Unpremul, + })! + ); const blendModes = [ "clear", @@ -82,6 +97,68 @@ describe("Paint", () => { ); checkImage(image, "snapshots/paint/circle.png"); }); + it("should not mutate a user-provided paint via child effects", async () => { + const { Skia } = importSkia(); + const { width, height } = surface; + const paint = Skia.Paint(); + paint.setColor(Skia.Color("red")); + const rect = { + x: width / 4, + y: height / 4, + width: width / 2, + height: height / 2, + }; + // 8 logical pixels to the left of the rectangle, on its vertical center. + const outside = { + x: (rect.x - 8) * PIXEL_RATIO, + y: (height / 2) * PIXEL_RATIO, + }; + const center = { + x: (width / 2) * PIXEL_RATIO, + y: (height / 2) * PIXEL_RATIO, + }; + const blurred = await surface.draw( + + + + ); + // Sanity check: the blur bleeds outside the rectangle bounds. + expect(readPixel(blurred, outside.x, outside.y)[3]).toBeGreaterThan(0); + // The blur was materialized on the paint stack, not on the user's paint: + // reusing the paint without children must render a sharp rectangle. + const image = await surface.draw(); + expect(readPixel(image, outside.x, outside.y)).toEqual([0, 0, 0, 0]); + expect(readPixel(image, center.x, center.y)).toEqual([255, 0, 0, 255]); + }); + it("should keep the base paint anti-aliased when the paint pool is reused", async () => { + const { Skia } = importSkia(); + const drawFrame = (paintPool: SkPaint[]) => { + const ckSurface = Skia.Surface.MakeOffscreen(64, 64)!; + const canvas = ckSurface.getCanvas(); + const ctx = createDrawingContext(Skia, paintPool, canvas); + ctx.paint.setColor(Skia.Color("red")); + canvas.drawCircle(32, 32, 24, ctx.paint); + ctx.dispose(); + ckSurface.flush(); + return Array.from( + ckSurface.makeImageSnapshot().readPixels(0, 0, { + width: 64, + height: 64, + colorType: ColorType.RGBA_8888, + alphaType: AlphaType.Unpremul, + })! + ); + }; + const paintPool: SkPaint[] = []; + const frame1 = drawFrame(paintPool); + const frame2 = drawFrame(paintPool); + // The circle edge shows partial coverage: anti-aliasing survived the + // paintPool[0].reset() performed when the pool is reused. + const alphas = frame2.filter((_, i) => i % 4 === 3); + expect(alphas.some((a) => a > 0 && a < 255)).toBe(true); + // A redraw reusing the pool renders exactly like the first frame. + expect(frame2).toEqual(frame1); + }); it("should accept a paint object as path property", async () => { const { Skia } = importSkia(); const paint = Skia.Paint(); diff --git a/packages/skia/src/sksg/Recorder/DrawingContext.ts b/packages/skia/src/sksg/Recorder/DrawingContext.ts index 9d5e2ea9e4..95986557ad 100644 --- a/packages/skia/src/sksg/Recorder/DrawingContext.ts +++ b/packages/skia/src/sksg/Recorder/DrawingContext.ts @@ -37,7 +37,10 @@ export const createDrawingContext = ( if (paintPool.length === 0) { paintPool.push(Skia.Paint()); } else { + // reset() produces an anti-alias false paint, unlike the Skia.Paint() + // factory: restore the default so reused pools render like the first frame. paintPool[0].reset(); + paintPool[0].setAntiAlias(true); } paints.push(paintPool[0]); opacities.push(1); @@ -56,12 +59,14 @@ export const createDrawingContext = ( nextPaintIndex++; }; - // Pushes an externally owned paint (the `paint` prop) onto the stack. It must - // push an opacity alongside it: restorePaint() pops both, so pushing only the - // paint would underflow the opacity stack and leak the enclosing group's - // opacity onto everything drawn afterwards. + // Pushes an externally owned paint (the `paint` prop) onto the stack. It + // pushes a frame-scoped copy, like the C++ DrawingCtx: materializePaint() + // mutates the top of the stack, and those mutations must not leak into the + // user-owned paint. It must also push an opacity alongside it: restorePaint() + // pops both, so pushing only the paint would underflow the opacity stack and + // leak the enclosing group's opacity onto everything drawn afterwards. const pushPaint = (paint: SkPaint) => { - paints.push(paint); + paints.push(track(paint.copy())); opacities.push(opacities[opacities.length - 1]); };