From 947f7b1d765917ce546de24a1338ab8c9d31e8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Mon, 8 Apr 2024 13:36:12 +0200 Subject: [PATCH] [culling] Improve setting of display none. (#3376) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small improvement for culling shapes. We now use reactor to do it. . Before: ![image](https://github.com/tldraw/tldraw/assets/2523721/7f791cdd-c0e2-4b92-84d1-8b071540de10) After: ![image](https://github.com/tldraw/tldraw/assets/2523721/ca2e2a9e-f9f6-48a8-936f-05a402c1e7a2) ### Change Type - [ ] `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 - [ ] `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 --- .../custom-renderer/CustomRenderer.tsx | 3 +- .../useRenderingShapesChange.ts | 6 +- packages/editor/api-report.md | 5 +- packages/editor/api/api.json | 71 +++++++++++++++--- .../src/lib/components/CulledShapes.tsx | 5 +- packages/editor/src/lib/components/Shape.tsx | 21 +++--- packages/editor/src/lib/editor/Editor.ts | 73 ++++++++++++------- packages/editor/src/lib/editor/getSvgJsx.tsx | 3 +- .../HelperButtons/BackToContent.tsx | 7 +- .../tldraw/src/test/renderingShapes.test.tsx | 56 ++++++++------ 10 files changed, 167 insertions(+), 83 deletions(-) diff --git a/apps/examples/src/examples/custom-renderer/CustomRenderer.tsx b/apps/examples/src/examples/custom-renderer/CustomRenderer.tsx index 7e20457ea..66cbe70a1 100644 --- a/apps/examples/src/examples/custom-renderer/CustomRenderer.tsx +++ b/apps/examples/src/examples/custom-renderer/CustomRenderer.tsx @@ -36,7 +36,8 @@ export function CustomRenderer() { const theme = getDefaultColorTheme({ isDarkMode: editor.user.getIsDarkMode() }) const currentPageId = editor.getCurrentPageId() - for (const { shape, maskedPageBounds, opacity } of renderingShapes) { + for (const { shape, opacity } of renderingShapes) { + const maskedPageBounds = editor.getShapeMaskedPageBounds(shape) if (!maskedPageBounds) continue ctx.save() diff --git a/apps/examples/src/examples/rendering-shape-changes/useRenderingShapesChange.ts b/apps/examples/src/examples/rendering-shape-changes/useRenderingShapesChange.ts index ae181b28d..39b6145f3 100644 --- a/apps/examples/src/examples/rendering-shape-changes/useRenderingShapesChange.ts +++ b/apps/examples/src/examples/rendering-shape-changes/useRenderingShapesChange.ts @@ -22,9 +22,11 @@ export function useChangedShapesReactor( if (!beforeInfo) { continue } else { - if (afterInfo.isCulled && !beforeInfo.isCulled) { + const isAfterCulled = editor.isShapeCulled(afterInfo.id) + const isBeforeCulled = editor.isShapeCulled(beforeInfo.id) + if (isAfterCulled && !isBeforeCulled) { culled.push(afterInfo.shape) - } else if (!afterInfo.isCulled && beforeInfo.isCulled) { + } else if (!isAfterCulled && isBeforeCulled) { restored.push(afterInfo.shape) } beforeToVisit.delete(beforeInfo) diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index 8366a85cd..6b5a73d95 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -720,8 +720,6 @@ export class Editor extends EventEmitter { index: number; backgroundIndex: number; opacity: number; - isCulled: boolean; - maskedPageBounds: Box | undefined; }[]; getSelectedShapeAtPoint(point: VecLike): TLShape | undefined; getSelectedShapeIds(): TLShapeId[]; @@ -782,8 +780,6 @@ export class Editor extends EventEmitter { index: number; backgroundIndex: number; opacity: number; - isCulled: boolean; - maskedPageBounds: Box | undefined; }[]; getViewportPageBounds(): Box; getViewportPageCenter(): Vec; @@ -821,6 +817,7 @@ export class Editor extends EventEmitter { margin?: number | undefined; hitInside?: boolean | undefined; }): boolean; + isShapeCulled(shape: TLShape | TLShapeId): boolean; isShapeInPage(shape: TLShape | TLShapeId, pageId?: TLPageId): boolean; isShapeOfType(shape: TLUnknownShape, type: T['type']): shape is T; // (undocumented) diff --git a/packages/editor/api/api.json b/packages/editor/api/api.json index eeb1401c2..a607d0b27 100644 --- a/packages/editor/api/api.json +++ b/packages/editor/api/api.json @@ -11955,16 +11955,7 @@ }, { "kind": "Content", - "text": ">;\n index: number;\n backgroundIndex: number;\n opacity: number;\n isCulled: boolean;\n maskedPageBounds: " - }, - { - "kind": "Reference", - "text": "Box", - "canonicalReference": "@tldraw/editor!Box:class" - }, - { - "kind": "Content", - "text": " | undefined;\n }[]" + "text": ">;\n index: number;\n backgroundIndex: number;\n opacity: number;\n }[]" }, { "kind": "Content", @@ -11974,7 +11965,7 @@ "isStatic": false, "returnTypeTokenRange": { "startIndex": 1, - "endIndex": 12 + "endIndex": 10 }, "releaseTag": "Public", "isProtected": false, @@ -14823,6 +14814,64 @@ "isAbstract": false, "name": "isPointInShape" }, + { + "kind": "Method", + "canonicalReference": "@tldraw/editor!Editor#isShapeCulled:member(1)", + "docComment": "/**\n * Get whether the shape is culled or not.\n *\n * @param shape - The shape (or shape id) to get the culled info for.\n *\n * @example\n * ```ts\n * editor.isShapeCulled(myShape)\n * editor.isShapeCulled(myShapeId)\n * ```\n *\n * @public\n */\n", + "excerptTokens": [ + { + "kind": "Content", + "text": "isShapeCulled(shape: " + }, + { + "kind": "Reference", + "text": "TLShape", + "canonicalReference": "@tldraw/tlschema!TLShape:type" + }, + { + "kind": "Content", + "text": " | " + }, + { + "kind": "Reference", + "text": "TLShapeId", + "canonicalReference": "@tldraw/tlschema!TLShapeId:type" + }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Content", + "text": "boolean" + }, + { + "kind": "Content", + "text": ";" + } + ], + "isStatic": false, + "returnTypeTokenRange": { + "startIndex": 5, + "endIndex": 6 + }, + "releaseTag": "Public", + "isProtected": false, + "overloadIndex": 1, + "parameters": [ + { + "parameterName": "shape", + "parameterTypeTokenRange": { + "startIndex": 1, + "endIndex": 4 + }, + "isOptional": false + } + ], + "isOptional": false, + "isAbstract": false, + "name": "isShapeCulled" + }, { "kind": "Method", "canonicalReference": "@tldraw/editor!Editor#isShapeInPage:member(1)", diff --git a/packages/editor/src/lib/components/CulledShapes.tsx b/packages/editor/src/lib/components/CulledShapes.tsx index 5a6020621..9b2851534 100644 --- a/packages/editor/src/lib/components/CulledShapes.tsx +++ b/packages/editor/src/lib/components/CulledShapes.tsx @@ -112,8 +112,9 @@ export function CulledShapes() { const shapeVertices = computed('shape vertices', function calculateCulledShapeVertices() { const results: number[] = [] - for (const { isCulled, maskedPageBounds } of editor.getRenderingShapes()) { - if (isCulled && maskedPageBounds) { + for (const { id } of editor.getUnorderedRenderingShapes(true)) { + const maskedPageBounds = editor.getShapeMaskedPageBounds(id) + if (editor.isShapeCulled(id) && maskedPageBounds) { results.push( // triangle 1 maskedPageBounds.minX, diff --git a/packages/editor/src/lib/components/Shape.tsx b/packages/editor/src/lib/components/Shape.tsx index 19a33b64a..8e21a66ed 100644 --- a/packages/editor/src/lib/components/Shape.tsx +++ b/packages/editor/src/lib/components/Shape.tsx @@ -1,6 +1,6 @@ import { useQuickReactor, useStateTracking } from '@tldraw/state' import { TLShape, TLShapeId } from '@tldraw/tlschema' -import { memo, useCallback, useLayoutEffect, useRef } from 'react' +import { memo, useCallback, useRef } from 'react' import { ShapeUtil } from '../editor/shapes/ShapeUtil' import { useEditor } from '../hooks/useEditor' import { useEditorComponents } from '../hooks/useEditorComponents' @@ -26,7 +26,6 @@ export const Shape = memo(function Shape({ index, backgroundIndex, opacity, - isCulled, dprMultiple, }: { id: TLShapeId @@ -35,7 +34,6 @@ export const Shape = memo(function Shape({ index: number backgroundIndex: number opacity: number - isCulled: boolean dprMultiple: number }) { const editor = useEditor() @@ -120,13 +118,18 @@ export const Shape = memo(function Shape({ [opacity, index, backgroundIndex] ) - useLayoutEffect(() => { - const container = containerRef.current - const bgContainer = bgContainerRef.current - setStyleProperty(container, 'display', isCulled ? 'none' : 'block') - setStyleProperty(bgContainer, 'display', isCulled ? 'none' : 'block') - }, [isCulled]) + useQuickReactor( + 'set display', + () => { + const shape = editor.getShape(id) + if (!shape) return // probably the shape was just deleted + const isCulled = editor.isShapeCulled(shape) + setStyleProperty(containerRef.current, 'display', isCulled ? 'none' : 'block') + setStyleProperty(bgContainerRef.current, 'display', isCulled ? 'none' : 'block') + }, + [editor] + ) const annotateError = useCallback( (error: any) => editor.annotateError(error, { origin: 'shape', willCrashApp: false }), [editor] diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 7c635e66f..695471389 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -3126,48 +3126,26 @@ export class Editor extends EventEmitter { index: number backgroundIndex: number opacity: number - isCulled: boolean - maskedPageBounds: Box | undefined }[] = [] let nextIndex = MAX_SHAPES_PER_PAGE * 2 let nextBackgroundIndex = MAX_SHAPES_PER_PAGE - // We only really need these if we're using editor state, but that's ok - const editingShapeId = this.getEditingShapeId() - const selectedShapeIds = this.getSelectedShapeIds() const erasingShapeIds = this.getErasingShapeIds() - const renderingBoundsExpanded = this.getRenderingBoundsExpanded() - - // If renderingBoundsMargin is set to Infinity, then we won't cull offscreen shapes - const isCullingOffScreenShapes = Number.isFinite(this.renderingBoundsMargin) const addShapeById = (id: TLShapeId, opacity: number, isAncestorErasing: boolean) => { const shape = this.getShape(id) if (!shape) return opacity *= shape.opacity - let isCulled = false let isShapeErasing = false const util = this.getShapeUtil(shape) - const maskedPageBounds = this.getShapeMaskedPageBounds(id) if (useEditorState) { isShapeErasing = !isAncestorErasing && erasingShapeIds.includes(id) if (isShapeErasing) { opacity *= 0.32 } - - isCulled = - isCullingOffScreenShapes && - // never cull editingg shapes - editingShapeId !== id && - // if the shape is fully outside of its parent's clipping bounds... - (maskedPageBounds === undefined || - // ...or if the shape is outside of the expanded viewport bounds... - (!renderingBoundsExpanded.includes(maskedPageBounds) && - // ...and if it's not selected... then cull it - !selectedShapeIds.includes(id))) } renderingShapes.push({ @@ -3177,8 +3155,6 @@ export class Editor extends EventEmitter { index: nextIndex, backgroundIndex: nextBackgroundIndex, opacity, - isCulled, - maskedPageBounds, }) nextIndex += 1 @@ -4266,6 +4242,51 @@ export class Editor extends EventEmitter { return this.isShapeOrAncestorLocked(this.getShapeParent(shape)) } + @computed + private _getShapeCullingInfoCache(): ComputedCache { + return this.store.createComputedCache( + 'shapeCullingInfo', + ({ id }) => { + // We don't cull shapes that are being edited + if (this.getEditingShapeId() === id) return false + + const maskedPageBounds = this.getShapeMaskedPageBounds(id) + // if the shape is fully outside of its parent's clipping bounds... + if (maskedPageBounds === undefined) return true + + // We don't cull selected shapes + if (this.getSelectedShapeIds().includes(id)) return false + const renderingBoundsExpanded = this.getRenderingBoundsExpanded() + // the shape is outside of the expanded viewport bounds... + return !renderingBoundsExpanded.includes(maskedPageBounds) + }, + (a, b) => this.getShapeMaskedPageBounds(a) === this.getShapeMaskedPageBounds(b) + ) + } + + /** + * Get whether the shape is culled or not. + * + * @example + * ```ts + * editor.isShapeCulled(myShape) + * editor.isShapeCulled(myShapeId) + * ``` + * + * @param shape - The shape (or shape id) to get the culled info for. + * + * @public + */ + isShapeCulled(shape: TLShape | TLShapeId): boolean { + // If renderingBoundsMargin is set to Infinity, then we won't cull offscreen shapes + const isCullingOffScreenShapes = Number.isFinite(this.renderingBoundsMargin) + if (!isCullingOffScreenShapes) return false + + const id = typeof shape === 'string' ? shape : shape.id + + return this._getShapeCullingInfoCache().get(id)! as boolean + } + /** * The bounds of the current page (the common bounds of all of the shapes on the page). * @@ -4637,8 +4658,8 @@ export class Editor extends EventEmitter { * @public */ @computed getCurrentPageRenderingShapesSorted(): TLShape[] { - return this.getRenderingShapes() - .filter(({ isCulled }) => !isCulled) + return this.getUnorderedRenderingShapes(true) + .filter(({ id }) => !this.isShapeCulled(id)) .sort((a, b) => a.index - b.index) .map(({ shape }) => shape) } diff --git a/packages/editor/src/lib/editor/getSvgJsx.tsx b/packages/editor/src/lib/editor/getSvgJsx.tsx index 34965135c..1f16b0ce1 100644 --- a/packages/editor/src/lib/editor/getSvgJsx.tsx +++ b/packages/editor/src/lib/editor/getSvgJsx.tsx @@ -38,7 +38,8 @@ export async function getSvgJsx( if (opts.bounds) { bbox = opts.bounds } else { - for (const { maskedPageBounds } of renderingShapes) { + for (const { id } of renderingShapes) { + const maskedPageBounds = editor.getShapeMaskedPageBounds(id) if (!maskedPageBounds) continue if (bbox) { bbox.union(maskedPageBounds) diff --git a/packages/tldraw/src/lib/ui/components/HelperButtons/BackToContent.tsx b/packages/tldraw/src/lib/ui/components/HelperButtons/BackToContent.tsx index 89bfd2660..004dedfdf 100644 --- a/packages/tldraw/src/lib/ui/components/HelperButtons/BackToContent.tsx +++ b/packages/tldraw/src/lib/ui/components/HelperButtons/BackToContent.tsx @@ -19,9 +19,10 @@ export function BackToContent() { // 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) - ) + const visibleShapes = renderingShapes.filter((s) => { + const maskedPageBounds = editor.getShapeMaskedPageBounds(s.id) + return maskedPageBounds && renderingBounds.includes(maskedPageBounds) + }) const showBackToContentNow = visibleShapes.length === 0 && editor.getCurrentPageShapes().length > 0 diff --git a/packages/tldraw/src/test/renderingShapes.test.tsx b/packages/tldraw/src/test/renderingShapes.test.tsx index 322dfc34f..9b7c2ebbd 100644 --- a/packages/tldraw/src/test/renderingShapes.test.tsx +++ b/packages/tldraw/src/test/renderingShapes.test.tsx @@ -63,42 +63,50 @@ it('updates the rendering viewport when the camera stops moving', () => { it('lists shapes in viewport', () => { const ids = createShapes() editor.selectNone() - expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([ - [ids.A, false], // A is within the expanded rendering bounds, so should not be culled; and it's in the regular viewport too, so it's on screen. - [ids.B, false], - [ids.C, false], - [ids.D, true], // D is clipped and so should always be culled / outside of viewport - ]) + expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual( + [ + [ids.A, false], // A is within the expanded rendering bounds, so should not be culled; and it's in the regular viewport too, so it's on screen. + [ids.B, false], + [ids.C, false], + [ids.D, true], // D is clipped and so should always be culled / outside of viewport + ] + ) // Move the camera 201 pixels to the right and 201 pixels down editor.pan({ x: -201, y: -201 }) jest.advanceTimersByTime(500) - expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([ - [ids.A, false], // A should not be culled, even though it's no longer in the viewport (because it's still in the EXPANDED viewport) - [ids.B, false], - [ids.C, false], - [ids.D, true], // D is clipped and so should always be culled / outside of viewport - ]) + expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual( + [ + [ids.A, false], // A should not be culled, even though it's no longer in the viewport (because it's still in the EXPANDED viewport) + [ids.B, false], + [ids.C, false], + [ids.D, true], // D is clipped and so should always be culled / outside of viewport + ] + ) editor.pan({ x: -100, y: -100 }) jest.advanceTimersByTime(500) - expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([ - [ids.A, true], // A should be culled now that it's outside of the expanded viewport too - [ids.B, false], - [ids.C, false], - [ids.D, true], // D is clipped and so should always be culled, even if it's in the viewport - ]) + expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual( + [ + [ids.A, true], // A should be culled now that it's outside of the expanded viewport too + [ids.B, false], + [ids.C, false], + [ids.D, true], // D is clipped and so should always be culled, even if it's in the viewport + ] + ) editor.pan({ x: -900, y: -900 }) jest.advanceTimersByTime(500) - expect(editor.getRenderingShapes().map(({ id, isCulled }) => [id, isCulled])).toStrictEqual([ - [ids.A, true], - [ids.B, true], - [ids.C, true], - [ids.D, true], - ]) + expect(editor.getRenderingShapes().map(({ id }) => [id, editor.isShapeCulled(id)])).toStrictEqual( + [ + [ids.A, true], + [ids.B, true], + [ids.C, true], + [ids.D, true], + ] + ) }) it('lists shapes in viewport sorted by id with correct indexes & background indexes', () => {