From 45dffd1af682e0b3378ba08a99b63ac82908117b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Mon, 15 Apr 2024 18:28:18 +0200 Subject: [PATCH] RBush again? (#3439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds RBush to handle spatial querying. We use it for: - Culling. Helps a lot with panning as we don't have to compute the culled shapes from scratch. Instead we just query rbush again. It makes culling quite granular: spatial index updates when shapes change (additions, removals, changes to bounds), visible shapes depends on that, but also updates when the viewport page bound change, culled shapes then depend on that but also change with selections changes. The api stayed the same, which is great since the fuzz tests can stay as they are. - Brushing - Erasing - Scribble brushing - Getting shapes at point (for example, when updating the hover id) This improves performance of all of those operations. I might have missed some places where this might also be useful. ### Erasing before (Test on my old ipad) https://github.com/tldraw/tldraw/assets/2523721/edb9c004-a44a-4779-b2d0-98617b057314 ### Erasing after https://github.com/tldraw/tldraw/assets/2523721/8f8367fd-fa8e-4963-ba13-720c5f0c2da5 ### Creating an arrow before https://github.com/tldraw/tldraw/assets/2523721/4068f8b7-f7b8-4826-83f2-083b1f3783bc ### After (much better, but still bad) https://github.com/tldraw/tldraw/assets/2523721/11af6be6-01d8-4740-bf15-896e2dd31dd6 ### 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 --------- Co-authored-by: Steve Ruiz --- packages/editor/api-report.md | 4 + packages/editor/api/api.json | 174 ++++++++++++++++++ packages/editor/package.json | 4 +- packages/editor/src/lib/editor/Editor.ts | 51 ++++- .../editor/derivations/notVisibleShapes.ts | 105 ----------- .../lib/editor/derivations/spatialIndex.ts | 172 +++++++++++++++++ packages/editor/src/lib/primitives/Box.ts | 4 + .../tools/EraserTool/childStates/Erasing.ts | 13 +- .../tools/SelectTool/childStates/Brushing.ts | 11 +- .../childStates/ScribbleBrushing.ts | 7 +- .../tldraw/src/test/getCulledShapes.test.tsx | 12 +- .../tldraw/src/test/selection-omnibus.test.ts | 6 +- packages/tldraw/src/test/spatialIndex.test.ts | 114 ++++++++++++ yarn.lock | 25 +++ 14 files changed, 570 insertions(+), 132 deletions(-) delete mode 100644 packages/editor/src/lib/editor/derivations/notVisibleShapes.ts create mode 100644 packages/editor/src/lib/editor/derivations/spatialIndex.ts create mode 100644 packages/tldraw/src/test/spatialIndex.test.ts diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index 1aa6fda13..f6ce49953 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -188,6 +188,8 @@ export interface BoundsSnapPoint { export class Box { constructor(x?: number, y?: number, w?: number, h?: number); // (undocumented) + static AroundPoint(point: VecLike, n: number): Box; + // (undocumented) get aspectRatio(): number; // (undocumented) get center(): Vec; @@ -755,6 +757,7 @@ export class Editor extends EventEmitter { getShapeClipPath(shape: TLShape | TLShapeId): string | undefined; getShapeGeometry(shape: TLShape | TLShapeId): T; getShapeHandles(shape: T | T['id']): TLHandle[] | undefined; + getShapeIdsInsideBounds(bounds: Box): TLShapeId[]; getShapeLocalTransform(shape: TLShape | TLShapeId): Mat; getShapeMask(shape: TLShape | TLShapeId): undefined | VecLike[]; getShapeMaskedPageBounds(shape: TLShape | TLShapeId): Box | undefined; @@ -766,6 +769,7 @@ export class Editor extends EventEmitter { hitInside?: boolean | undefined; margin?: number | undefined; }): TLShape[]; + getShapesInsideBounds(bounds: Box): TLShape[]; // (undocumented) getShapeStyleIfExists(shape: TLShape, style: StyleProp): T | undefined; getShapeUtil(shape: S | TLShapePartial): ShapeUtil; diff --git a/packages/editor/api/api.json b/packages/editor/api/api.json index a8272a9ed..45d17be4a 100644 --- a/packages/editor/api/api.json +++ b/packages/editor/api/api.json @@ -1623,6 +1623,72 @@ } ] }, + { + "kind": "Method", + "canonicalReference": "@tldraw/editor!Box.AroundPoint:member(1)", + "docComment": "", + "excerptTokens": [ + { + "kind": "Content", + "text": "static AroundPoint(point: " + }, + { + "kind": "Reference", + "text": "VecLike", + "canonicalReference": "@tldraw/editor!VecLike:type" + }, + { + "kind": "Content", + "text": ", n: " + }, + { + "kind": "Content", + "text": "number" + }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Reference", + "text": "Box", + "canonicalReference": "@tldraw/editor!Box:class" + }, + { + "kind": "Content", + "text": ";" + } + ], + "isStatic": true, + "returnTypeTokenRange": { + "startIndex": 5, + "endIndex": 6 + }, + "releaseTag": "Public", + "isProtected": false, + "overloadIndex": 1, + "parameters": [ + { + "parameterName": "point", + "parameterTypeTokenRange": { + "startIndex": 1, + "endIndex": 2 + }, + "isOptional": false + }, + { + "parameterName": "n", + "parameterTypeTokenRange": { + "startIndex": 3, + "endIndex": 4 + }, + "isOptional": false + } + ], + "isOptional": false, + "isAbstract": false, + "name": "AroundPoint" + }, { "kind": "Property", "canonicalReference": "@tldraw/editor!Box#aspectRatio:member", @@ -12734,6 +12800,60 @@ "isAbstract": false, "name": "getShapeHandles" }, + { + "kind": "Method", + "canonicalReference": "@tldraw/editor!Editor#getShapeIdsInsideBounds:member(1)", + "docComment": "/**\n * Get the shapes ids of shapes that are are (at least partially) inside the bounds.\n *\n * @param bounds - The bounds to check.\n *\n * @returns The shape ids of shapes that are at least partially inside the bounds.\n *\n * @public\n */\n", + "excerptTokens": [ + { + "kind": "Content", + "text": "getShapeIdsInsideBounds(bounds: " + }, + { + "kind": "Reference", + "text": "Box", + "canonicalReference": "@tldraw/editor!Box:class" + }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Reference", + "text": "TLShapeId", + "canonicalReference": "@tldraw/tlschema!TLShapeId:type" + }, + { + "kind": "Content", + "text": "[]" + }, + { + "kind": "Content", + "text": ";" + } + ], + "isStatic": false, + "returnTypeTokenRange": { + "startIndex": 3, + "endIndex": 5 + }, + "releaseTag": "Public", + "isProtected": false, + "overloadIndex": 1, + "parameters": [ + { + "parameterName": "bounds", + "parameterTypeTokenRange": { + "startIndex": 1, + "endIndex": 2 + }, + "isOptional": false + } + ], + "isOptional": false, + "isAbstract": false, + "name": "getShapeIdsInsideBounds" + }, { "kind": "Method", "canonicalReference": "@tldraw/editor!Editor#getShapeLocalTransform:member(1)", @@ -13237,6 +13357,60 @@ "isAbstract": false, "name": "getShapesAtPoint" }, + { + "kind": "Method", + "canonicalReference": "@tldraw/editor!Editor#getShapesInsideBounds:member(1)", + "docComment": "/**\n * Get the shapes that are are (at least partially) inside the bounds.\n *\n * @param bounds - The bounds to check.\n *\n * @returns The shapes that are at least partially inside the bounds.\n *\n * @public\n */\n", + "excerptTokens": [ + { + "kind": "Content", + "text": "getShapesInsideBounds(bounds: " + }, + { + "kind": "Reference", + "text": "Box", + "canonicalReference": "@tldraw/editor!Box:class" + }, + { + "kind": "Content", + "text": "): " + }, + { + "kind": "Reference", + "text": "TLShape", + "canonicalReference": "@tldraw/tlschema!TLShape:type" + }, + { + "kind": "Content", + "text": "[]" + }, + { + "kind": "Content", + "text": ";" + } + ], + "isStatic": false, + "returnTypeTokenRange": { + "startIndex": 3, + "endIndex": 5 + }, + "releaseTag": "Public", + "isProtected": false, + "overloadIndex": 1, + "parameters": [ + { + "parameterName": "bounds", + "parameterTypeTokenRange": { + "startIndex": 1, + "endIndex": 2 + }, + "isOptional": false + } + ], + "isOptional": false, + "isAbstract": false, + "name": "getShapesInsideBounds" + }, { "kind": "Method", "canonicalReference": "@tldraw/editor!Editor#getShapeStyleIfExists:member(1)", diff --git a/packages/editor/package.json b/packages/editor/package.json index 6b5d134aa..4b28dea1d 100644 --- a/packages/editor/package.json +++ b/packages/editor/package.json @@ -59,7 +59,8 @@ "is-plain-object": "^5.0.0", "lodash.throttle": "^4.1.1", "lodash.uniq": "^4.5.0", - "nanoid": "4.0.2" + "nanoid": "4.0.2", + "rbush": "^3.0.1" }, "peerDependencies": { "react": "^18", @@ -72,6 +73,7 @@ "@types/benchmark": "^2.1.2", "@types/lodash.throttle": "^4.1.7", "@types/lodash.uniq": "^4.5.7", + "@types/rbush": "^3.0.3", "@types/react-test-renderer": "^18.0.0", "@types/wicg-file-system-access": "^2020.9.5", "benchmark": "^2.1.4", diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 2dcc3f2f8..54ba3c210 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -100,9 +100,9 @@ import { getReorderingShapesChanges } from '../utils/reorderShapes' import { applyRotationToSnapshotShapes, getRotationSnapshot } from '../utils/rotation' import { uniqueId } from '../utils/uniqueId' import { arrowBindingsIndex } from './derivations/arrowBindingsIndex' -import { notVisibleShapes } from './derivations/notVisibleShapes' import { parentsToChildren } from './derivations/parentsToChildren' import { deriveShapeIdsInCurrentPage } from './derivations/shapeIdsInCurrentPage' +import { SpatialIndex } from './derivations/spatialIndex' import { getSvgJsx } from './getSvgJsx' import { ClickManager } from './managers/ClickManager' import { EnvironmentManager } from './managers/EnvironmentManager' @@ -589,6 +589,7 @@ export class Editor extends EventEmitter { ) this._parentIdsToChildIds = parentsToChildren(this.store) + this._spatialIndex = new SpatialIndex(this) this.disposables.add( this.store.listen((changes) => { this.emit('change', changes) @@ -4198,10 +4199,8 @@ export class Editor extends EventEmitter { return this.isShapeOrAncestorLocked(this.getShapeParent(shape)) } - @computed - private _notVisibleShapes() { - return notVisibleShapes(this) - } + /* @internal */ + private readonly _spatialIndex: SpatialIndex /** * Get culled shapes. @@ -4210,7 +4209,7 @@ export class Editor extends EventEmitter { */ @computed getCulledShapes() { - const notVisibleShapes = this._notVisibleShapes().get() + const notVisibleShapes = this._spatialIndex.getNotVisibleShapes() const selectedShapeIds = this.getSelectedShapeIds() const editingId = this.getEditingShapeId() const culledShapes = new Set(notVisibleShapes) @@ -4225,6 +4224,31 @@ export class Editor extends EventEmitter { return culledShapes } + /** + * Get the shapes ids of shapes that are are (at least partially) inside the bounds. + * + * @param bounds - The bounds to check. + * @returns The shape ids of shapes that are at least partially inside the bounds. + * + * @public + */ + getShapeIdsInsideBounds(bounds: Box): TLShapeId[] { + return this._spatialIndex.getShapeIdsInsideBounds(bounds) + } + + /** + * Get the shapes that are are (at least partially) inside the bounds. + * + * @param bounds - The bounds to check. + * @returns The shapes that are at least partially inside the bounds. + * + * @public + */ + getShapesInsideBounds(bounds: Box): TLShape[] { + const shapeIds = this.getShapeIdsInsideBounds(bounds) + return compact(shapeIds.map((id) => this.getShape(id))) + } + /** * The bounds of the current page (the common bounds of all of the shapes on the page). * @@ -4254,9 +4278,18 @@ export class Editor extends EventEmitter { * @returns The top-most selected shape at the given point, or undefined if there is no shape at the point. */ getSelectedShapeAtPoint(point: VecLike): TLShape | undefined { + const shapesCloseToPoint = new Set( + this.getShapeIdsInsideBounds(Box.AroundPoint(point, HIT_TEST_MARGIN)) + ) const selectedShapeIds = this.getSelectedShapeIds() + return this.getCurrentPageShapesSorted() - .filter((shape) => shape.type !== 'group' && selectedShapeIds.includes(shape.id)) + .filter( + (shape) => + shape.type !== 'group' && + shapesCloseToPoint.has(shape.id) && + selectedShapeIds.includes(shape.id) + ) .reverse() // findlast .find((shape) => this.isPointInShape(shape, point, { hitInside: true, margin: 0 })) } @@ -4296,11 +4329,15 @@ export class Editor extends EventEmitter { let inMarginClosestToEdgeDistance = Infinity let inMarginClosestToEdgeHit: TLShape | null = null + const shapesCloseToPoint = new Set( + this.getShapeIdsInsideBounds(Box.AroundPoint(point, HIT_TEST_MARGIN)) + ) const shapesToCheck = ( opts.renderingOnly ? this.getCurrentPageRenderingShapesSorted() : this.getCurrentPageShapesSorted() ).filter((shape) => { + if (!shapesCloseToPoint.has(shape.id)) return if (this.isShapeOfType(shape, 'group')) return false const pageMask = this.getShapeMask(shape) if (pageMask && !pointInPolygon(point, pageMask)) return false diff --git a/packages/editor/src/lib/editor/derivations/notVisibleShapes.ts b/packages/editor/src/lib/editor/derivations/notVisibleShapes.ts deleted file mode 100644 index 461835500..000000000 --- a/packages/editor/src/lib/editor/derivations/notVisibleShapes.ts +++ /dev/null @@ -1,105 +0,0 @@ -import { RESET_VALUE, computed, isUninitialized } from '@tldraw/state' -import { TLPageId, TLShapeId, isShape, isShapeId } from '@tldraw/tlschema' -import { Box } from '../../primitives/Box' -import { Editor } from '../Editor' - -function isShapeNotVisible(editor: Editor, id: TLShapeId, viewportPageBounds: Box): boolean { - const maskedPageBounds = editor.getShapeMaskedPageBounds(id) - // if the shape is fully outside of its parent's clipping bounds... - if (maskedPageBounds === undefined) return true - - // if the shape is fully outside of the viewport page bounds... - return !viewportPageBounds.includes(maskedPageBounds) -} - -/** - * Incremental derivation of not visible shapes. - * Non visible shapes are shapes outside of the viewport page bounds and shapes outside of parent's clipping bounds. - * - * @param editor - Instance of the tldraw Editor. - * @returns Incremental derivation of non visible shapes. - */ -export const notVisibleShapes = (editor: Editor) => { - const isCullingOffScreenShapes = Number.isFinite(editor.renderingBoundsMargin) - const shapeHistory = editor.store.query.filterHistory('shape') - let lastPageId: TLPageId | null = null - let prevViewportPageBounds: Box - - function fromScratch(editor: Editor): Set { - const shapes = editor.getCurrentPageShapeIds() - lastPageId = editor.getCurrentPageId() - const viewportPageBounds = editor.getViewportPageBounds() - prevViewportPageBounds = viewportPageBounds.clone() - const notVisibleShapes = new Set() - shapes.forEach((id) => { - if (isShapeNotVisible(editor, id, viewportPageBounds)) { - notVisibleShapes.add(id) - } - }) - return notVisibleShapes - } - return computed>('getCulledShapes', (prevValue, lastComputedEpoch) => { - if (!isCullingOffScreenShapes) return new Set() - - if (isUninitialized(prevValue)) { - return fromScratch(editor) - } - const diff = shapeHistory.getDiffSince(lastComputedEpoch) - - if (diff === RESET_VALUE) { - return fromScratch(editor) - } - - const currentPageId = editor.getCurrentPageId() - if (lastPageId !== currentPageId) { - return fromScratch(editor) - } - const viewportPageBounds = editor.getViewportPageBounds() - if (!prevViewportPageBounds || !viewportPageBounds.equals(prevViewportPageBounds)) { - return fromScratch(editor) - } - - let nextValue = null as null | Set - const addId = (id: TLShapeId) => { - // Already added - if (prevValue.has(id)) return - if (!nextValue) nextValue = new Set(prevValue) - nextValue.add(id) - } - const deleteId = (id: TLShapeId) => { - // No need to delete since it's not there - if (!prevValue.has(id)) return - if (!nextValue) nextValue = new Set(prevValue) - nextValue.delete(id) - } - - for (const changes of diff) { - for (const record of Object.values(changes.added)) { - if (isShape(record)) { - const isCulled = isShapeNotVisible(editor, record.id, viewportPageBounds) - if (isCulled) { - addId(record.id) - } - } - } - - for (const [_from, to] of Object.values(changes.updated)) { - if (isShape(to)) { - const isCulled = isShapeNotVisible(editor, to.id, viewportPageBounds) - if (isCulled) { - addId(to.id) - } else { - deleteId(to.id) - } - } - } - for (const id of Object.keys(changes.removed)) { - if (isShapeId(id)) { - deleteId(id) - } - } - } - - return nextValue ?? prevValue - }) -} diff --git a/packages/editor/src/lib/editor/derivations/spatialIndex.ts b/packages/editor/src/lib/editor/derivations/spatialIndex.ts new file mode 100644 index 000000000..d73dd14e9 --- /dev/null +++ b/packages/editor/src/lib/editor/derivations/spatialIndex.ts @@ -0,0 +1,172 @@ +import { RESET_VALUE, computed, isUninitialized } from '@tldraw/state' +import { TLPageId, TLShapeId, isShape, isShapeId } from '@tldraw/tlschema' +import RBush from 'rbush' +import { Box } from '../../primitives/Box' +import { Editor } from '../Editor' + +type Element = { + minX: number + minY: number + maxX: number + maxY: number + id: TLShapeId +} + +class TldrawRBush extends RBush {} + +export class SpatialIndex { + private readonly spatialIndex: ReturnType + private lastPageId: TLPageId | null = null + private shapesInTree: Map + private rBush: TldrawRBush + + constructor(private editor: Editor) { + this.spatialIndex = this.createSpatialIndex() + this.shapesInTree = new Map() + this.rBush = new TldrawRBush() + } + + private addElement(id: TLShapeId, a: Element[], existingBounds?: Box) { + const e = this.getElement(id, existingBounds) + if (!e) return + a.push(e) + this.shapesInTree.set(id, e) + } + + private getElement(id: TLShapeId, existingBounds?: Box): Element | null { + const bounds = existingBounds ?? this.editor.getShapeMaskedPageBounds(id) + if (!bounds) return null + return { + minX: bounds.minX, + minY: bounds.minY, + maxX: bounds.maxX, + maxY: bounds.maxY, + id, + } + } + + private fromScratch(lastComputedEpoch: number) { + this.lastPageId = this.editor.getCurrentPageId() + this.shapesInTree = new Map() + const elementsToAdd: Element[] = [] + + this.editor.getCurrentPageShapeIds().forEach((id) => { + this.addElement(id, elementsToAdd) + }) + + this.rBush = new TldrawRBush().load(elementsToAdd) + + return lastComputedEpoch + } + + private createSpatialIndex() { + const shapeHistory = this.editor.store.query.filterHistory('shape') + + return computed('spatialIndex', (prevValue, lastComputedEpoch) => { + if (isUninitialized(prevValue)) { + return this.fromScratch(lastComputedEpoch) + } + + const diff = shapeHistory.getDiffSince(lastComputedEpoch) + if (diff === RESET_VALUE) { + return this.fromScratch(lastComputedEpoch) + } + + const currentPageId = this.editor.getCurrentPageId() + if (!this.lastPageId || this.lastPageId !== currentPageId) { + return this.fromScratch(lastComputedEpoch) + } + + let isDirty = false + for (const changes of diff) { + const elementsToAdd: Element[] = [] + for (const record of Object.values(changes.added)) { + if (isShape(record)) { + this.addElement(record.id, elementsToAdd) + } + } + + for (const [_from, to] of Object.values(changes.updated)) { + if (isShape(to)) { + const currentElement = this.shapesInTree.get(to.id) + const newBounds = this.editor.getShapeMaskedPageBounds(to.id) + if (currentElement) { + if ( + newBounds?.minX === currentElement.minX && + newBounds.minY === currentElement.minY && + newBounds.maxX === currentElement.maxX && + newBounds.maxY === currentElement.maxY + ) { + continue + } + this.shapesInTree.delete(to.id) + this.rBush.remove(currentElement) + isDirty = true + } + this.addElement(to.id, elementsToAdd, newBounds) + } + } + if (elementsToAdd.length) { + this.rBush.load(elementsToAdd) + isDirty = true + } + for (const id of Object.keys(changes.removed)) { + if (isShapeId(id)) { + const currentElement = this.shapesInTree.get(id) + if (currentElement) { + this.shapesInTree.delete(id) + this.rBush.remove(currentElement) + isDirty = true + } + } + } + } + + return isDirty ? lastComputedEpoch : prevValue + }) + } + + @computed + private _getVisibleShapes() { + return computed>('visible shapes', (prevValue) => { + // Make sure the spatial index is up to date + const _index = this.spatialIndex.get() + const newValue = this.rBush.search(this.editor.getViewportPageBounds()).map((s) => s.id) + if (isUninitialized(prevValue)) { + return new Set(newValue) + } + const isSame = prevValue.size === newValue.length && newValue.every((id) => prevValue.has(id)) + return isSame ? prevValue : new Set(newValue) + }) + } + + @computed + getVisibleShapes() { + return this._getVisibleShapes().get() + } + + @computed + _getNotVisibleShapes() { + return computed>('not visible shapes', (prevValue) => { + const visibleShapes = this._getVisibleShapes().get() + const pageShapes = this.editor.getCurrentPageShapeIds() + const nonVisibleShapes = [...pageShapes].filter((id) => !visibleShapes.has(id)) + if (isUninitialized(prevValue)) return new Set(nonVisibleShapes) + const isSame = + prevValue.size === nonVisibleShapes.length && + nonVisibleShapes.every((id) => prevValue.has(id)) + return isSame ? prevValue : new Set(nonVisibleShapes) + }) + } + + @computed + getNotVisibleShapes() { + return this._getNotVisibleShapes().get() + } + + getShapeIdsInsideBounds(bounds: Box) { + // Make sure the spatial index is up to date + const _index = this.spatialIndex.get() + return this.rBush.search(bounds).map((s) => s.id) + } +} diff --git a/packages/editor/src/lib/primitives/Box.ts b/packages/editor/src/lib/primitives/Box.ts index ff0abc795..a30d92dd3 100644 --- a/packages/editor/src/lib/primitives/Box.ts +++ b/packages/editor/src/lib/primitives/Box.ts @@ -376,6 +376,10 @@ export class Box { return new Box(minX, minY, maxX - minX, maxY - minY) } + static AroundPoint(point: VecLike, n: number) { + return new Box(point.x - n, point.y - n, 2 * n, 2 * n) + } + static Expand(A: Box, B: Box) { const minX = Math.min(B.minX, A.minX) const minY = Math.min(B.minY, A.minY) diff --git a/packages/tldraw/src/lib/tools/EraserTool/childStates/Erasing.ts b/packages/tldraw/src/lib/tools/EraserTool/childStates/Erasing.ts index c3367bef3..5cb5adfd8 100644 --- a/packages/tldraw/src/lib/tools/EraserTool/childStates/Erasing.ts +++ b/packages/tldraw/src/lib/tools/EraserTool/childStates/Erasing.ts @@ -1,4 +1,5 @@ import { + Box, HIT_TEST_MARGIN, StateNode, TLEventHandlers, @@ -80,9 +81,8 @@ export class Erasing extends StateNode { update() { const { editor, excludedShapeIds } = this - const erasingShapeIds = editor.getErasingShapeIds() - const zoomLevel = editor.getZoomLevel() - const currentPageShapes = editor.getCurrentPageShapes() + const erasingShapeIds = this.editor.getErasingShapeIds() + const zoomLevel = this.editor.getZoomLevel() const { inputs: { currentPagePoint, previousPagePoint }, } = editor @@ -92,8 +92,11 @@ export class Erasing extends StateNode { const erasing = new Set(erasingShapeIds) const minDist = HIT_TEST_MARGIN / zoomLevel - for (const shape of currentPageShapes) { - if (editor.isShapeOfType(shape, 'group')) continue + const shapesNearPoint = this.editor.getShapesInsideBounds( + Box.FromPoints([currentPagePoint, previousPagePoint]).expandBy(minDist) + ) + for (const shape of shapesNearPoint) { + if (this.editor.isShapeOfType(shape, 'group')) continue // Avoid testing masked shapes, unless the pointer is inside the mask const pageMask = editor.getShapeMask(shape.id) diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts index 593e3e36c..98d5c0ad2 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts @@ -125,12 +125,13 @@ export class Brushing extends StateNode { pageTransform: Mat | undefined, localCorners: Vec[] - const currentPageShapes = editor.getCurrentPageShapes() const currentPageId = editor.getCurrentPageId() - testAllShapes: for (let i = 0, n = currentPageShapes.length; i < n; i++) { - shape = currentPageShapes[i] - if (excludedShapeIds.has(shape.id) || results.has(shape.id)) continue testAllShapes + const shapesInsideBounds = this.editor.getShapesInsideBounds(brush) + testAllShapes: for (let i = 0, n = shapesInsideBounds.length; i < n; i++) { + shape = shapesInsideBounds[i] + if (excludedShapeIds.has(shape.id)) continue testAllShapes + if (results.has(shape.id)) continue testAllShapes pageBounds = editor.getShapePageBounds(shape) if (!pageBounds) continue testAllShapes @@ -176,7 +177,7 @@ export class Brushing extends StateNode { const current = editor.getSelectedShapeIds() if (current.length !== results.size || current.some((id) => !results.has(id))) { - editor.setSelectedShapes(Array.from(results), { squashing: true }) + editor.setSelectedShapes(Array.from(results).sort(), { squashing: true }) } } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts index 7f9504b42..fd9c21ea7 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts @@ -1,4 +1,5 @@ import { + Box, Geometry2d, StateNode, TLEventHandlers, @@ -83,8 +84,6 @@ export class ScribbleBrushing extends StateNode { private updateScribbleSelection(addPoint: boolean) { const { editor } = this - // const zoomLevel = this.editor.getZoomLevel() - const currentPageShapes = this.editor.getCurrentPageShapes() const { inputs: { shiftKey, originPagePoint, previousPagePoint, currentPagePoint }, } = this.editor @@ -95,7 +94,9 @@ export class ScribbleBrushing extends StateNode { this.pushPointToScribble() } - const shapes = currentPageShapes + const shapes = this.editor.getShapesInsideBounds( + Box.FromPoints([previousPagePoint, currentPagePoint]) + ) let shape: TLShape, geometry: Geometry2d, A: Vec, B: Vec const minDist = 0 // HIT_TEST_MARGIN / zoomLevel diff --git a/packages/tldraw/src/test/getCulledShapes.test.tsx b/packages/tldraw/src/test/getCulledShapes.test.tsx index 53c99bd35..68ced17f8 100644 --- a/packages/tldraw/src/test/getCulledShapes.test.tsx +++ b/packages/tldraw/src/test/getCulledShapes.test.tsx @@ -1,4 +1,4 @@ -import { Box, TLShapeId, createShapeId } from '@tldraw/editor' +import { Box, PageRecordType, TLShapeId, createShapeId } from '@tldraw/editor' import { TestEditor } from './TestEditor' import { TL } from './test-jsx' @@ -130,8 +130,14 @@ it('correctly calculates the culled shapes when adding and deleting shapes', () const culledShapesIncremental = editor.getCulledShapes() // force full refresh - editor.pan({ x: -1, y: 0 }) - editor.pan({ x: 1, y: 0 }) + const currentPage = editor.getCurrentPageId() + const id = PageRecordType.createId('page2') + editor.createPage({ id, name: 'Page 2' }) + editor.setCurrentPage(id) + // Make sure we access the culled shapes so that the editor knows to update them + // In prod this happens automatically since rendering of shapes depends on the culled shapes + const _culledOnOtherPage = editor.getCulledShapes() + editor.setCurrentPage(currentPage) const culledShapeFromScratch = editor.getCulledShapes() expect(culledShapesIncremental).toEqual(culledShapeFromScratch) diff --git a/packages/tldraw/src/test/selection-omnibus.test.ts b/packages/tldraw/src/test/selection-omnibus.test.ts index e02acabf0..a4deff8cb 100644 --- a/packages/tldraw/src/test/selection-omnibus.test.ts +++ b/packages/tldraw/src/test/selection-omnibus.test.ts @@ -1612,16 +1612,16 @@ describe('shift brushes to add to the selection', () => { editor.keyDown('Shift') editor.pointerDown() editor.pointerMove(1, 1) - expect(editor.getSelectedShapeIds()).toEqual([ids.box2, ids.box1]) + expect(editor.getSelectedShapeIds()).toEqual([ids.box1, ids.box2]) editor.keyUp('Shift') // there's a timer here—we should keep the shift mode until the timer expires - expect(editor.getSelectedShapeIds()).toEqual([ids.box2, ids.box1]) + expect(editor.getSelectedShapeIds()).toEqual([ids.box1, ids.box2]) jest.advanceTimersByTime(500) // once the timer expires, we should be back in regular mode expect(editor.getSelectedShapeIds()).toEqual([ids.box1]) editor.keyDown('Shift') // there's no timer on key down, so go right into shift mode again - expect(editor.getSelectedShapeIds()).toEqual([ids.box2, ids.box1]) + expect(editor.getSelectedShapeIds()).toEqual([ids.box1, ids.box2]) }) }) diff --git a/packages/tldraw/src/test/spatialIndex.test.ts b/packages/tldraw/src/test/spatialIndex.test.ts new file mode 100644 index 000000000..27cd56688 --- /dev/null +++ b/packages/tldraw/src/test/spatialIndex.test.ts @@ -0,0 +1,114 @@ +import { Box, PageRecordType, TLShapeId, createShapeId } from '@tldraw/editor' +import { TestEditor } from './TestEditor' + +let editor: TestEditor + +beforeEach(() => { + editor = new TestEditor() +}) + +const NUM_SHAPES = 1000 +const SHAPE_SIZE = { min: 100, max: 300 } +const NUM_QUERIES = 100 + +type IdAndBounds = { id: TLShapeId; bounds: Box } + +function generateShapes() { + const result: IdAndBounds[] = [] + for (let i = 0; i < NUM_SHAPES; i++) { + const xNegative = Math.random() > 0.5 + const yNegative = Math.random() > 0.5 + const x = Math.random() * 10000 * (xNegative ? -1 : 1) + const y = Math.random() * 10000 * (yNegative ? -1 : 1) + const id = createShapeId() + editor.createShape({ + id, + type: 'geo', + x, + y, + props: { + w: Math.random() * (SHAPE_SIZE.max - SHAPE_SIZE.min) + SHAPE_SIZE.min, + h: Math.random() * (SHAPE_SIZE.max - SHAPE_SIZE.min) + SHAPE_SIZE.min, + }, + }) + const shape = editor.getShape(id) + if (!shape) continue + const bounds = editor.getShapePageBounds(shape) + if (!bounds) continue + result.push({ id, bounds }) + } + return result +} + +function pickShapes(shapes: IdAndBounds[]) { + // We pick at max 1/40 of the shapes, so that the common bounds have more chance not to cover the whole area + const numOfShapes = Math.floor((Math.random() * NUM_SHAPES) / 40) + const pickedShapes: IdAndBounds[] = [] + for (let i = 0; i < numOfShapes; i++) { + const index = Math.floor(Math.random() * shapes.length) + pickedShapes.push(shapes[index]) + } + return pickedShapes +} + +describe('Spatial Index', () => { + it('finds the shapes inside and outside bounds', () => { + const shapes = generateShapes() + for (let i = 0; i < NUM_QUERIES; i++) { + const pickedShapes = pickShapes(shapes) + const commonBounds = Box.Common(pickedShapes.map((s) => s.bounds)) + let shapeIdsInsideBounds = editor.getShapeIdsInsideBounds(commonBounds) + // It should include all the shapes inside common bounds + expect(pickedShapes.every((s) => shapeIdsInsideBounds.includes(s.id))).toBe(true) + + // It also works when we shrink the bounds so that we don't fully contain shapes + shapeIdsInsideBounds = editor.getShapeIdsInsideBounds( + commonBounds.expandBy(-SHAPE_SIZE.min / 2) + ) + expect(pickedShapes.every((s) => shapeIdsInsideBounds.includes(s.id))).toBe(true) + + const shapeIdsOutsideBounds = shapes + .map((i) => i.id) + .filter((id) => { + const shape = editor.getShape(id) + if (!shape) return false + const bounds = editor.getShapePageBounds(shape) + if (!bounds) return false + return !commonBounds.includes(bounds) + }) + // It should not contain any shapes outside the bounds + expect(shapeIdsOutsideBounds.every((id) => !shapeIdsInsideBounds.includes(id))).toBe(true) + expect(shapeIdsInsideBounds.length + shapeIdsOutsideBounds.length).toBe(NUM_SHAPES) + } + }) + + it('works when switching pages', () => { + const currentPageId = editor.getCurrentPageId() + let shapesInsideBounds: TLShapeId[] + + const page1Shapes = generateShapes() + const page1Picks = pickShapes(page1Shapes) + const page1CommonBounds = Box.Common(page1Picks.map((s) => s.bounds)) + shapesInsideBounds = editor.getShapeIdsInsideBounds(page1CommonBounds) + expect(page1Picks.every((s) => shapesInsideBounds.includes(s.id))).toBe(true) + + const newPage = { + id: PageRecordType.createId(), + name: 'Page 2', + } + editor.createPage(newPage) + editor.setCurrentPage(newPage.id) + + const page2Shapes = generateShapes() + const page2Picks = pickShapes(page2Shapes) + const page2CommonBounds = Box.Common(page2Picks.map((s) => s.bounds)) + shapesInsideBounds = editor.getShapeIdsInsideBounds(page2CommonBounds) + expect(page2Picks.every((s) => shapesInsideBounds.includes(s.id))).toBe(true) + expect(page1Shapes.every((s) => !shapesInsideBounds.includes(s.id))).toBe(true) + + editor.setCurrentPage(currentPageId) + shapesInsideBounds = editor.getShapeIdsInsideBounds(page1CommonBounds) + expect(page1Picks.every((s) => shapesInsideBounds.includes(s.id))).toBe(true) + expect(page2Shapes.every((s) => !shapesInsideBounds.includes(s.id))).toBe(true) + }) +}) diff --git a/yarn.lock b/yarn.lock index 0b94e4b30..cba84c448 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7489,6 +7489,7 @@ __metadata: "@types/core-js": "npm:^2.5.5" "@types/lodash.throttle": "npm:^4.1.7" "@types/lodash.uniq": "npm:^4.5.7" + "@types/rbush": "npm:^3.0.3" "@types/react-test-renderer": "npm:^18.0.0" "@types/wicg-file-system-access": "npm:^2020.9.5" "@use-gesture/react": "npm:^10.2.27" @@ -7505,6 +7506,7 @@ __metadata: lodash.throttle: "npm:^4.1.1" lodash.uniq: "npm:^4.5.0" nanoid: "npm:4.0.2" + rbush: "npm:^3.0.1" react-test-renderer: "npm:^18.2.0" resize-observer-polyfill: "npm:^1.5.1" peerDependencies: @@ -8331,6 +8333,13 @@ __metadata: languageName: node linkType: hard +"@types/rbush@npm:^3.0.3": + version: 3.0.3 + resolution: "@types/rbush@npm:3.0.3" + checksum: 59c75d20d3ebf95f8853a98f67d437adc047bf875df6e6bba90884fdfa8fa927402ccec762ecbc8724d98f9ed14c9e97d16eddb709a702021ce1874da5d0d8d7 + languageName: node + linkType: hard + "@types/react-dom@npm:^18.0.0, @types/react-dom@npm:^18.2.18": version: 18.2.18 resolution: "@types/react-dom@npm:18.2.18" @@ -21070,6 +21079,13 @@ __metadata: languageName: node linkType: hard +"quickselect@npm:^2.0.0": + version: 2.0.0 + resolution: "quickselect@npm:2.0.0" + checksum: ed2e78431050d223fb75da20ee98011aef1a03f7cb04e1a32ee893402e640be3cfb76d72e9dbe01edf3bb457ff6a62e5c2d85748424d1aa531f6ba50daef098c + languageName: node + linkType: hard + "raf@npm:^3.4.1": version: 3.4.1 resolution: "raf@npm:3.4.1" @@ -21107,6 +21123,15 @@ __metadata: languageName: node linkType: hard +"rbush@npm:^3.0.1": + version: 3.0.1 + resolution: "rbush@npm:3.0.1" + dependencies: + quickselect: "npm:^2.0.0" + checksum: 489e2e7d9889888ad533518f194e3ab7cc19b1f1365a38ee99fbdda542a47f41cda7dc89870180050f4d04ea402e9ff294e1d767d03c0f1694e0028b7609eec9 + languageName: node + linkType: hard + "rc@npm:^1.2.7, rc@npm:^1.2.8, rc@npm:~1.2.7": version: 1.2.8 resolution: "rc@npm:1.2.8"