Display none for culled shapes (#3291)

Comparing different culling optimizations:


https://github.com/tldraw/tldraw/assets/2523721/0b3b8b42-ed70-45b7-bf83-41023c36a563

I think we should go with the `display: none` + showing the skeleteon.
The way it works is:
- We now add a sibling to the shape wrapper div which serves as the
skeleton for the culled shapes.
- Only one of the two divs (shape wrapper and skeleton div) is
displayed. The other one is using `display: none` to improve
performance.

### Change Type

<!--  Please select a 'Scope' label ️ -->

- [ ] `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
- [x] `internal` — Does not affect user-facing stuff

<!--  Please select a 'Type' label ️ -->

- [ ] `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


- Improve performance of culled shapes by using `display: none`.

---------

Co-authored-by: Steve Ruiz <steveruizok@gmail.com>
This commit is contained in:
Mitja Bezenšek 2024-04-05 15:23:02 +02:00 committed by GitHub
parent 4a494a2eaf
commit f1e0af7631
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 43 additions and 176 deletions

View file

@ -1623,7 +1623,6 @@ export abstract class ShapeUtil<Shape extends TLUnknownShape = TLUnknownShape> {
canResize: TLShapeUtilFlag<Shape>;
canScroll: TLShapeUtilFlag<Shape>;
canSnap: TLShapeUtilFlag<Shape>;
canUnmount: TLShapeUtilFlag<Shape>;
abstract component(shape: Shape): any;
// (undocumented)
editor: Editor;

View file

@ -30760,41 +30760,6 @@
"isProtected": false,
"isAbstract": false
},
{
"kind": "Property",
"canonicalReference": "@tldraw/editor!ShapeUtil#canUnmount:member",
"docComment": "/**\n * Whether the shape should unmount when not visible in the editor. Consider keeping this to false if the shape's `component` has local state.\n *\n * @public\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "canUnmount: "
},
{
"kind": "Reference",
"text": "TLShapeUtilFlag",
"canonicalReference": "@tldraw/editor!TLShapeUtilFlag:type"
},
{
"kind": "Content",
"text": "<Shape>"
},
{
"kind": "Content",
"text": ";"
}
],
"isReadonly": false,
"isOptional": false,
"releaseTag": "Public",
"name": "canUnmount",
"propertyTypeTokenRange": {
"startIndex": 1,
"endIndex": 3
},
"isStatic": false,
"isProtected": false,
"isAbstract": false
},
{
"kind": "Method",
"canonicalReference": "@tldraw/editor!ShapeUtil#component:member(1)",

View file

@ -338,10 +338,13 @@ input,
}
.tl-shape__culled {
position: relative;
position: absolute;
pointer-events: none;
overflow: visible;
transform-origin: top left;
contain: size layout;
background-color: var(--color-culled);
width: 100%;
height: 100%;
z-index: 0;
}
/* ---------------- Shape Containers ---------------- */

View file

