From 7792f6978273bb077c827490aeb964922b0d3f10 Mon Sep 17 00:00:00 2001 From: Marcel Mraz Date: Sun, 23 Jul 2023 16:08:15 +0200 Subject: [PATCH] Extracting common calculations to the canvases wrapper --- src/components/App.tsx | 8 +- src/components/canvases/CanvasesWrapper.tsx | 25 +++--- src/components/canvases/InteractiveCanvas.tsx | 22 +++-- src/components/canvases/StaticCanvas.tsx | 24 ++++-- src/element/sizeHelpers.ts | 40 ++++++++- src/hooks/useMutatedElements.ts | 16 ++-- src/hooks/useVisibleElements.ts | 39 +++++++++ src/renderer/renderScene.ts | 83 +++---------------- src/scene/export.ts | 25 +++++- src/scene/types.ts | 8 +- .../linearElementEditor.test.tsx.snap | 2 +- src/tests/linearElementEditor.test.tsx | 9 +- 12 files changed, 174 insertions(+), 127 deletions(-) create mode 100644 src/hooks/useVisibleElements.ts diff --git a/src/components/App.tsx b/src/components/App.tsx index 977783c00..35321cc3b 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -187,7 +187,7 @@ import { KEYS, } from "../keys"; import { distance2d, getGridPoint, isPathALoop } from "../math"; -import { isVisibleElement } from "../renderer/renderScene"; +import { isVisibleElement } from "../element/sizeHelpers"; import { invalidateShapeForElement } from "../renderer/renderElement"; import { calculateScrollCenter, @@ -839,16 +839,18 @@ class App extends React.Component { appState={this.state} scene={this.scene} > - {(elements, versionNonce) => ( + {(versionNonce, elements, visibleElements) => ( <> { ReactNode; }; -const CanvasesWrapper = (props: CanvasesWrapperProps) => { - const [elements, versionNonce] = useMutatedElements({ - appState: props.appState, - scene: props.scene, - }); +const CanvasesWrapper = ({ + appState, + scene, + children, +}: CanvasesWrapperProps) => { + const versionNonce = scene.getVersionNonce(); + const elements = useCanvasElements(appState, scene); + const visibleElements = useVisibleCanvasElements(appState, elements); - return
{props.children(elements, versionNonce)}
; + return
{children(versionNonce, elements, visibleElements)}
; }; export default CanvasesWrapper; diff --git a/src/components/canvases/InteractiveCanvas.tsx b/src/components/canvases/InteractiveCanvas.tsx index 774e56f25..cc499e307 100644 --- a/src/components/canvases/InteractiveCanvas.tsx +++ b/src/components/canvases/InteractiveCanvas.tsx @@ -14,8 +14,10 @@ import type { NonDeletedExcalidrawElement } from "../../element/types"; type InteractiveCanvasProps = { canvas: HTMLCanvasElement | null; elements: readonly NonDeletedExcalidrawElement[]; + visibleElements: readonly NonDeletedExcalidrawElement[]; versionNonce: number | undefined; selectionNonce: number | undefined; + scale: number; appState: InteractiveCanvasAppState; renderInteractiveSceneCallback: ( data: RenderInteractiveSceneCallback, @@ -83,9 +85,10 @@ const InteractiveCanvas = (props: InteractiveCanvasProps) => { renderInteractiveScene( { - scale: window.devicePixelRatio, - elements: props.elements, canvas: props.canvas, + elements: props.elements, + visibleElements: props.visibleElements, + scale: window.devicePixelRatio, appState: props.appState, renderConfig: { remotePointerViewportCoords: pointerViewportCoords, @@ -117,8 +120,8 @@ const InteractiveCanvas = (props: InteractiveCanvasProps) => { ? CURSOR_TYPE.GRAB : CURSOR_TYPE.AUTO, }} - width={props.appState.width * window.devicePixelRatio} - height={props.appState.height * window.devicePixelRatio} + width={props.appState.width * props.scale} + height={props.appState.height * props.scale} ref={props.handleCanvasRef} onContextMenu={props.onContextMenu} onPointerMove={props.onPointerMove} @@ -133,7 +136,7 @@ const InteractiveCanvas = (props: InteractiveCanvasProps) => { ); }; -const stripIrrelevantAppStateProps = ( +const getRelevantAppStateProps = ( appState: AppState, ): Omit => ({ zoom: appState.zoom, @@ -167,10 +170,11 @@ const areEqual = ( prevProps: InteractiveCanvasProps, nextProps: InteractiveCanvasProps, ) => { - // This could be further optimised if needed, as we don't have to render interactive canvas on each mutation + // This could be further optimised if needed, as we don't have to render interactive canvas on each scene mutation if ( prevProps.selectionNonce !== nextProps.selectionNonce || - prevProps.versionNonce !== nextProps.versionNonce + prevProps.versionNonce !== nextProps.versionNonce || + prevProps.scale !== nextProps.scale ) { return false; } @@ -179,8 +183,8 @@ const areEqual = ( return isShallowEqual( // asserting AppState because we're being passed the whole AppState // but resolve to only the InteractiveCanvas-relevant props - stripIrrelevantAppStateProps(prevProps.appState as AppState), - stripIrrelevantAppStateProps(nextProps.appState as AppState), + getRelevantAppStateProps(prevProps.appState as AppState), + getRelevantAppStateProps(nextProps.appState as AppState), ); }; diff --git a/src/components/canvases/StaticCanvas.tsx b/src/components/canvases/StaticCanvas.tsx index ebe3dc42c..2cd119d4c 100644 --- a/src/components/canvases/StaticCanvas.tsx +++ b/src/components/canvases/StaticCanvas.tsx @@ -10,8 +10,10 @@ type StaticCanvasProps = { canvas: HTMLCanvasElement | null; rc: RoughCanvas | null; elements: readonly NonDeletedExcalidrawElement[]; + visibleElements: readonly NonDeletedExcalidrawElement[]; versionNonce: number | undefined; selectionNonce: number | undefined; + scale: number; appState: StaticCanvasAppState; renderConfig: StaticCanvasRenderConfig; handleCanvasRef: (canvas: HTMLCanvasElement) => void; @@ -27,10 +29,11 @@ const StaticCanvas = (props: StaticCanvasProps) => { } renderStaticScene( { - scale: window.devicePixelRatio, - elements: props.elements, canvas: props.canvas, rc: props.rc!, + scale: props.scale, + elements: props.elements, + visibleElements: props.visibleElements, appState: props.appState, renderConfig: props.renderConfig, }, @@ -51,14 +54,14 @@ const StaticCanvas = (props: StaticCanvasProps) => { height: props.appState.height, pointerEvents: "none", }} - width={props.appState.width * window.devicePixelRatio} - height={props.appState.height * window.devicePixelRatio} + width={props.appState.width * props.scale} + height={props.appState.height * props.scale} ref={props.handleCanvasRef} /> ); }; -const stripIrrelevantAppStateProps = ( +const getRelevantAppStateProps = ( appState: AppState, ): Omit< StaticCanvasAppState, @@ -89,15 +92,18 @@ const areEqual = ( prevProps: StaticCanvasProps, nextProps: StaticCanvasProps, ) => { - if (prevProps.versionNonce !== nextProps.versionNonce) { + if ( + prevProps.versionNonce !== nextProps.versionNonce || + prevProps.scale !== nextProps.scale + ) { return false; } return isShallowEqual( // asserting AppState because we're being passed the whole AppState - // but resolve to only the InteractiveCanvas-relevant props - stripIrrelevantAppStateProps(prevProps.appState as AppState), - stripIrrelevantAppStateProps(nextProps.appState as AppState), + // but resolve to only the StaticCanvas-relevant props + getRelevantAppStateProps(prevProps.appState as AppState), + getRelevantAppStateProps(nextProps.appState as AppState), ); }; diff --git a/src/element/sizeHelpers.ts b/src/element/sizeHelpers.ts index b5810dbeb..abc51dc68 100644 --- a/src/element/sizeHelpers.ts +++ b/src/element/sizeHelpers.ts @@ -2,7 +2,9 @@ import { ExcalidrawElement } from "./types"; import { mutateElement } from "./mutateElement"; import { isFreeDrawElement, isLinearElement } from "./typeChecks"; import { SHIFT_LOCKING_ANGLE } from "../constants"; -import { AppState } from "../types"; +import { AppState, Zoom } from "../types"; +import { getElementBounds } from "./bounds"; +import { viewportCoordsToSceneCoords } from "../utils"; export const isInvisiblySmallElement = ( element: ExcalidrawElement, @@ -13,6 +15,42 @@ export const isInvisiblySmallElement = ( return element.width === 0 && element.height === 0; }; +export const isVisibleElement = ( + element: ExcalidrawElement, + width: number, + height: number, + viewTransformations: { + zoom: Zoom; + offsetLeft: number; + offsetTop: number; + scrollX: number; + scrollY: number; + }, +) => { + const [x1, y1, x2, y2] = getElementBounds(element); // scene coordinates + const topLeftSceneCoords = viewportCoordsToSceneCoords( + { + clientX: viewTransformations.offsetLeft, + clientY: viewTransformations.offsetTop, + }, + viewTransformations, + ); + const bottomRightSceneCoords = viewportCoordsToSceneCoords( + { + clientX: viewTransformations.offsetLeft + width, + clientY: viewTransformations.offsetTop + height, + }, + viewTransformations, + ); + + return ( + topLeftSceneCoords.x <= x2 && + topLeftSceneCoords.y <= y2 && + bottomRightSceneCoords.x >= x1 && + bottomRightSceneCoords.y >= y1 + ); +}; + /** * Makes a perfect shape or diagonal/horizontal/vertical line */ diff --git a/src/hooks/useMutatedElements.ts b/src/hooks/useMutatedElements.ts index cf03d74ba..bdc239a5b 100644 --- a/src/hooks/useMutatedElements.ts +++ b/src/hooks/useMutatedElements.ts @@ -1,17 +1,13 @@ import Scene from "../scene/Scene"; import { useMemo } from "react"; -import { InteractiveCanvasAppState, StaticCanvasAppState } from "../types"; import { isImageElement } from "../element/typeChecks"; import { NonDeletedExcalidrawElement } from "../element/types"; +import { CommonCanvasAppState } from "../types"; -export const useMutatedElements = ({ - appState, - scene, -}: { - appState: InteractiveCanvasAppState | StaticCanvasAppState; - scene: Scene; -}): [readonly NonDeletedExcalidrawElement[], number | undefined] => { - const versionNonce = scene.getVersionNonce(); +export const useCanvasElements = ( + appState: CommonCanvasAppState, + scene: Scene, +): readonly NonDeletedExcalidrawElement[] => { const nonDeletedElements = scene.getNonDeletedElements(); const elements = useMemo(() => { @@ -38,5 +34,5 @@ export const useMutatedElements = ({ appState.pendingImageElementId, ]); - return [elements, versionNonce]; + return elements; }; diff --git a/src/hooks/useVisibleElements.ts b/src/hooks/useVisibleElements.ts new file mode 100644 index 000000000..914415d2c --- /dev/null +++ b/src/hooks/useVisibleElements.ts @@ -0,0 +1,39 @@ +import { useMemo } from "react"; +import { CommonCanvasAppState } from "../types"; +import { NonDeletedExcalidrawElement } from "../element/types"; +import { isVisibleElement } from "../element/sizeHelpers"; + +export const useVisibleCanvasElements = ( + appState: CommonCanvasAppState, + elements: readonly NonDeletedExcalidrawElement[], +): readonly NonDeletedExcalidrawElement[] => { + const visibleElements = useMemo(() => { + const viewTransformations = { + zoom: appState.zoom, + offsetLeft: appState.offsetLeft, + offsetTop: appState.offsetTop, + scrollX: appState.scrollX, + scrollY: appState.scrollY, + }; + + return elements.filter((element) => + isVisibleElement( + element, + appState.width, + appState.height, + viewTransformations, + ), + ); + }, [ + appState.offsetLeft, + appState.offsetTop, + appState.scrollX, + appState.scrollY, + appState.height, + appState.width, + appState.zoom, + elements, + ]); + + return visibleElements; +}; diff --git a/src/renderer/renderScene.ts b/src/renderer/renderScene.ts index cb4cb58a4..379ebe0e3 100644 --- a/src/renderer/renderScene.ts +++ b/src/renderer/renderScene.ts @@ -6,7 +6,6 @@ import { StaticCanvasAppState, BinaryFiles, Point, - Zoom, CommonCanvasAppState, } from "../types"; import { @@ -23,7 +22,6 @@ import { OMIT_SIDES_FOR_MULTIPLE_ELEMENTS, getTransformHandlesFromCoords, getTransformHandles, - getElementBounds, getCommonBounds, } from "../element"; @@ -62,11 +60,7 @@ import { TransformHandles, TransformHandleType, } from "../element/transformHandles"; -import { - viewportCoordsToSceneCoords, - throttleRAF, - isOnlyExportingSingleFrame, -} from "../utils"; +import { throttleRAF, isOnlyExportingSingleFrame } from "../utils"; import { UserIdleState } from "../types"; import { FRAME_STYLE, THEME_FILTER } from "../constants"; import { @@ -418,10 +412,11 @@ const bootstrapCanvas = ({ }; const _renderInteractiveScene = ({ - elements, - appState, - scale, canvas, + elements, + visibleElements, + scale, + appState, renderConfig, }: InteractiveSceneRenderConfig) => { if (canvas === null) { @@ -444,17 +439,6 @@ const _renderInteractiveScene = ({ let editingLinearElement: NonDeleted | undefined = undefined; - // FIXME I: memo? - const visibleElements = elements.filter((element) => - isVisibleElement(element, normalizedWidth, normalizedHeight, { - zoom: appState.zoom, - offsetLeft: appState.offsetLeft, - offsetTop: appState.offsetTop, - scrollX: appState.scrollX, - scrollY: appState.scrollY, - }), - ); - visibleElements.forEach((element) => { // Getting the element using LinearElementEditor during collab mismatches version - being one head of visible elements due to // ShapeCache returns empty hence making sure that we get the @@ -867,11 +851,12 @@ const _renderInteractiveScene = ({ }; const _renderStaticScene = ({ - elements, - appState, - scale, - rc, canvas, + rc, + elements, + visibleElements, + scale, + appState, renderConfig, }: StaticSceneRenderConfig) => { if (canvas === null) { @@ -910,17 +895,6 @@ const _renderStaticScene = ({ ); } - // Paint visible elements - const visibleElements = elements.filter((element) => - isVisibleElement(element, normalizedWidth, normalizedHeight, { - zoom: appState.zoom, - offsetLeft: appState.offsetLeft, - offsetTop: appState.offsetTop, - scrollX: appState.scrollX, - scrollY: appState.scrollY, - }), - ); - const groupsToBeAddedToFrame = new Set(); visibleElements.forEach((element) => { @@ -937,6 +911,7 @@ const _renderStaticScene = ({ } }); + // Paint visible elements visibleElements.forEach((element) => { try { // - when exporting the whole canvas, we DO NOT apply clipping @@ -1353,42 +1328,6 @@ const renderLinkIcon = ( } }; -export const isVisibleElement = ( - element: ExcalidrawElement, - canvasWidth: number, - canvasHeight: number, - viewTransformations: { - zoom: Zoom; - offsetLeft: number; - offsetTop: number; - scrollX: number; - scrollY: number; - }, -) => { - const [x1, y1, x2, y2] = getElementBounds(element); // scene coordinates - const topLeftSceneCoords = viewportCoordsToSceneCoords( - { - clientX: viewTransformations.offsetLeft, - clientY: viewTransformations.offsetTop, - }, - viewTransformations, - ); - const bottomRightSceneCoords = viewportCoordsToSceneCoords( - { - clientX: viewTransformations.offsetLeft + canvasWidth, - clientY: viewTransformations.offsetTop + canvasHeight, - }, - viewTransformations, - ); - - return ( - topLeftSceneCoords.x <= x2 && - topLeftSceneCoords.y <= y2 && - bottomRightSceneCoords.x >= x1 && - bottomRightSceneCoords.y >= y1 - ); -}; - // This should be only called for exporting purposes export const renderSceneToSvg = ( elements: readonly NonDeletedExcalidrawElement[], diff --git a/src/scene/export.ts b/src/scene/export.ts index 985602289..e0304abcb 100644 --- a/src/scene/export.ts +++ b/src/scene/export.ts @@ -12,6 +12,7 @@ import { updateImageCache, } from "../element/image"; import Scene from "./Scene"; +import { isVisibleElement } from "../element/sizeHelpers"; export const SVG_EXPORT_TAG = ``; @@ -54,8 +55,29 @@ export const exportToCanvas = async ( const onlyExportingSingleFrame = isOnlyExportingSingleFrame(elements); + const viewTransformations = { + zoom: appState.zoom, + offsetLeft: appState.offsetLeft, + offsetTop: appState.offsetTop, + scrollX: appState.scrollX, + scrollY: appState.scrollY, + }; + + const visibleElements = elements.filter((element) => + isVisibleElement( + element, + appState.width, + appState.height, + viewTransformations, + ), + ); + renderStaticScene({ + canvas, + rc: rough.canvas(canvas), elements, + visibleElements, + scale, appState: { ...appState, scrollX: -minX + (onlyExportingSingleFrame ? 0 : exportPadding), @@ -64,9 +86,6 @@ export const exportToCanvas = async ( shouldCacheIgnoreZoom: false, theme: appState.exportWithDarkMode ? "dark" : "light", }, - scale, - rc: rough.canvas(canvas), - canvas, renderConfig: { imageCache, renderGrid: false, diff --git a/src/scene/types.ts b/src/scene/types.ts index 148773055..3065fa45c 100644 --- a/src/scene/types.ts +++ b/src/scene/types.ts @@ -40,17 +40,19 @@ export type RenderInteractiveSceneCallback = { }; export type StaticSceneRenderConfig = { - elements: readonly NonDeletedExcalidrawElement[]; - scale: number; canvas: HTMLCanvasElement | null; rc: RoughCanvas; + elements: readonly NonDeletedExcalidrawElement[]; + visibleElements: readonly NonDeletedExcalidrawElement[]; + scale: number; appState: StaticCanvasAppState; renderConfig: StaticCanvasRenderConfig; }; export type InteractiveSceneRenderConfig = { - elements: readonly NonDeletedExcalidrawElement[]; canvas: HTMLCanvasElement | null; + elements: readonly NonDeletedExcalidrawElement[]; + visibleElements: readonly NonDeletedExcalidrawElement[]; scale: number; appState: InteractiveCanvasAppState; renderConfig: InteractiveCanvasRenderConfig; diff --git a/src/tests/__snapshots__/linearElementEditor.test.tsx.snap b/src/tests/__snapshots__/linearElementEditor.test.tsx.snap index d1a73e72f..9522a00e7 100644 --- a/src/tests/__snapshots__/linearElementEditor.test.tsx.snap +++ b/src/tests/__snapshots__/linearElementEditor.test.tsx.snap @@ -5,7 +5,7 @@ exports[`Test Linear Elements Test bound text element should match styles for te class="excalidraw-wysiwyg" data-type="wysiwyg" dir="auto" - style="position: absolute; display: inline-block; min-height: 1em; margin: 0px; padding: 0px; border: 0px; outline: 0; resize: none; background: transparent; overflow: hidden; z-index: var(--zIndex-wysiwyg); word-break: break-word; white-space: pre-wrap; overflow-wrap: break-word; box-sizing: content-box; width: 10.5px; height: 25px; left: 35px; top: 7.5px; transform: translate(0px, 0px) scale(1) rotate(0deg); text-align: center; vertical-align: middle; color: rgb(30, 30, 30); opacity: 1; filter: var(--theme-filter); max-height: -7.5px; font: Emoji 20px 20px; line-height: 1.25; font-family: Virgil, Segoe UI Emoji;" + style="position: absolute; display: inline-block; min-height: 1em; margin: 0px; padding: 0px; border: 0px; outline: 0; resize: none; background: transparent; overflow: hidden; z-index: var(--zIndex-wysiwyg); word-break: break-word; white-space: pre-wrap; overflow-wrap: break-word; box-sizing: content-box; width: 10.5px; height: 25px; left: 35px; top: 7.5px; transform: translate(0px, 0px) scale(1) rotate(0deg); text-align: center; vertical-align: middle; color: rgb(30, 30, 30); opacity: 1; filter: var(--theme-filter); max-height: 992.5px; font: Emoji 20px 20px; line-height: 1.25; font-family: Virgil, Segoe UI Emoji;" tabindex="0" wrap="off" /> diff --git a/src/tests/linearElementEditor.test.tsx b/src/tests/linearElementEditor.test.tsx index 111deca0e..2d571b6cf 100644 --- a/src/tests/linearElementEditor.test.tsx +++ b/src/tests/linearElementEditor.test.tsx @@ -25,7 +25,6 @@ import { import * as textElementUtils from "../element/textElement"; import { ROUNDNESS, VERTICAL_ALIGN } from "../constants"; -// FIXME I: add specific tests for render of both components (when they should both, when just one, when just second, what is the order of renders - first static, then interactive, etc., all tests) const renderInteractiveScene = jest.spyOn(Renderer, "renderInteractiveScene"); const renderStaticScene = jest.spyOn(Renderer, "renderStaticScene"); @@ -34,7 +33,6 @@ const font = "20px Cascadia, width: Segoe UI Emoji" as FontString; describe("Test Linear Elements", () => { let container: HTMLElement; - let canvas: HTMLCanvasElement; let interactiveCanvas: HTMLCanvasElement; beforeEach(async () => { @@ -45,13 +43,10 @@ describe("Test Linear Elements", () => { renderStaticScene.mockClear(); reseed(7); const comp = await render(); + h.state.width = 1000; + h.state.height = 1000; container = comp.container; - canvas = container.querySelector("canvas.static")!; - canvas.width = 1000; - canvas.height = 1000; interactiveCanvas = container.querySelector("canvas.interactive")!; - interactiveCanvas.width = 1000; - interactiveCanvas.height = 1000; }); const p1: Point = [20, 20];