fix(editor): ensure canvas is cleared when background color is invalid to prevent ghosting (#11458)
Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
c070c8ffa6
commit
4ce70b815e
@@ -0,0 +1,90 @@
|
|||||||
|
import { COLOR_WHITE } from "@excalidraw/common";
|
||||||
|
|
||||||
|
import { bootstrapCanvas } from "./helpers";
|
||||||
|
|
||||||
|
const setup = () => {
|
||||||
|
const canvas = document.createElement("canvas");
|
||||||
|
canvas.width = 200;
|
||||||
|
canvas.height = 100;
|
||||||
|
const context = canvas.getContext("2d")!;
|
||||||
|
const clearRect = vi.spyOn(context, "clearRect");
|
||||||
|
const fillRect = vi.spyOn(context, "fillRect");
|
||||||
|
return { canvas, context, clearRect, fillRect };
|
||||||
|
};
|
||||||
|
|
||||||
|
const run = (viewBackgroundColor: unknown) => {
|
||||||
|
const { canvas, context, clearRect, fillRect } = setup();
|
||||||
|
bootstrapCanvas({
|
||||||
|
canvas,
|
||||||
|
scale: 1,
|
||||||
|
normalizedWidth: 200,
|
||||||
|
normalizedHeight: 100,
|
||||||
|
viewBackgroundColor: viewBackgroundColor as string,
|
||||||
|
});
|
||||||
|
return { context, clearRect, fillRect };
|
||||||
|
};
|
||||||
|
|
||||||
|
describe("bootstrapCanvas background painting", () => {
|
||||||
|
it("skips clearRect for an opaque hex color (fill fully repaints)", () => {
|
||||||
|
const { clearRect, fillRect } = run("#ffffff");
|
||||||
|
expect(clearRect).not.toHaveBeenCalled();
|
||||||
|
expect(fillRect).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips clearRect for a 3-digit opaque hex color", () => {
|
||||||
|
const { clearRect, fillRect } = run("#fff");
|
||||||
|
expect(clearRect).not.toHaveBeenCalled();
|
||||||
|
expect(fillRect).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears for a hex color with alpha (#RGBA / #RRGGBBAA)", () => {
|
||||||
|
expect(run("#ffff").clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
expect(run("#ffffff80").clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears and skips fill for the transparent keyword", () => {
|
||||||
|
const { clearRect, fillRect } = run("transparent");
|
||||||
|
expect(clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
expect(fillRect).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears for rgba()/hsla() colors and still fills", () => {
|
||||||
|
const rgba = run("rgba(255, 0, 0, 0.5)");
|
||||||
|
expect(rgba.clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
expect(rgba.fillRect).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
// the ghosting bug (#10931): a corrupted value must never leave the prior
|
||||||
|
// frame on screen — we always clear when we can't prove the color is opaque
|
||||||
|
it("clears for a corrupted color value to prevent ghosting", () => {
|
||||||
|
expect(run("0000").clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
expect(run("asdfgh").clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to white when the color is rejected by the canvas", () => {
|
||||||
|
const { canvas, context } = setup();
|
||||||
|
// simulate a stale fillStyle left over from a previous frame's drawing
|
||||||
|
context.fillStyle = "#ff0000";
|
||||||
|
let fillStyleAtFillTime = "";
|
||||||
|
vi.spyOn(context, "fillRect").mockImplementation(() => {
|
||||||
|
fillStyleAtFillTime = context.fillStyle as string;
|
||||||
|
});
|
||||||
|
|
||||||
|
bootstrapCanvas({
|
||||||
|
canvas,
|
||||||
|
scale: 1,
|
||||||
|
normalizedWidth: 200,
|
||||||
|
normalizedHeight: 100,
|
||||||
|
viewBackgroundColor: "not-a-color",
|
||||||
|
});
|
||||||
|
|
||||||
|
// not the stale red — the seeded default
|
||||||
|
expect(fillStyleAtFillTime).toBe(COLOR_WHITE);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clears for a non-string background", () => {
|
||||||
|
const { clearRect, fillRect } = run(undefined);
|
||||||
|
expect(clearRect).toHaveBeenCalledTimes(1);
|
||||||
|
expect(fillRect).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
import { THEME, applyDarkModeFilter } from "@excalidraw/common";
|
import { COLOR_WHITE, THEME, applyDarkModeFilter } from "@excalidraw/common";
|
||||||
|
|
||||||
import type { StaticCanvasRenderConfig } from "../scene/types";
|
import type { StaticCanvasRenderConfig } from "../scene/types";
|
||||||
import type { AppState, StaticCanvasAppState } from "../types";
|
import type { AppState, StaticCanvasAppState } from "../types";
|
||||||
@@ -53,21 +53,31 @@ export const bootstrapCanvas = ({
|
|||||||
|
|
||||||
// Paint background
|
// Paint background
|
||||||
if (typeof viewBackgroundColor === "string") {
|
if (typeof viewBackgroundColor === "string") {
|
||||||
const hasTransparence =
|
// An opaque fill repaints every pixel, so clearRect would be redundant.
|
||||||
viewBackgroundColor === "transparent" ||
|
// For anything else — transparency, or a value we can't be certain about
|
||||||
viewBackgroundColor.length === 5 || // #RGBA
|
// (e.g. corrupted persisted state like "0000") — clear first so the
|
||||||
viewBackgroundColor.length === 9 || // #RRGGBBA
|
// previous frame can't bleed through.
|
||||||
/(hsla|rgba)\(/.test(viewBackgroundColor);
|
//
|
||||||
if (hasTransparence) {
|
// We skip opaque #RRGGBB and #RGB hex colors as a quick optimization.
|
||||||
|
const isOpaque = /^#([0-9a-f]{3}|[0-9a-f]{6})$/i.test(viewBackgroundColor);
|
||||||
|
|
||||||
|
if (!isOpaque) {
|
||||||
context.clearRect(0, 0, normalizedWidth, normalizedHeight);
|
context.clearRect(0, 0, normalizedWidth, normalizedHeight);
|
||||||
}
|
}
|
||||||
context.save();
|
|
||||||
context.fillStyle = applyDarkModeFilter(
|
if (viewBackgroundColor !== "transparent") {
|
||||||
viewBackgroundColor,
|
context.save();
|
||||||
theme === THEME.DARK,
|
// The canvas silently ignores an invalid fillStyle, which would leave a
|
||||||
);
|
// stale color from a previous draw. Seed a sane default so corrupted
|
||||||
context.fillRect(0, 0, normalizedWidth, normalizedHeight);
|
// values fall back to white instead of painting garbage.
|
||||||
context.restore();
|
context.fillStyle = COLOR_WHITE;
|
||||||
|
context.fillStyle = applyDarkModeFilter(
|
||||||
|
viewBackgroundColor,
|
||||||
|
theme === THEME.DARK,
|
||||||
|
);
|
||||||
|
context.fillRect(0, 0, normalizedWidth, normalizedHeight);
|
||||||
|
context.restore();
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
context.clearRect(0, 0, normalizedWidth, normalizedHeight);
|
context.clearRect(0, 0, normalizedWidth, normalizedHeight);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import {
|
import {
|
||||||
applyDarkModeFilter,
|
applyDarkModeFilter,
|
||||||
|
COLOR_WHITE,
|
||||||
FRAME_STYLE,
|
FRAME_STYLE,
|
||||||
THEME,
|
THEME,
|
||||||
throttleRAF,
|
throttleRAF,
|
||||||
@@ -204,7 +205,13 @@ const renderLinkIcon = (
|
|||||||
window.devicePixelRatio * appState.zoom.value,
|
window.devicePixelRatio * appState.zoom.value,
|
||||||
window.devicePixelRatio * appState.zoom.value,
|
window.devicePixelRatio * appState.zoom.value,
|
||||||
);
|
);
|
||||||
linkCanvasCacheContext.fillStyle = appState.viewBackgroundColor || "#fff";
|
|
||||||
|
// Seed a sane default so a corrupted color (silently rejected by the
|
||||||
|
// canvas) falls back to white instead of a stale fillStyle.
|
||||||
|
linkCanvasCacheContext.fillStyle = COLOR_WHITE;
|
||||||
|
linkCanvasCacheContext.fillStyle =
|
||||||
|
appState.viewBackgroundColor || COLOR_WHITE;
|
||||||
|
|
||||||
linkCanvasCacheContext.fillRect(0, 0, width, height);
|
linkCanvasCacheContext.fillRect(0, 0, width, height);
|
||||||
|
|
||||||
if (canvasKey === "elementLink") {
|
if (canvasKey === "elementLink") {
|
||||||
|
|||||||
Reference in New Issue
Block a user