From d2a508104e19446e9cc4b95f47e74f3c3775b08d Mon Sep 17 00:00:00 2001 From: Preet <833927+pshihn@users.noreply.github.com> Date: Fri, 20 Oct 2023 06:08:24 -0700 Subject: [PATCH] fix: Better fill rendering with latest RoughJS (#7031) Co-authored-by: dwelle --- package.json | 4 ++-- src/element/collision.ts | 2 +- src/element/textWysiwyg.test.tsx | 19 ++++--------------- src/scene/Shape.ts | 4 +++- src/tests/helpers/ui.ts | 7 ++----- src/tests/queries/dom.ts | 13 +++++++++++++ .../scene/__snapshots__/export.test.ts.snap | 8 ++++---- yarn.lock | 19 +++++++++++++++---- 8 files changed, 44 insertions(+), 32 deletions(-) create mode 100644 src/tests/queries/dom.ts diff --git a/package.json b/package.json index 5ae2d1ff..5e297fdd 100644 --- a/package.json +++ b/package.json @@ -49,11 +49,11 @@ "png-chunk-text": "1.0.0", "png-chunks-encode": "1.0.0", "png-chunks-extract": "1.0.0", - "points-on-curve": "0.2.0", + "points-on-curve": "1.0.1", "pwacompat": "2.0.17", "react": "18.2.0", "react-dom": "18.2.0", - "roughjs": "4.5.2", + "roughjs": "4.6.4", "sass": "1.51.0", "socket.io-client": "2.3.1", "tunnel-rat": "0.1.2" diff --git a/src/element/collision.ts b/src/element/collision.ts index d04f1a0e..103d3fd9 100644 --- a/src/element/collision.ts +++ b/src/element/collision.ts @@ -494,7 +494,7 @@ const hitTestFreeDrawElement = ( // for filled freedraw shapes, support // selecting from inside if (shape && shape.sets.length) { - return hitTestRoughShape(shape, x, y, threshold); + return hitTestCurveInside(shape, x, y, "round"); } return false; diff --git a/src/element/textWysiwyg.test.tsx b/src/element/textWysiwyg.test.tsx index 842d75af..e02fa993 100644 --- a/src/element/textWysiwyg.test.tsx +++ b/src/element/textWysiwyg.test.tsx @@ -8,7 +8,7 @@ import { mockBoundingClientRect, restoreOriginalGetBoundingClientRect, } from "../tests/test-utils"; -import { queryByText, waitFor } from "@testing-library/react"; +import { queryByText } from "@testing-library/react"; import { FONT_FAMILY, TEXT_ALIGN, VERTICAL_ALIGN } from "../constants"; import { @@ -18,6 +18,7 @@ import { import { API } from "../tests/helpers/api"; import { mutateElement } from "./mutateElement"; import { getOriginalContainerHeightFromCache } from "./textWysiwyg"; +import { getTextEditor } from "../tests/queries/dom"; // Unmount ReactDOM from root ReactDOM.unmountComponentAtNode(document.getElementById("root")!); @@ -25,18 +26,6 @@ ReactDOM.unmountComponentAtNode(document.getElementById("root")!); const tab = " "; const mouse = new Pointer("mouse"); -const getTextEditor = async (waitForEditor = false) => { - const query = () => - document.querySelector( - ".excalidraw-textEditorContainer > textarea", - ) as HTMLTextAreaElement; - if (waitForEditor) { - waitFor(() => expect(query()).not.toBe(null)); - return query(); - } - return query(); -}; - const updateTextEditor = (editor: HTMLTextAreaElement, value: string) => { fireEvent.change(editor, { target: { value } }); editor.dispatchEvent(new Event("input")); @@ -206,7 +195,7 @@ describe("textWysiwyg", () => { mouse.clickAt(text.x + 50, text.y + 50); - const editor = await getTextEditor(); + const editor = await getTextEditor(false); expect(editor).not.toBe(null); expect(h.state.editingElement?.id).toBe(text.id); @@ -228,7 +217,7 @@ describe("textWysiwyg", () => { mouse.doubleClickAt(text.x + 50, text.y + 50); - const editor = await getTextEditor(); + const editor = await getTextEditor(false); expect(editor).not.toBe(null); expect(h.state.editingElement?.id).toBe(text.id); diff --git a/src/scene/Shape.ts b/src/scene/Shape.ts index f43a1260..5a2f68bb 100644 --- a/src/scene/Shape.ts +++ b/src/scene/Shape.ts @@ -12,6 +12,7 @@ import type { import { isPathALoop, getCornerRadius } from "../math"; import { generateFreeDrawShape } from "../renderer/renderElement"; import { isTransparent, assertNever } from "../utils"; +import { simplify } from "points-on-curve"; const getDashArrayDashed = (strokeWidth: number) => [8, 8 + strokeWidth]; @@ -334,7 +335,8 @@ export const _generateElementShape = ( if (isPathALoop(element.points)) { // generate rough polygon to fill freedraw shape - shape = generator.polygon(element.points as [number, number][], { + const simplifiedPoints = simplify(element.points, 0.75); + shape = generator.curve(simplifiedPoints as [number, number][], { ...generateRoughOptions(element), stroke: "none", }); diff --git a/src/tests/helpers/ui.ts b/src/tests/helpers/ui.ts index 62290fe2..105d7a39 100644 --- a/src/tests/helpers/ui.ts +++ b/src/tests/helpers/ui.ts @@ -32,6 +32,7 @@ import { } from "../../element/typeChecks"; import { getCommonBounds, getElementPointsCoords } from "../../element/bounds"; import { rotatePoint } from "../../math"; +import { getTextEditor } from "../queries/dom"; const { h } = window; @@ -476,11 +477,7 @@ export class UI { Keyboard.keyPress(KEYS.ENTER); } - const editor = - openedEditor ?? - document.querySelector( - ".excalidraw-textEditorContainer > textarea", - ); + const editor = await getTextEditor(); if (!editor) { throw new Error("Can't find wysiwyg text editor in the dom"); } diff --git a/src/tests/queries/dom.ts b/src/tests/queries/dom.ts new file mode 100644 index 00000000..62364e4c --- /dev/null +++ b/src/tests/queries/dom.ts @@ -0,0 +1,13 @@ +import { waitFor } from "@testing-library/dom"; + +export const getTextEditor = async (waitForEditor = true) => { + const query = () => + document.querySelector( + ".excalidraw-textEditorContainer > textarea", + ) as HTMLTextAreaElement; + if (waitForEditor) { + waitFor(() => expect(query()).not.toBe(null)); + return query(); + } + return query(); +}; diff --git a/src/tests/scene/__snapshots__/export.test.ts.snap b/src/tests/scene/__snapshots__/export.test.ts.snap index 2b959db2..8d000f84 100644 --- a/src/tests/scene/__snapshots__/export.test.ts.snap +++ b/src/tests/scene/__snapshots__/export.test.ts.snap @@ -42,7 +42,7 @@ exports[`exportToSvg > with default arguments 1`] = ` transform="translate(10 10) rotate(0 50 50)" > with default arguments 1`] = ` transform="translate(10 10) rotate(0 50 50)" > with elements that have a link 1`] = ` - " + " `; exports[`exportToSvg > with exportEmbedScene 1`] = ` @@ -111,5 +111,5 @@ exports[`exportToSvg > with exportEmbedScene 1`] = ` - " + " `; diff --git a/yarn.lock b/yarn.lock index 95a7ed8b..8b34b8de 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4684,6 +4684,11 @@ grapheme-splitter@^1.0.4: resolved "https://registry.yarnpkg.com/grapheme-splitter/-/grapheme-splitter-1.0.4.tgz#9cf3a665c6247479896834af35cf1dbb4400767e" integrity sha512-bzh50DW9kTPM00T8y4o8vQg89Di9oLJVLW/KaOGIXJWP/iqCN6WKYkbNOF04vFLJhwcpYUh9ydh/+5vpOqV4YQ== +hachure-fill@^0.5.2: + version "0.5.2" + resolved "https://registry.yarnpkg.com/hachure-fill/-/hachure-fill-0.5.2.tgz#d19bc4cc8750a5962b47fb1300557a85fcf934cc" + integrity sha512-3GKBOn+m2LX9iq+JC1064cSFprJY4jL1jCXTcpnfER5HYE2l/4EfWSGzkPa/ZDBmYI0ZOEj5VHV/eKnPGkHuOg== + has-bigints@^1.0.1, has-bigints@^1.0.2: version "1.0.2" resolved "https://registry.yarnpkg.com/has-bigints/-/has-bigints-1.0.2.tgz#0871bd3e3d51626f6ca0966668ba35d5602d6eaa" @@ -6028,6 +6033,11 @@ points-on-curve@0.2.0, points-on-curve@^0.2.0: resolved "https://registry.yarnpkg.com/points-on-curve/-/points-on-curve-0.2.0.tgz#7dbb98c43791859434284761330fa893cb81b4d1" integrity sha512-0mYKnYYe9ZcqMCWhUjItv/oHjvgEsfKvnUTg8sAtnHr3GVy7rGkXCb6d5cSyqrWqL4k81b9CPg3urd+T7aop3A== +points-on-curve@1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/points-on-curve/-/points-on-curve-1.0.1.tgz#03fcdc08e48e3bfdc7e8bd1d7ccd4d5f11e132c6" + integrity sha512-3nmX4/LIiyuwGLwuUrfhTlDeQFlAhi7lyK/zcRNGhalwapDWgAGR82bUpmn2mA03vII3fvNCG8jAONzKXwpxAg== + points-on-path@^0.2.1: version "0.2.1" resolved "https://registry.yarnpkg.com/points-on-path/-/points-on-path-0.2.1.tgz#553202b5424c53bed37135b318858eacff85dd52" @@ -6449,11 +6459,12 @@ rollup@^3.25.2: optionalDependencies: fsevents "~2.3.2" -roughjs@4.5.2: - version "4.5.2" - resolved "https://registry.yarnpkg.com/roughjs/-/roughjs-4.5.2.tgz#aab644dcb41e9a75826c8bd5a5b0a859095f2f10" - integrity sha512-2xSlLDKdsWyFxrveYWk9YQ/Y9UfK38EAMRNkYkMqYBJvPX8abCa9PN0x3w02H8Oa6/0bcZICJU+U95VumPqseg== +roughjs@4.6.4: + version "4.6.4" + resolved "https://registry.yarnpkg.com/roughjs/-/roughjs-4.6.4.tgz#b6f39b44645854a6e0a4a28b078368701eb7f939" + integrity sha512-s6EZ0BntezkFYMf/9mGn7M8XGIoaav9QQBCnJROWB3brUWQ683Q2LbRD/hq0Z3bAJ/9NVpU/5LpiTWvQMyLDhw== dependencies: + hachure-fill "^0.5.2" path-data-parser "^0.1.0" points-on-curve "^0.2.0" points-on-path "^0.2.1"