@ -1,12 +1,10 @@
import { useQuickReactor, useStateTracking } from '@tldraw/state'
import { IdOf } from '@tldraw/store'
import { TLShape, TLShapeId } from '@tldraw/tlschema'
import { memo, useCallback, useRef } 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 { setStyleProperty } from '../utils/dom'
import { OptionalErrorBoundary } from './ErrorBoundary'
@ -45,6 +43,7 @@ export const Shape = memo(function Shape({
const { ShapeErrorFallback } = useEditorComponents()
const containerRef = useRef<HTMLDivElement>(null)
const culledContainerRef = useRef<HTMLDivElement>(null)
const bgContainerRef = useRef<HTMLDivElement>(null)
const memoizedStuffRef = useRef({
@ -52,6 +51,8 @@ export const Shape = memo(function Shape({
clipPath: 'none',
width: 0,
height: 0,
x: 0,
y: 0,
})
useQuickReactor(
@ -66,22 +67,31 @@ export const Shape = memo(function Shape({
const clipPath = editor.getShapeClipPath(id) ?? 'none'
if (clipPath !== prev.clipPath) {
setStyleProperty(containerRef.current, 'clip-path', clipPath)
setStyleProperty(culledContainerRef.current, 'clip-path', clipPath)
setStyleProperty(bgContainerRef.current, 'clip-path', clipPath)
prev.clipPath = clipPath
}
// Page transform
const transform = Mat.toCssString(editor.getShapePageTransform(id))
const pageTransform = editor.getShapePageTransform(id)
const transform = Mat.toCssString(pageTransform)
const bounds = editor.getShapeGeometry(shape).bounds
// Update if the tranform has changed
if (transform !== prev.transform) {
setStyleProperty(containerRef.current, 'transform', transform)
setStyleProperty(bgContainerRef.current, 'transform', transform)
setStyleProperty(
culledContainerRef.current,
'transform',
`${Mat.toCssString(pageTransform)} translate(${bounds.x}px, ${bounds.y}px)`
)
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 widthRemainder = bounds.w % dprMultiple
const heightRemainder = bounds.h % dprMultiple
const width = widthRemainder === 0 ? bounds.w : bounds.w + (dprMultiple - widthRemainder)
@ -90,6 +100,8 @@ export const Shape = memo(function Shape({
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(culledContainerRef.current, 'width', Math.max(width, dprMultiple) + 'px')
setStyleProperty(culledContainerRef.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
@ -117,6 +129,15 @@ export const Shape = memo(function Shape({
[opacity, index, backgroundIndex]
)
useLayoutEffect(() => {
const container = containerRef.current
const bgContainer = bgContainerRef.current
const culledContainer = culledContainerRef.current
setStyleProperty(container, 'display', isCulled ? 'none' : 'block')
setStyleProperty(bgContainer, 'display', isCulled ? 'none' : 'block')
setStyleProperty(culledContainer, 'display', isCulled ? 'block' : 'none')
}, [isCulled])
const annotateError = useCallback(
(error: any) => editor.annotateError(error, { origin: 'shape', willCrashApp: false }),
[editor]
@ -126,6 +147,7 @@ export const Shape = memo(function Shape({
return (
<>
<div ref={culledContainerRef} className="tl-shape__culled" draggable={false} />
{util.backgroundComponent && (
<div
ref={bgContainerRef}
@ -133,21 +155,15 @@ export const Shape = memo(function Shape({
data-shape-type={shape.type}
draggable={false}
>
{isCulled ? null : (
<OptionalErrorBoundary fallback={ShapeErrorFallback} onError={annotateError}>
<InnerShapeBackground shape={shape} util={util} />
</OptionalErrorBoundary>
)}
<OptionalErrorBoundary fallback={ShapeErrorFallback} onError={annotateError}>
<InnerShapeBackground shape={shape} util={util} />
</OptionalErrorBoundary>
</div>
)}
<div ref={containerRef} className="tl-shape" data-shape-type={shape.type} draggable={false}>
{isCulled ? (
<CulledShape shapeId={shape.id} />
) : (
<OptionalErrorBoundary fallback={ShapeErrorFallback as any} onError={annotateError}>
<InnerShape shape={shape} util={util} />
</OptionalErrorBoundary>
)}
<OptionalErrorBoundary fallback={ShapeErrorFallback as any} onError={annotateError}>
<InnerShape shape={shape} util={util} />
</OptionalErrorBoundary>
</div>
</>
)
@ -172,23 +188,3 @@ const InnerShapeBackground = memo(
},
(prev, next) => prev.shape.props === next.shape.props && prev.shape.meta === next.shape.meta
)
const CulledShape = function CulledShape<T extends TLShape>({ shapeId }: { shapeId: IdOf<T> }) {
const editor = useEditor()
const culledRef = useRef<HTMLDivElement>(null)
useQuickReactor(
'set shape stuff',
() => {
const bounds = editor.getShapeGeometry(shapeId).bounds
setStyleProperty(
culledRef.current,
'transform',
`translate(${toDomPrecision(bounds.minX)}px, ${toDomPrecision(bounds.minY)}px)`
)
},
[editor]
)
return <div ref={culledRef} className="tl-shape__culled" />
}

View file

@ -3160,8 +3160,6 @@ export class Editor extends EventEmitter<TLEventMap> {
isCulled =
isCullingOffScreenShapes &&
// only cull shapes that allow unmounting, i.e. not stateful components
util.canUnmount(shape) &&
// never cull editingg shapes
editingShapeId !== id &&
// if the shape is fully outside of its parent's clipping bounds...

View file

@ -89,13 +89,6 @@ export abstract class ShapeUtil<Shape extends TLUnknownShape = TLUnknownShape> {
*/
canScroll: TLShapeUtilFlag<Shape> = () => false
/**
* Whether the shape should unmount when not visible in the editor. Consider keeping this to false if the shape's `component` has local state.
*
* @public
*/
canUnmount: TLShapeUtilFlag<Shape> = () => true
/**
* Whether the shape can be bound to by an arrow.
*

View file

@ -536,8 +536,6 @@ export class EmbedShapeUtil extends BaseBoxShapeUtil<TLEmbedShape> {
// (undocumented)
canResize: (shape: TLEmbedShape) => boolean;
// (undocumented)
canUnmount: TLShapeUtilFlag<TLEmbedShape>;
// (undocumented)
component(shape: TLEmbedShape): JSX_2.Element;
// (undocumented)
getDefaultProps(): TLEmbedShape['props'];

View file

@ -5705,50 +5705,6 @@
"isProtected": false,
"isAbstract": false
},
{
"kind": "Property",
"canonicalReference": "tldraw!EmbedShapeUtil#canUnmount:member",
"docComment": "",
"excerptTokens": [
{
"kind": "Content",
"text": "canUnmount: "
},
{
"kind": "Reference",
"text": "TLShapeUtilFlag",
"canonicalReference": "@tldraw/editor!TLShapeUtilFlag:type"
},
{
"kind": "Content",
"text": "<"
},
{
"kind": "Reference",
"text": "TLEmbedShape",
"canonicalReference": "@tldraw/tlschema!TLEmbedShape:type"
},
{
"kind": "Content",
"text": ">"
},
{
"kind": "Content",
"text": ";"
}
],
"isReadonly": false,
"isOptional": false,
"releaseTag": "Public",
"name": "canUnmount",
"propertyTypeTokenRange": {
"startIndex": 1,
"endIndex": 5
},
"isStatic": false,
"isProtected": false,
"isAbstract": false
},
{
"kind": "Method",
"canonicalReference": "tldraw!EmbedShapeUtil#component:member(1)",

View file

@ -34,9 +34,6 @@ export class EmbedShapeUtil extends BaseBoxShapeUtil<TLEmbedShape> {
override hideSelectionBoundsFg: TLShapeUtilFlag<TLEmbedShape> = (shape) => !this.canResize(shape)
override canEdit: TLShapeUtilFlag<TLEmbedShape> = () => true
override canUnmount: TLShapeUtilFlag<TLEmbedShape> = (shape: TLEmbedShape) => {
return !!getEmbedInfo(shape.props.url)?.definition?.canUnmount
}
override canResize = (shape: TLEmbedShape) => {
return !!getEmbedInfo(shape.props.url)?.definition?.doesResize
}

View file

@ -17,10 +17,8 @@ export function BackToContent() {
const renderingShapes = editor.getRenderingShapes()
const renderingBounds = editor.getRenderingBounds()
// renderingShapes will also include shapes that have the canUnmount flag
// set to true. These shapes will be on the canvas but may not be in the
// viewport... so we also need to narrow down the list to only shapes that
// are ALSO in the viewport.
// Rendering shapes includes all the shapes in the current page.
// We have to filter them down to just the shapes that are inside the renderingBounds.
const visibleShapes = renderingShapes.filter(
(s) => s.maskedPageBounds && renderingBounds.includes(s.maskedPageBounds)
)

View file

@ -218,7 +218,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: true;
readonly overridePermissions: {
readonly 'allow-top-navigation': true;
};
@ -231,7 +230,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: true;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -241,7 +239,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -253,7 +250,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -265,7 +261,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -277,7 +272,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 520;
readonly height: 400;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -287,7 +281,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 520;
readonly height: 400;
readonly doesResize: false;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -297,7 +290,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 800;
readonly height: 450;
readonly doesResize: true;
readonly canUnmount: false;
readonly overridePermissions: {
readonly 'allow-presentation': true;
};
@ -313,7 +305,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly minWidth: 460;
readonly minHeight: 360;
readonly doesResize: true;
readonly canUnmount: false;
readonly instructionLink: "https://support.google.com/calendar/answer/41207?hl=en";
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
@ -326,7 +317,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly minWidth: 460;
readonly minHeight: 360;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -336,7 +326,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: true;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -346,7 +335,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -356,7 +344,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -368,7 +355,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly minHeight: 500;
readonly overrideOutlineRadius: 12;
readonly doesResize: true;
readonly canUnmount: false;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
}, {
@ -378,7 +364,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 640;
readonly height: 360;
readonly doesResize: true;
readonly canUnmount: false;
readonly isAspectRatioLocked: true;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
@ -389,7 +374,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly isAspectRatioLocked: true;
readonly toEmbedUrl: (url: string) => string | undefined;
readonly fromEmbedUrl: (url: string) => string | undefined;
@ -400,7 +384,6 @@ export const EMBED_DEFINITIONS: readonly [{
readonly width: 720;
readonly height: 500;
readonly doesResize: true;
readonly canUnmount: false;
readonly isAspectRatioLocked: false;
readonly backgroundColor: "#fff";
readonly toEmbedUrl: (url: string) => string | undefined;
@ -417,7 +400,6 @@ export type EmbedDefinition = {
readonly width: number;
readonly height: number;
readonly doesResize: boolean;
readonly canUnmount: boolean;
readonly isAspectRatioLocked?: boolean;
readonly overridePermissions?: TLEmbedShapePermissions;
readonly instructionLink?: string;

File diff suppressed because one or more lines are too long

View file

@ -24,7 +24,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: true,
overridePermissions: {
'allow-top-navigation': true,
},
@ -50,7 +49,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: true,
toEmbedUrl: (url) => {
if (
!!url.match(
@ -81,7 +79,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
if (url.includes('/maps/')) {
const match = url.match(/@(.*),(.*),(.*)z/)
@ -120,7 +117,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
// e.g. extract "steveruizok.mathFact" from https://www.val.town/v/steveruizok.mathFact
@ -149,7 +145,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
const matches = urlObj && urlObj.pathname.match(/\/s\/([^/]+)\/?/)
@ -176,7 +171,6 @@ export const EMBED_DEFINITIONS = [
width: 520,
height: 400,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const CODEPEN_URL_REGEXP = /https:\/\/codepen.io\/([^/]+)\/pen\/([^/]+)/
const matches = url.match(CODEPEN_URL_REGEXP)
@ -203,7 +197,6 @@ export const EMBED_DEFINITIONS = [
width: 520,
height: 400,
doesResize: false,
canUnmount: false,
toEmbedUrl: (url) => {
const SCRATCH_URL_REGEXP = /https?:\/\/scratch.mit.edu\/projects\/([^/]+)/
const matches = url.match(SCRATCH_URL_REGEXP)
@ -230,7 +223,6 @@ export const EMBED_DEFINITIONS = [
width: 800,
height: 450,
doesResize: true,
canUnmount: false,
overridePermissions: {
'allow-presentation': true,
},
@ -275,7 +267,6 @@ export const EMBED_DEFINITIONS = [
minWidth: 460,
minHeight: 360,
doesResize: true,
canUnmount: false,
instructionLink: 'https://support.google.com/calendar/answer/41207?hl=en',
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
@ -318,7 +309,6 @@ export const EMBED_DEFINITIONS = [
minWidth: 460,
minHeight: 360,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
@ -353,7 +343,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: true,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
if (urlObj && urlObj.pathname.match(/\/([^/]+)\/([^/]+)/)) {
@ -378,7 +367,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
if (urlObj && urlObj.pathname.match(/\/@([^/]+)\/([^/]+)/)) {
@ -406,7 +394,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
if (urlObj && urlObj.pathname.match(/^\/map\//)) {
@ -432,7 +419,6 @@ export const EMBED_DEFINITIONS = [
minHeight: 500,
overrideOutlineRadius: 12,
doesResize: true,
canUnmount: false,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
if (urlObj && urlObj.pathname.match(/^\/(artist|album)\//)) {
@ -455,7 +441,6 @@ export const EMBED_DEFINITIONS = [
width: 640,
height: 360,
doesResize: true,
canUnmount: false,
isAspectRatioLocked: true,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
@ -486,7 +471,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
isAspectRatioLocked: true,
toEmbedUrl: (url) => {
const urlObj = safeParseUrl(url)
@ -510,7 +494,6 @@ export const EMBED_DEFINITIONS = [
width: 720,
height: 500,
doesResize: true,
canUnmount: false,
isAspectRatioLocked: false,
backgroundColor: '#fff',
toEmbedUrl: (url) => {
@ -619,7 +602,6 @@ export type EmbedDefinition = {
readonly width: number
readonly height: number
readonly doesResize: boolean
readonly canUnmount: boolean
readonly isAspectRatioLocked?: boolean
readonly overridePermissions?: TLEmbedShapePermissions
readonly instructionLink?: string