From 4801b35768108b0569b054e762b5b12c9f488d83 Mon Sep 17 00:00:00 2001 From: Steve Ruiz Date: Sun, 17 Mar 2024 21:37:37 +0000 Subject: [PATCH] [tinyish] Simplify / skip some work in Shape (#3176) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is a minor cleanup of the Shape component. Here we: - use some dumb memoized info to avoid unnecessary style changes - move the dpr check up out of the shapes themselves, avoiding renders on instance state changes Culled shapes: - move the props setting on the culled shape component to a layout reactor - no longer set the height / width on the culled shape component - no longer update the culled shape component when the shape changes Random: - move the arrow shape defs to the arrow shape util (using that neat API we didn't used to have) ### Change Type - [x] `sdk` — Changes the tldraw SDK - [ ] `dotcom` — Changes the tldraw.com web app - [ ] `docs` — Changes to the documentation, examples, or templates. - [ ] `vs code` — Changes to the vscode plugin - [ ] `internal` — Does not affect user-facing stuff - [ ] `bugfix` — Bug fix - [ ] `feature` — New feature - [x] `improvement` — Improving existing features - [ ] `chore` — Updating dependencies, other boring stuff - [ ] `galaxy brain` — Architectural changes - [ ] `tests` — Changes to any test code - [ ] `tools` — Changes to infrastructure, CI, internal scripts, debugging tools, etc. - [ ] `dunno` — I don't know ### Test Plan 1. Use shapes 2. Use culled shapes ### Release Notes - SDK: minor improvements to the Shape component --- packages/editor/editor.css | 2 + packages/editor/src/lib/components/Shape.tsx | 141 ++++++++++-------- .../default-components/DefaultCanvas.tsx | 100 +++++++------ packages/editor/src/lib/utils/dom.ts | 10 ++ .../src/lib/shapes/arrow/ArrowShapeUtil.tsx | 29 +++- 5 files changed, 170 insertions(+), 112 deletions(-) diff --git a/packages/editor/editor.css b/packages/editor/editor.css index 78730becc..d2626a3a6 100644 --- a/packages/editor/editor.css +++ b/packages/editor/editor.css @@ -332,6 +332,8 @@ input, .tl-shape__culled { position: relative; background-color: var(--color-culled); + width: 100%; + height: 100%; } /* ---------------- Shape Containers ---------------- */ diff --git a/packages/editor/src/lib/components/Shape.tsx b/packages/editor/src/lib/components/Shape.tsx index 6afa14aee..0823f49a4 100644 --- a/packages/editor/src/lib/components/Shape.tsx +++ b/packages/editor/src/lib/components/Shape.tsx @@ -1,26 +1,27 @@ -import { track, useLayoutReaction, useStateTracking } from '@tldraw/state' +import { useLayoutReaction, useStateTracking } from '@tldraw/state' +import { IdOf } from '@tldraw/store' import { TLShape, TLShapeId } from '@tldraw/tlschema' -import * as React from 'react' +import { memo, useCallback, useLayoutEffect, useRef } from 'react' import { ShapeUtil } from '../editor/shapes/ShapeUtil' import { useEditor } from '../hooks/useEditor' import { useEditorComponents } from '../hooks/useEditorComponents' import { Mat } from '../primitives/Mat' import { toDomPrecision } from '../primitives/utils' -import { nearestMultiple } from '../utils/nearestMultiple' +import { setStyleProperty } from '../utils/dom' import { OptionalErrorBoundary } from './ErrorBoundary' /* This component renders shapes on the canvas. There are two stages: positioning and styling the shape's container using CSS, and then rendering the shape's JSX using its shape util's render method. Rendering the "inside" of a shape is -more expensive than positioning it or changing its color, so we use React.memo +more expensive than positioning it or changing its color, so we use memo to wrap the inner shape and only re-render it when the shape's props change. The shape also receives props for its index and opacity. The index is used to determine the z-index of the shape, and the opacity is used to set the shape's opacity based on its own opacity and that of its parent's. */ -export const Shape = track(function Shape({ +export const Shape = memo(function Shape({ id, shape, util, @@ -28,6 +29,7 @@ export const Shape = track(function Shape({ backgroundIndex, opacity, isCulled, + dprMultiple, }: { id: TLShapeId shape: TLShape @@ -36,56 +38,79 @@ export const Shape = track(function Shape({ backgroundIndex: number opacity: number isCulled: boolean + dprMultiple: number }) { const editor = useEditor() const { ShapeErrorFallback } = useEditorComponents() - const containerRef = React.useRef(null) - const backgroundContainerRef = React.useRef(null) + const containerRef = useRef(null) + const bgContainerRef = useRef(null) - const setProperty = React.useCallback((property: string, value: string) => { - containerRef.current?.style.setProperty(property, value) - backgroundContainerRef.current?.style.setProperty(property, value) - }, []) + const memoizedStuffRef = useRef({ + transform: '', + clipPath: 'none', + width: 0, + height: 0, + }) useLayoutReaction('set shape stuff', () => { const shape = editor.getShape(id) if (!shape) return // probably the shape was just deleted - const pageTransform = editor.getShapePageTransform(id) - const transform = Mat.toCssString(pageTransform) - setProperty('transform', transform) + const prev = memoizedStuffRef.current - const clipPath = editor.getShapeClipPath(id) - setProperty('clip-path', clipPath ?? 'none') + // Clip path + const clipPath = editor.getShapeClipPath(id) ?? 'none' + if (clipPath !== prev.clipPath) { + setStyleProperty(containerRef.current, 'clip-path', clipPath) + setStyleProperty(bgContainerRef.current, 'clip-path', clipPath) + prev.clipPath = clipPath + } + // Page transform + const transform = Mat.toCssString(editor.getShapePageTransform(id)) + if (transform !== prev.transform) { + setStyleProperty(containerRef.current, 'transform', transform) + setStyleProperty(bgContainerRef.current, 'transform', transform) + prev.transform = transform + } + + // Width / Height + // We round the shape width and height up to the nearest multiple of dprMultiple + // to avoid the browser making miscalculations when applying the transform. const bounds = editor.getShapeGeometry(shape).bounds - const dpr = Math.floor(editor.getInstanceState().devicePixelRatio * 100) / 100 - // dprMultiple is the smallest number we can multiply dpr by to get an integer - // it's usually 1, 2, or 4 (for e.g. dpr of 2, 2.5 and 2.25 respectively) - const dprMultiple = nearestMultiple(dpr) - // We round the shape width and height up to the nearest multiple of dprMultiple to avoid the browser - // making miscalculations when applying the transform. const widthRemainder = bounds.w % dprMultiple - const width = widthRemainder === 0 ? bounds.w : bounds.w + (dprMultiple - widthRemainder) const heightRemainder = bounds.h % dprMultiple + const width = widthRemainder === 0 ? bounds.w : bounds.w + (dprMultiple - widthRemainder) const height = heightRemainder === 0 ? bounds.h : bounds.h + (dprMultiple - heightRemainder) - setProperty('width', Math.max(width, dprMultiple) + 'px') - setProperty('height', Math.max(height, dprMultiple) + 'px') + + if (width !== prev.width || height !== prev.height) { + setStyleProperty(containerRef.current, 'width', Math.max(width, dprMultiple) + 'px') + setStyleProperty(containerRef.current, 'height', Math.max(height, dprMultiple) + 'px') + setStyleProperty(bgContainerRef.current, 'width', Math.max(width, dprMultiple) + 'px') + setStyleProperty(bgContainerRef.current, 'height', Math.max(height, dprMultiple) + 'px') + prev.width = width + prev.height = height + } }) - // Set the opacity of the container when the opacity changes - React.useLayoutEffect(() => { - setProperty('opacity', opacity + '') - containerRef.current?.style.setProperty('z-index', index + '') - backgroundContainerRef.current?.style.setProperty('z-index', backgroundIndex + '') - }, [opacity, index, backgroundIndex, setProperty]) + // This stuff changes pretty infrequently, so we can change them together + useLayoutEffect(() => { + const container = containerRef.current + const bgContainer = bgContainerRef.current - const annotateError = React.useCallback( - (error: any) => { - editor.annotateError(error, { origin: 'react.shape', willCrashApp: false }) - }, + // Opacity + setStyleProperty(container, 'opacity', opacity) + setStyleProperty(bgContainer, 'opacity', opacity) + + // Z-Index + setStyleProperty(container, 'z-index', index) + setStyleProperty(bgContainer, 'z-index', backgroundIndex) + }, [opacity, index, backgroundIndex]) + + const annotateError = useCallback( + (error: any) => editor.annotateError(error, { origin: 'shape', willCrashApp: false }), [editor] ) @@ -95,12 +120,12 @@ export const Shape = track(function Shape({ <> {util.backgroundComponent && (
- {!isCulled && ( + {isCulled ? null : ( @@ -109,7 +134,7 @@ export const Shape = track(function Shape({ )}
{isCulled ? ( - + ) : ( @@ -120,17 +145,14 @@ export const Shape = track(function Shape({ ) }) -const InnerShape = React.memo( +const InnerShape = memo( function InnerShape({ shape, util }: { shape: T; util: ShapeUtil }) { return useStateTracking('InnerShape:' + shape.type, () => util.component(shape)) }, - (prev, next) => - prev.shape.props === next.shape.props && - prev.shape.meta === next.shape.meta && - prev.util === next.util + (prev, next) => prev.shape.props === next.shape.props && prev.shape.meta === next.shape.meta ) -const InnerShapeBackground = React.memo( +const InnerShapeBackground = memo( function InnerShapeBackground({ shape, util, @@ -143,23 +165,18 @@ const InnerShapeBackground = React.memo( (prev, next) => prev.shape.props === next.shape.props && prev.shape.meta === next.shape.meta ) -const CulledShape = React.memo( - function CulledShape({ shape }: { shape: T }) { - const editor = useEditor() - const bounds = editor.getShapeGeometry(shape).bounds +const CulledShape = function CulledShape({ shapeId }: { shapeId: IdOf }) { + const editor = useEditor() + const culledRef = useRef(null) - return ( -
+ useLayoutReaction('set shape stuff', () => { + const bounds = editor.getShapeGeometry(shapeId).bounds + setStyleProperty( + culledRef.current, + 'transform', + `translate(${toDomPrecision(bounds.minX)}px, ${toDomPrecision(bounds.minY)}px)` ) - }, - () => true -) + }) + + return
+} diff --git a/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx b/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx index 2f4df4ef5..f0990e938 100644 --- a/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx +++ b/packages/editor/src/lib/components/default-components/DefaultCanvas.tsx @@ -1,8 +1,8 @@ -import { react, track, useLayoutReaction, useValue } from '@tldraw/state' +import { react, useLayoutReaction, useValue } from '@tldraw/state' import { TLHandle, TLShapeId } from '@tldraw/tlschema' import { dedupe, modulate, objectMapValues } from '@tldraw/utils' import classNames from 'classnames' -import React from 'react' +import { Fragment, JSX, useEffect, useRef, useState } from 'react' import { COARSE_HANDLE_RADIUS, HANDLE_RADIUS } from '../../constants' import { useCanvasEvents } from '../../hooks/useCanvasEvents' import { useCoarsePointer } from '../../hooks/useCoarsePointer' @@ -17,6 +17,8 @@ import { Mat } from '../../primitives/Mat' import { Vec } from '../../primitives/Vec' import { toDomPrecision } from '../../primitives/utils' import { debugFlags } from '../../utils/debug-flags' +import { setStyleProperty } from '../../utils/dom' +import { nearestMultiple } from '../../utils/nearestMultiple' import { GeometryDebuggingView } from '../GeometryDebuggingView' import { LiveCollaborators } from '../LiveCollaborators' import { Shape } from '../Shape' @@ -30,9 +32,9 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) { const { Background, SvgDefs } = useEditorComponents() - const rCanvas = React.useRef(null) - const rHtmlLayer = React.useRef(null) - const rHtmlLayer2 = React.useRef(null) + const rCanvas = useRef(null) + const rHtmlLayer = useRef(null) + const rHtmlLayer2 = useRef(null) useScreenBounds(rCanvas) useDocumentEvents() @@ -42,11 +44,6 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) { useFixSafariDoubleTapZoomPencilEvents(rCanvas) useLayoutReaction('position layers', () => { - const htmlElm = rHtmlLayer.current - if (!htmlElm) return - const htmlElm2 = rHtmlLayer2.current - if (!htmlElm2) return - const { x, y, z } = editor.getCamera() // Because the html container has a width/height of 1px, we @@ -58,8 +55,8 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) { const transform = `scale(${toDomPrecision(z)}) translate(${toDomPrecision( x + offset )}px,${toDomPrecision(y + offset)}px)` - htmlElm.style.setProperty('transform', transform) - htmlElm2.style.setProperty('transform', transform) + setStyleProperty(rHtmlLayer.current, 'transform', transform) + setStyleProperty(rHtmlLayer2.current, 'transform', transform) }) const events = useCanvasEvents() @@ -67,7 +64,7 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) { const shapeSvgDefs = useValue( 'shapeSvgDefs', () => { - const shapeSvgDefsByKey = new Map() + const shapeSvgDefsByKey = new Map() for (const util of objectMapValues(editor.shapeUtils)) { if (!util) return const defs = util.getCanvasSvgDefs() @@ -98,10 +95,8 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) { {shapeSvgDefs} - {Cursor && } - - - + + {SvgDefs && } @@ -330,13 +325,22 @@ function ShapesWithSVGs() { const renderingShapes = useValue('rendering shapes', () => editor.getRenderingShapes(), [editor]) + const dprMultiple = useValue( + 'dpr multiple', + () => + // dprMultiple is the smallest number we can multiply dpr by to get an integer + // it's usually 1, 2, or 4 (for e.g. dpr of 2, 2.5 and 2.25 respectively) + nearestMultiple(Math.floor(editor.getInstanceState().devicePixelRatio * 100) / 100), + [editor] + ) + return ( <> {renderingShapes.map((result) => ( - - + + - + ))} ) @@ -347,10 +351,19 @@ function ShapesToDisplay() { const renderingShapes = useValue('rendering shapes', () => editor.getRenderingShapes(), [editor]) + const dprMultiple = useValue( + 'dpr multiple', + () => + // dprMultiple is the smallest number we can multiply dpr by to get an integer + // it's usually 1, 2, or 4 (for e.g. dpr of 2, 2.5 and 2.25 respectively) + nearestMultiple(Math.floor(editor.getInstanceState().devicePixelRatio * 100) / 100), + [editor] + ) + return ( <> {renderingShapes.map((result) => ( - + ))} ) @@ -420,11 +433,11 @@ const HoveredShapeIndicator = function HoveredShapeIndicator() { return } -const HintedShapeIndicator = track(function HintedShapeIndicator() { +function HintedShapeIndicator() { const editor = useEditor() const { ShapeIndicator } = useEditorComponents() - const ids = dedupe(editor.getHintingShapeIds()) + const ids = useValue('hinting shape ids', () => dedupe(editor.getHintingShapeIds()), [editor]) if (!ids.length) return null if (!ShapeIndicator) return null @@ -436,9 +449,9 @@ const HintedShapeIndicator = track(function HintedShapeIndicator() { ))} ) -}) +} -function Cursor() { +function CursorDef() { return ( @@ -457,36 +470,25 @@ function Cursor() { ) } -function CollaboratorHint() { +function CollaboratorHintDef() { return } -function ArrowheadDot() { - return ( - - - - ) -} - -function ArrowheadCross() { - return ( - - - - - ) -} - -const DebugSvgCopy = track(function DupSvg({ id }: { id: TLShapeId }) { +function DebugSvgCopy({ id }: { id: TLShapeId }) { const editor = useEditor() - const shape = editor.getShape(id) - const [html, setHtml] = React.useState('') + const [html, setHtml] = useState('') - const isInRoot = shape?.parentId === editor.getCurrentPageId() + const isInRoot = useValue( + 'is in root', + () => { + const shape = editor.getShape(id) + return shape?.parentId === editor.getCurrentPageId() + }, + [editor, id] + ) - React.useEffect(() => { + useEffect(() => { if (!isInRoot) return let latest = null @@ -520,7 +522,7 @@ const DebugSvgCopy = track(function DupSvg({ id }: { id: TLShapeId }) {
) -}) +} function SelectionForegroundWrapper() { const editor = useEditor() diff --git a/packages/editor/src/lib/utils/dom.ts b/packages/editor/src/lib/utils/dom.ts index 309701dff..78b3f062c 100644 --- a/packages/editor/src/lib/utils/dom.ts +++ b/packages/editor/src/lib/utils/dom.ts @@ -80,3 +80,13 @@ export function releasePointerCapture( /** @public */ export const stopEventPropagation = (e: any) => e.stopPropagation() + +/** @internal */ +export const setStyleProperty = ( + elm: HTMLElement | null, + property: string, + value: string | number +) => { + if (!elm) return + elm.style.setProperty(property, value as string) +} diff --git a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx index cce97a310..8f63fcacf 100644 --- a/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx +++ b/packages/tldraw/src/lib/shapes/arrow/ArrowShapeUtil.tsx @@ -1014,7 +1014,17 @@ export class ArrowShapeUtil extends ShapeUtil { } override getCanvasSvgDefs(): TLShapeUtilCanvasSvgDef[] { - return [getFillDefForCanvas()] + return [ + getFillDefForCanvas(), + { + key: `arrow:dot`, + component: ArrowheadDotDef, + }, + { + key: `arrow:cross`, + component: ArrowheadCrossDef, + }, + ] } } @@ -1082,3 +1092,20 @@ const shapeAtTranslationStart = new WeakMap< > } >() + +function ArrowheadDotDef() { + return ( + + + + ) +} + +function ArrowheadCrossDef() { + return ( + + + + + ) +}