diff --git a/apps/docs/docs/mask.md b/apps/docs/docs/mask.md index c5944822c3..c2d3e73871 100644 --- a/apps/docs/docs/mask.md +++ b/apps/docs/docs/mask.md @@ -12,12 +12,12 @@ Just like its [CSS counterpart](https://developer.mozilla.org/en-US/docs/Web/CSS The first child of `Mask` is the drawing used as a mask, and the remaining children are the drawings to mask. -By default, the mask is not clipped. If you want to clip the mask with the bounds of the contents, use the `clip` property. +By default, the mask is clipped to the content: only the masked content is drawn. With `clip={false}`, the mask drawing itself stays visible wherever the content doesn't cover it. | Name | Type | Description | |:----------|:--------------------------|:--------------------------------------------------------------| | mode? | `alpha` or `luminance` | Is it a luminance or alpha mask (default is `alpha`) | -| clip? | `boolean` | clip the mask so it doesn't exceed the content | +| clip? | `boolean` | clip the mask so it doesn't exceed the content (default is `true`) | | mask | `ReactNode[] | ReactNode` | Mask definition | | children | `ReactNode[] | ReactNode` | Content to mask | diff --git a/packages/skia/cpp/api/JsiSkMatrix.h b/packages/skia/cpp/api/JsiSkMatrix.h index a7c6e531ce..243f80dc8c 100644 --- a/packages/skia/cpp/api/JsiSkMatrix.h +++ b/packages/skia/cpp/api/JsiSkMatrix.h @@ -69,12 +69,14 @@ class JsiSkMatrix void postTranslate(double x, double y) { getObject()->postTranslate(x, y); } + // A missing y means a uniform scale, matching the CanvasKit backend and the + // SkMatrix type, where y is optional rather than defaulted to 1. void scale(double x, JsiOptional y) { - getObject()->preScale(x, y.has_value() ? *y : 1); + getObject()->preScale(x, y.has_value() ? *y : x); } void postScale(double x, JsiOptional y) { - getObject()->postScale(x, y.has_value() ? *y : 1); + getObject()->postScale(x, y.has_value() ? *y : x); } void skew(double x, double y) { getObject()->preSkew(x, y); } diff --git a/packages/skia/cpp/api/recorder/Convertor.h b/packages/skia/cpp/api/recorder/Convertor.h index f3c9e0c18a..d2fcef4fec 100644 --- a/packages/skia/cpp/api/recorder/Convertor.h +++ b/packages/skia/cpp/api/recorder/Convertor.h @@ -361,13 +361,16 @@ SkM44 getPropertyValue(jsi::Runtime &runtime, const jsi::Value &value) { m4.preScale(1, s); } else if (key == "skewX") { auto angle = value.getProperty(runtime, key.c_str()).asNumber(); - SkM44 skewX(1, 0, 0, 0, std::tan(angle), 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, + // The SkM44 constructor takes its arguments in row-major reading + // order, so the shear factor of a horizontal skew belongs in row 0, + // where it scales y into x. + SkM44 skewX(1, std::tan(angle), 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); m4.preConcat(skewX); } else if (key == "skewY") { auto angle = value.getProperty(runtime, key.c_str()).asNumber(); - SkM44 skewY(1, std::tan(angle), 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, + SkM44 skewY(1, 0, 0, 0, std::tan(angle), 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1); m4.preConcat(skewY); } else if (key == "rotate" || key == "rotateZ") { diff --git a/packages/skia/cpp/api/recorder/Drawings.h b/packages/skia/cpp/api/recorder/Drawings.h index c5ce24b14c..a0398a4244 100644 --- a/packages/skia/cpp/api/recorder/Drawings.h +++ b/packages/skia/cpp/api/recorder/Drawings.h @@ -149,6 +149,9 @@ class PathCmd : public Command { if (hasStrokeOptions) { const auto &stroke = props.stroke.value(); SkPaint strokePaint; + // A default SkPaint is fill-style, and FillPathWithPaint only + // outlines a paint that strokes. + strokePaint.setStyle(SkPaint::kStroke_Style); if (stroke.cap.has_value()) { strokePaint.setStrokeCap(stroke.cap.value()); @@ -166,11 +169,18 @@ class PathCmd : public Command { strokePaint.setStrokeMiter(stroke.miter_limit.value()); } + float precision = stroke.precision.value_or(1.0f); + SkPathBuilder resultBuilder; - if (!skpathutils::FillPathWithPaint(*p, strokePaint, &resultBuilder)) { - throw std::runtime_error("Failed to apply stroke to path"); + auto ctm = SkMatrix::Scale(precision, precision); + if (skpathutils::FillPathWithPaint(*p, strokePaint, &resultBuilder, + nullptr, ctm)) { + pathToUse = std::make_shared(resultBuilder.snapshot()); + } else { + // The JS player keeps the unstroked path when Path.Stroke returns + // null (e.g. a hairline width of 0). + pathToUse = std::const_pointer_cast(p); } - pathToUse = std::make_shared(resultBuilder.snapshot()); } else { pathToUse = std::const_pointer_cast(p); } diff --git a/packages/skia/cpp/api/recorder/ImageFilters.h b/packages/skia/cpp/api/recorder/ImageFilters.h index 1a50a403c0..886f85c821 100644 --- a/packages/skia/cpp/api/recorder/ImageFilters.h +++ b/packages/skia/cpp/api/recorder/ImageFilters.h @@ -135,8 +135,22 @@ class DropShadowImageFilterCmd : public Command { auto sourceAlpha = SkImageFilters::ColorFilter( SkColorFilters::Blend(SK_ColorBLACK, SkBlendMode::kSrcIn), nullptr); + // The shadow is generated outside the shape and then clipped back into it, + // so "outside" has to be the complement of the shape's silhouette. Taking + // SrcOut against the source graphic itself makes it the complement of the + // source's *alpha*: inside a translucent shape 1 - alpha is non-zero, so + // the shadow is generated across the whole interior and tints it, no matter + // how small the blur and the offset are (issue #2990). Saturating alpha + // first keeps the shadow tied to the shape's outline instead of to its + // opacity. 255 is the saturation point of an 8-bit alpha channel, so one + // unit of coverage is enough to reach 1. + static constexpr float kAlphaSaturate[20] = { + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0}; + auto coverage = SkImageFilters::ColorFilter( + SkColorFilters::Matrix(kAlphaSaturate), nullptr); + auto f1 = SkImageFilters::ColorFilter( - SkColorFilters::Blend(color, SkBlendMode::kSrcOut), nullptr); + SkColorFilters::Blend(color, SkBlendMode::kSrcOut), coverage); auto f2 = SkImageFilters::Offset(dx, dy, f1); auto f3 = SkImageFilters::Blur(sigmaX, sigmaY, SkTileMode::kDecal, f2); diff --git a/packages/skia/cpp/api/recorder/Paint.h b/packages/skia/cpp/api/recorder/Paint.h index b4b2f1567b..5e3dec216a 100644 --- a/packages/skia/cpp/api/recorder/Paint.h +++ b/packages/skia/cpp/api/recorder/Paint.h @@ -154,8 +154,11 @@ class SavePaintCmd : public Command { } ctx->savePaint(); if (standalone) { - SkPaint freshPaint; - ctx->pushPaint(freshPaint); + // 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(); } auto &paint = ctx->getPaint(); if (props.opacity.has_value()) { diff --git a/packages/skia/src/__tests__/snapshots/drawings/ctm-balance-nested.png b/packages/skia/src/__tests__/snapshots/drawings/ctm-balance-nested.png new file mode 100644 index 0000000000..5d67228c53 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/ctm-balance-nested.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/ctm-balance.png b/packages/skia/src/__tests__/snapshots/drawings/ctm-balance.png new file mode 100644 index 0000000000..8687d52ae7 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/ctm-balance.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-opaque.png b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-opaque.png new file mode 100644 index 0000000000..37dc2ea7e9 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-opaque.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent-blur.png b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent-blur.png new file mode 100644 index 0000000000..06b75f0a29 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent-blur.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent-center.png b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent-center.png new file mode 100644 index 0000000000..df1fe0019b Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent-center.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent.png b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent.png new file mode 100644 index 0000000000..5cdefdeac5 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/inner-shadow-translucent.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/mask-composite-clipped.png b/packages/skia/src/__tests__/snapshots/drawings/mask-composite-clipped.png new file mode 100644 index 0000000000..92e65c3f1e Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/mask-composite-clipped.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/mask-composite-plain.png b/packages/skia/src/__tests__/snapshots/drawings/mask-composite-plain.png new file mode 100644 index 0000000000..e4a3084d33 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/mask-composite-plain.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/mask-composite-visible.png b/packages/skia/src/__tests__/snapshots/drawings/mask-composite-visible.png new file mode 100644 index 0000000000..3128386463 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/mask-composite-visible.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/paint-declaration-opacity-balance.png b/packages/skia/src/__tests__/snapshots/drawings/paint-declaration-opacity-balance.png new file mode 100644 index 0000000000..379248dc8d Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/paint-declaration-opacity-balance.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/paint-prop-opacity-balance.png b/packages/skia/src/__tests__/snapshots/drawings/paint-prop-opacity-balance.png new file mode 100644 index 0000000000..6fbecf2035 Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/paint-prop-opacity-balance.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/path-trim-stroke-full.png b/packages/skia/src/__tests__/snapshots/drawings/path-trim-stroke-full.png new file mode 100644 index 0000000000..73e89b46dc Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/path-trim-stroke-full.png differ diff --git a/packages/skia/src/__tests__/snapshots/drawings/path-trim-stroke.png b/packages/skia/src/__tests__/snapshots/drawings/path-trim-stroke.png new file mode 100644 index 0000000000..6f89bdd18d Binary files /dev/null and b/packages/skia/src/__tests__/snapshots/drawings/path-trim-stroke.png differ diff --git a/packages/skia/src/renderer/__tests__/e2e/CTMBalance.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/CTMBalance.spec.tsx new file mode 100644 index 0000000000..26a410d3bd --- /dev/null +++ b/packages/skia/src/renderer/__tests__/e2e/CTMBalance.spec.tsx @@ -0,0 +1,51 @@ +import React from "react"; + +import { checkImage } from "../../../__tests__/setup"; +import { Fill, Group, Rect } from "../../components"; +import { surface } from "../setup"; + +// A CTM that saves nothing must not emit a restore, otherwise it pops the save +// of an enclosing group and every later sibling loses that group's transform. +// Each case wraps a sibling in a group whose only CTM prop is inert: the scene +// has to come out identical to the reference, where that group has no props. + +const SHIFT = 128; +const REF = "snapshots/drawings/ctm-balance.png"; + +const drawScene = (inert: Record) => + surface.draw( + <> + + + + + + + + + ); + +describe("CTM save/restore balance", () => { + it("Build reference result", async () => { + checkImage(await drawScene({}), REF); + }); + + it("origin without transform or matrix", async () => { + checkImage(await drawScene({ origin: { x: 0, y: 0 } }), REF); + }); + + it("clip resolved to false by a conditional", async () => { + checkImage(await drawScene({ clip: false }), REF); + }); + + it("invertClip without a clip", async () => { + checkImage(await drawScene({ invertClip: false }), REF); + }); + + it("still restores for a CTM that does save", async () => { + // The inner group's own translate applies to the red rect only, and is + // restored before the blue sibling is drawn. + const image = await drawScene({ transform: [{ translateX: 32 }] }); + checkImage(image, "snapshots/drawings/ctm-balance-nested.png"); + }); +}); diff --git a/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx new file mode 100644 index 0000000000..2a2fd1e1c4 --- /dev/null +++ b/packages/skia/src/renderer/__tests__/e2e/InnerShadow.spec.tsx @@ -0,0 +1,155 @@ +import React from "react"; + +import { checkImage } from "../../../__tests__/setup"; +import { Fill, Group, RoundedRect, Shadow } from "../../components"; +import { importSkia, surface } from "../setup"; + +// An inner shadow is generated outside the shape and clipped back into it, so +// "outside" has to be the complement of the shape's silhouette. Taking SrcOut +// against the source graphic makes it the complement of the source's alpha +// instead: inside a translucent shape 1 - alpha is non-zero, so the shadow is +// generated across the whole interior and tints it - even when the blur and the +// offset are both zero (issue #2990). +// +// The reference results are drawn without a shadow, so they don't encode the +// behaviour under test. + +describe("Inner shadow", () => { + it("Build reference result", async () => { + const image = await surface.draw( + <> + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-translucent.png"); + }); + + it("should be a no-op without blur and offset on a translucent shape", async () => { + // Nothing outside the shape is moved or spread into it, so there is nothing + // to draw. + const image = await surface.draw( + <> + + + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-translucent.png"); + }); + + it("Build opaque reference result", async () => { + const image = await surface.draw( + <> + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-opaque.png"); + }); + + it("should be a no-op without blur and offset on an opaque shape", async () => { + // Regression guard: the opaque case is already a no-op and has to stay one. + const image = await surface.draw( + <> + + + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-opaque.png"); + }); + + it("Build centered reference result", async () => { + const { rect } = importSkia(); + const image = await surface.draw( + <> + + + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-translucent-center.png"); + }); + + it("should stay within reach of the blur on a translucent shape", async () => { + // The center of the shape is 48px away from its closest edge, far outside + // the reach of a 4px blur, so the shadow cannot touch it. + const { rect } = importSkia(); + const image = await surface.draw( + <> + + + + + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-translucent-center.png"); + }); + + it("should draw the blurred shadow along the edges of a translucent shape", async () => { + const image = await surface.draw( + <> + + + + + + ); + checkImage(image, "snapshots/drawings/inner-shadow-translucent-blur.png"); + }); +}); diff --git a/packages/skia/src/renderer/__tests__/e2e/MaskComposite.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/MaskComposite.spec.tsx new file mode 100644 index 0000000000..de6d885ddd --- /dev/null +++ b/packages/skia/src/renderer/__tests__/e2e/MaskComposite.spec.tsx @@ -0,0 +1,157 @@ +import React from "react"; + +import { checkImage } from "../../../__tests__/setup"; +import { Circle, Fill, Group, Mask, Rect } from "../../components"; +import { importSkia, surface } from "../setup"; + +// renders its children into a layer and applies the mask once, when the +// mask's own layer is restored. A blend mode applied per draw call instead +// composites every child after the first against the previous child rather +// than against the mask, and the overlap of two translucent children comes out +// wrong (issue #3254). With the default clip the mask is applied with dstIn, +// so only the masked children remain; with clip={false} it is applied with +// dstATop, which also keeps the mask artwork visible wherever the children +// leave it uncovered. +// +// None of the references below is drawn with , so none of them encodes +// the behaviour under test: they are the plain drawing, the same drawing +// behind a geometric clip, and the same drawing over the visible mask artwork. + +const REF_PLAIN = "snapshots/drawings/mask-composite-plain.png"; +const REF_CLIPPED = "snapshots/drawings/mask-composite-clipped.png"; +const REF_VISIBLE = "snapshots/drawings/mask-composite-visible.png"; + +// Two overlapping translucent circles: the overlap is only right if they are +// composited against each other before the mask is applied, and the 0.5 alpha +// is only right if the mask alpha is multiplied in exactly once. +const twoChildren = ( + <> + + + +); + +describe("Mask composition", () => { + it("Build reference result", async () => { + const image = await surface.draw( + <> + + {twoChildren} + + ); + checkImage(image, REF_PLAIN); + }); + + it("should not alter the drawing when the mask is opaque everywhere", async () => { + // An alpha mask that is opaque everywhere selects the whole drawing, so + // the result has to be the drawing itself - under the default clip, and + // with the translucent children rendered at their own alpha (the mask + // alpha must not be multiplied in twice). + const image = await surface.draw( + <> + + }>{twoChildren} + + ); + checkImage(image, REF_PLAIN); + }); + + it("Build clipped reference result", async () => { + const { rect } = importSkia(); + const image = await surface.draw( + <> + + {twoChildren} + + ); + checkImage(image, REF_CLIPPED); + }); + + it("should select the drawing without recompositing it", async () => { + // An alpha mask that is opaque on the left half and empty on the right + // half selects exactly what the matching rectangular clip selects. The + // mask is magenta: an alpha mask only contributes coverage, so none of + // its color may leak into the result - the pixels outside the children + // are load-bearing here. + const image = await surface.draw( + <> + + } + > + {twoChildren} + + + ); + checkImage(image, REF_CLIPPED); + }); + + it("should select the drawing by luminance", async () => { + // White has luminance 1 and black has luminance 0, so a white-on-black + // luminance mask selects exactly what the alpha mask above selects. + const image = await surface.draw( + <> + + + + + + } + > + {twoChildren} + + + ); + checkImage(image, REF_CLIPPED); + }); + + it("Build visible mask reference result", async () => { + const { rect } = importSkia(); + const image = await surface.draw( + <> + + + + {twoChildren} + + + ); + checkImage(image, REF_VISIBLE); + }); + + it("should keep the mask artwork visible with clip={false}", async () => { + // Without clip, the mask artwork itself remains wherever the children + // leave it uncovered, and the children composite over it - but the + // children still only show where the mask has coverage. + const image = await surface.draw( + <> + + } + > + {twoChildren} + + + ); + checkImage(image, REF_VISIBLE); + }); + + it("should composite every draw of a single child correctly", async () => { + // Regression guard: the per-draw compositing bug fired per draw call, not + // per React child - a single child recording several draws was corrupted + // the same way. A child-count fast path must not reintroduce it. + const image = await surface.draw( + <> + + }> + {twoChildren} + + + ); + checkImage(image, REF_PLAIN); + }); +}); diff --git a/packages/skia/src/renderer/__tests__/e2e/Matrix.spec.ts b/packages/skia/src/renderer/__tests__/e2e/Matrix.spec.ts new file mode 100644 index 0000000000..9bd7520adf --- /dev/null +++ b/packages/skia/src/renderer/__tests__/e2e/Matrix.spec.ts @@ -0,0 +1,30 @@ +import { surface } from "../setup"; + +describe("SkMatrix", () => { + it("scales uniformly when y is omitted", async () => { + const result = await surface.eval((Skia) => { + const matrix = Skia.Matrix(); + matrix.scale(2); + return matrix.get(); + }); + expect(result).toEqual([2, 0, 0, 0, 2, 0, 0, 0, 1]); + }); + + it("postScales uniformly when y is omitted", async () => { + const result = await surface.eval((Skia) => { + const matrix = Skia.Matrix(); + matrix.postScale(2); + return matrix.get(); + }); + expect(result).toEqual([2, 0, 0, 0, 2, 0, 0, 0, 1]); + }); + + it("keeps the two axes independent when y is given", async () => { + const result = await surface.eval((Skia) => { + const matrix = Skia.Matrix(); + matrix.scale(2, 3); + return matrix.get(); + }); + expect(result).toEqual([2, 0, 0, 0, 3, 0, 0, 0, 1]); + }); +}); diff --git a/packages/skia/src/renderer/__tests__/e2e/PaintStackBalance.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/PaintStackBalance.spec.tsx new file mode 100644 index 0000000000..374fcd9a3d --- /dev/null +++ b/packages/skia/src/renderer/__tests__/e2e/PaintStackBalance.spec.tsx @@ -0,0 +1,45 @@ +import React from "react"; + +import { checkImage } from "../../../__tests__/setup"; +import { Group, Paint, Rect } from "../../components"; +import { importSkia, surface } from "../setup"; + +// Every SavePaint command must push exactly one frame onto the paint/opacity +// stack, because RestorePaint and RestorePaintDeclaration each pop exactly one. +// When that invariant breaks, the opacity of an enclosing is left on +// the stack and applied to every sibling drawn after it (issue #3355). +describe("Paint stack balance", () => { + it("should not leak a group opacity through a paint prop", async () => { + const { Skia } = importSkia(); + const paint = Skia.Paint(); + paint.setColor(Skia.Color("red")); + const image = await surface.draw( + <> + + + + + + + ); + checkImage(image, "snapshots/drawings/paint-prop-opacity-balance.png"); + }); + + it("should not leak a group opacity through a paint declaration", async () => { + const image = await surface.draw( + <> + + + + + + + + + ); + checkImage( + image, + "snapshots/drawings/paint-declaration-opacity-balance.png" + ); + }); +}); diff --git a/packages/skia/src/renderer/__tests__/e2e/PathTrim.spec.tsx b/packages/skia/src/renderer/__tests__/e2e/PathTrim.spec.tsx new file mode 100644 index 0000000000..fe998aa984 --- /dev/null +++ b/packages/skia/src/renderer/__tests__/e2e/PathTrim.spec.tsx @@ -0,0 +1,45 @@ +import React from "react"; + +import { checkImage } from "../../../__tests__/setup"; +import { Fill, Path } from "../../components"; +import { surface } from "../setup"; + +// The stroke prop replaces the path with the outline of the stroke, so the +// trim has to run first - otherwise start/end walk that outline's perimeter +// and end up painting a different part of the line. + +const LINE = "M 20 128 L 236 128"; + +describe("Path trim", () => { + it("trims the path before turning it into a stroke outline", async () => { + const image = await surface.draw( + <> + + + + ); + checkImage(image, "snapshots/drawings/path-trim-stroke.png"); + }); + + it("leaves an untrimmed stroke alone", async () => { + const image = await surface.draw( + <> + + + + ); + checkImage(image, "snapshots/drawings/path-trim-stroke-full.png"); + }); +}); diff --git a/packages/skia/src/renderer/components/Mask.tsx b/packages/skia/src/renderer/components/Mask.tsx index f0100bf2e5..21325e229b 100644 --- a/packages/skia/src/renderer/components/Mask.tsx +++ b/packages/skia/src/renderer/components/Mask.tsx @@ -20,17 +20,24 @@ export const Mask = ({ }: MaskProps) => { return ( + {children} + {/* The children composite against each other in the layer above; the + mask is then applied once, when its own layer is restored. A blend + mode attached to a without a layer would instead be applied + per draw call, compositing every child after the first against the + previous child rather than against the mask (issue #3254). + dstIn keeps the children where the mask is opaque and erases them + where it is transparent. Without clip, dstATop additionally keeps + the mask artwork itself wherever the children leave it uncovered. */} + {mode === "luminance" && } } > {mask} - {clip && }>{children}} - {children} ); }; diff --git a/packages/skia/src/sksg/Recorder/DrawingContext.ts b/packages/skia/src/sksg/Recorder/DrawingContext.ts index 26faae3101..9d5e2ea9e4 100644 --- a/packages/skia/src/sksg/Recorder/DrawingContext.ts +++ b/packages/skia/src/sksg/Recorder/DrawingContext.ts @@ -56,6 +56,15 @@ 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. + const pushPaint = (paint: SkPaint) => { + paints.push(paint); + opacities.push(opacities[opacities.length - 1]); + }; + const getOpacity = () => { return opacities[opacities.length - 1]; }; @@ -144,6 +153,7 @@ export const createDrawingContext = ( // Public methods savePaint, + pushPaint, saveBackdropFilter, get paint() { return paints[paints.length - 1]; diff --git a/packages/skia/src/sksg/Recorder/Player.ts b/packages/skia/src/sksg/Recorder/Player.ts index 08537c3372..614989eefe 100644 --- a/packages/skia/src/sksg/Recorder/Player.ts +++ b/packages/skia/src/sksg/Recorder/Player.ts @@ -123,7 +123,7 @@ const play = (ctx: DrawingContext, _command: Command) => { ctx.canvas.saveLayer(paint); } else if (isDrawCommand(command, CommandType.SavePaint)) { if (command.props.paint) { - ctx.paints.push(command.props.paint); + ctx.pushPaint(command.props.paint); } else { // eslint-disable-next-line @typescript-eslint/no-explicit-any const { standalone } = command as any; diff --git a/packages/skia/src/sksg/Recorder/Visitor.ts b/packages/skia/src/sksg/Recorder/Visitor.ts index 2842b4b388..4862d0faa9 100644 --- a/packages/skia/src/sksg/Recorder/Visitor.ts +++ b/packages/skia/src/sksg/Recorder/Visitor.ts @@ -104,13 +104,16 @@ const processCTM = ({ if (layer) { ctm.layer = layer; } + // Only record a CTM command when saveCTM() will actually save the canvas. + // saveCTM() saves on transform/matrix, clip or layer; restoreCTM() always + // restores, so recording a CTM that saves nothing - `origin` with no + // transform, `clip={false}` from a conditional, a bare `invertClip` - pops + // an enclosing save that belongs to an ancestor. if ( - clip !== undefined || - invertClip !== undefined || - transform !== undefined || - origin !== undefined || - matrix !== undefined || - layer !== undefined + ctm.clip !== undefined || + ctm.transform !== undefined || + ctm.matrix !== undefined || + ctm.layer !== undefined ) { return ctm; } diff --git a/packages/skia/src/sksg/Recorder/commands/Drawing.ts b/packages/skia/src/sksg/Recorder/commands/Drawing.ts index 1cae0dde0f..959ae40274 100644 --- a/packages/skia/src/sksg/Recorder/commands/Drawing.ts +++ b/packages/skia/src/sksg/Recorder/commands/Drawing.ts @@ -217,6 +217,16 @@ export const drawPath = (ctx: DrawingContext, props: PathProps) => { let path = processPath(ctx.Skia, pathProps.path); + // Trim runs first, so start/end address the path the caller drew. Stroking + // first would replace it with its outline and leave the offsets walking that + // outline's perimeter instead. The native recorder orders these the same way. + if (hasStartOffset || hasEndOffset) { + const trimmed = ctx.Skia.Path.Trim(path, start, end, false); + if (trimmed) { + path = trimmed; + } + } + // Apply fill type using PathBuilder if (hasFillType) { const builder = ctx.Skia.PathBuilder.MakeFromPath(path); @@ -232,14 +242,6 @@ export const drawPath = (ctx: DrawingContext, props: PathProps) => { } } - // Apply trim using static Path.Trim - if (hasStartOffset || hasEndOffset) { - const trimmed = ctx.Skia.Path.Trim(path, start, end, false); - if (trimmed) { - path = trimmed; - } - } - ctx.canvas.drawPath(path, ctx.paint); }; diff --git a/packages/skia/src/sksg/Recorder/commands/ImageFilters.ts b/packages/skia/src/sksg/Recorder/commands/ImageFilters.ts index 6b1726b650..af27c99995 100644 --- a/packages/skia/src/sksg/Recorder/commands/ImageFilters.ts +++ b/packages/skia/src/sksg/Recorder/commands/ImageFilters.ts @@ -31,6 +31,14 @@ export enum MorphologyOperator { const Black = Float32Array.of(0, 0, 0, 1); +// Turns the source graphic into a hard silhouette: alpha is multiplied by 255 +// and clamped, so any pixel the shape covers at all becomes fully opaque, and +// the colour channels are dropped. 255 is the saturation point of an 8-bit +// alpha channel, so one unit of coverage is enough to reach 1. +const AlphaSaturate = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0, +]; + const MakeInnerShadow = ( Skia: Skia, shadowOnly: boolean | undefined, @@ -50,9 +58,20 @@ const MakeInnerShadow = ( Skia.ColorFilter.MakeBlend(Black, BlendMode.SrcIn), null ); + // The shadow is generated outside the shape and then clipped back into it, so + // "outside" has to be the complement of the shape's silhouette. Taking SrcOut + // against the source graphic itself makes it the complement of the source's + // *alpha*: inside a translucent shape 1 - alpha is non-zero, so the shadow is + // generated across the whole interior and tints it, no matter how small the + // blur and the offset are (issue #2990). Saturating alpha first keeps the + // shadow tied to the shape's outline instead of to its opacity. + const coverage = Skia.ImageFilter.MakeColorFilter( + Skia.ColorFilter.MakeMatrix(AlphaSaturate), + null + ); const f1 = Skia.ImageFilter.MakeColorFilter( Skia.ColorFilter.MakeBlend(color, BlendMode.SrcOut), - null + coverage ); const f2 = Skia.ImageFilter.MakeOffset(dx, dy, f1); const f3 = Skia.ImageFilter.MakeBlur(sigmaX, sigmaY, TileMode.Decal, f2);