[tinyish] Simplify / skip some work in Shape (#3176)

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
2024-03-17 21:37:37 +00:00 · 2024-03-17 21:37:37 +00:00 · 4801b35768
commit 4801b35768
--- 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 ---------------- */
--- 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<HTMLDivElement>(null)
-	const backgroundContainerRef = React.useRef<HTMLDivElement>(null)
+	const containerRef = useRef<HTMLDivElement>(null)
+	const bgContainerRef = useRef<HTMLDivElement>(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 && (
 				<div
-					ref={backgroundContainerRef}
+					ref={bgContainerRef}
 					className="tl-shape tl-shape-background"
 					data-shape-type={shape.type}
 					draggable={false}
 				>
-					{!isCulled && (
+					{isCulled ? null : (
 						<OptionalErrorBoundary fallback={ShapeErrorFallback} onError={annotateError}>
 							<InnerShapeBackground shape={shape} util={util} />
 						</OptionalErrorBoundary>
@ -109,7 +134,7 @@ export const Shape = track(function Shape({
 			)}
 			<div ref={containerRef} className="tl-shape" data-shape-type={shape.type} draggable={false}>
 				{isCulled ? (
-					<CulledShape shape={shape} />
+					<CulledShape shapeId={shape.id} />
 				) : (
 					<OptionalErrorBoundary fallback={ShapeErrorFallback as any} onError={annotateError}>
 						<InnerShape shape={shape} util={util} />
@ -120,17 +145,14 @@ export const Shape = track(function Shape({
 	)
 })

-const InnerShape = React.memo(
+const InnerShape = memo(
 	function InnerShape<T extends TLShape>({ shape, util }: { shape: T; util: ShapeUtil<T> }) {
 		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<T extends TLShape>({
 		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<T extends TLShape>({ shape }: { shape: T }) {
-		const editor = useEditor()
-		const bounds = editor.getShapeGeometry(shape).bounds
+const CulledShape = function CulledShape<T extends TLShape>({ shapeId }: { shapeId: IdOf<T> }) {
+	const editor = useEditor()
+	const culledRef = useRef<HTMLDivElement>(null)

-		return (
-			<div
-				className="tl-shape__culled"
-				style={{
-					transform: `translate(${toDomPrecision(bounds.minX)}px, ${toDomPrecision(
-						bounds.minY
-					)}px)`,
-					width: Math.max(1, toDomPrecision(bounds.width)),
-					height: Math.max(1, toDomPrecision(bounds.height)),
-				}}
-			/>
+	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 <div ref={culledRef} className="tl-shape__culled" />
+}
--- 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<HTMLDivElement>(null)
-	const rHtmlLayer = React.useRef<HTMLDivElement>(null)
-	const rHtmlLayer2 = React.useRef<HTMLDivElement>(null)
+	const rCanvas = useRef<HTMLDivElement>(null)
+	const rHtmlLayer = useRef<HTMLDivElement>(null)
+	const rHtmlLayer2 = useRef<HTMLDivElement>(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<string, React.JSX.Element>()
+			const shapeSvgDefsByKey = new Map<string, JSX.Element>()
 			for (const util of objectMapValues(editor.shapeUtils)) {
 				if (!util) return
 				const defs = util.getCanvasSvgDefs()
@ -98,10 +95,8 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) {
 			<svg className="tl-svg-context">
 				<defs>
 					{shapeSvgDefs}
-					{Cursor && <Cursor />}
-					<CollaboratorHint />
-					<ArrowheadDot />
-					<ArrowheadCross />
+					<CursorDef />
+					<CollaboratorHintDef />
 					{SvgDefs && <SvgDefs />}
 				</defs>
 			</svg>
@ -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) => (
-				<React.Fragment key={result.id + '_fragment'}>
-					<Shape {...result} />
+				<Fragment key={result.id + '_fragment'}>
+					<Shape {...result} dprMultiple={dprMultiple} />
 					<DebugSvgCopy id={result.id} />
-				</React.Fragment>
+				</Fragment>
 			))}
 		</>
 	)
@ -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) => (
-				<Shape key={result.id + '_shape'} {...result} />
+				<Shape key={result.id + '_shape'} {...result} dprMultiple={dprMultiple} />
 			))}
 		</>
 	)
@ -420,11 +433,11 @@ const HoveredShapeIndicator = function HoveredShapeIndicator() {
 	return <HoveredShapeIndicator shapeId={hoveredShapeId} />
 }

-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 (
 		<g id="cursor">
 			<g fill="rgba(0,0,0,.2)" transform="translate(-11,-11)">
@ -457,36 +470,25 @@ function Cursor() {
 	)
 }

-function CollaboratorHint() {
+function CollaboratorHintDef() {
 	return <path id="cursor_hint" fill="currentColor" d="M -2,-5 2,0 -2,5 Z" />
 }

-function ArrowheadDot() {
-	return (
-		<marker id="arrowhead-dot" className="tl-arrow-hint" refX="3.0" refY="3.0" orient="0">
-			<circle cx="3" cy="3" r="2" strokeDasharray="100%" />
-		</marker>
-	)
-}
-
-function ArrowheadCross() {
-	return (
-		<marker id="arrowhead-cross" className="tl-arrow-hint" refX="3.0" refY="3.0" orient="auto">
-			<line x1="1.5" y1="1.5" x2="4.5" y2="4.5" strokeDasharray="100%" />
-			<line x1="1.5" y1="4.5" x2="4.5" y2="1.5" strokeDasharray="100%" />
-		</marker>
-	)
-}
-
-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 }) {
 			<div style={{ display: 'flex' }} dangerouslySetInnerHTML={{ __html: html }} />
 		</div>
 	)
-})
+}

 function SelectionForegroundWrapper() {
 	const editor = useEditor()
--- 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)
+}
--- 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<TLArrowShape> {
 	}

 	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 (
+		<marker id="arrowhead-dot" className="tl-arrow-hint" refX="3.0" refY="3.0" orient="0">
+			<circle cx="3" cy="3" r="2" strokeDasharray="100%" />
+		</marker>
+	)
+}
+
+function ArrowheadCrossDef() {
+	return (
+		<marker id="arrowhead-cross" className="tl-arrow-hint" refX="3.0" refY="3.0" orient="auto">
+			<line x1="1.5" y1="1.5" x2="4.5" y2="4.5" strokeDasharray="100%" />
+			<line x1="1.5" y1="4.5" x2="4.5" y2="1.5" strokeDasharray="100%" />
+		</marker>
+	)
+}