[pull] main from tldraw:main

packages/editor/src/lib/editor/derivations/notVisibleShapes.ts

@@ -1,6 +1,7 @@
 import { computed, isUninitialized } from '@tldraw/state'
-import { TLShape, TLShapeId } from '@tldraw/tlschema'
+import { TLShapeId } from '@tldraw/tlschema'
 import type { Editor } from '../Editor'
+import { ShapeUtil } from '../shapes/ShapeUtil'
 
 /**
  * Non visible shapes are shapes outside of the viewport page bounds.
@@ -10,55 +11,62 @@ import type { Editor } from '../Editor'
  */
 export function notVisibleShapes(editor: Editor) {
 	const emptySet = new Set<TLShapeId>()
+	const defaultCanCull = ShapeUtil.prototype.canCull
 
 	return computed<Set<TLShapeId>>('notVisibleShapes', function (prevValue) {
-		const allShapes = editor.getCurrentPageShapes()
+		const allShapeIds = editor.getCurrentPageShapeIds()
 		const viewportPageBounds = editor.getViewportPageBounds()
 		const visibleIds = editor.getShapeIdsInsideBounds(viewportPageBounds)
 
-		let shape: TLShape | undefined
-
 		// Fast path: if all shapes are visible, return empty set
-		if (visibleIds.size === allShapes.length) {
+		if (visibleIds.size === allShapeIds.size) {
 			if (isUninitialized(prevValue) || prevValue.size > 0) {
 				return emptySet
 			}
 			return prevValue
 		}
 
-		// First run: compute from scratch
-		if (isUninitialized(prevValue)) {
-			const nextValue = new Set<TLShapeId>()
-			for (let i = 0; i < allShapes.length; i++) {
-				shape = allShapes[i]
-				if (visibleIds.has(shape.id)) continue
-				if (!editor.getShapeUtil(shape.type).canCull(shape)) continue
-				nextValue.add(shape.id)
+		const notVisibleIds = new Set<TLShapeId>()
+		for (const id of allShapeIds) {
+			if (visibleIds.has(id)) continue
+
+			// Peek at the shape without subscribing — we only need its type to look up the util.
+			// Type is treated as immutable for a given id, so this is safe.
+			const peek = editor.store.unsafeGetWithoutCapture(id)
+			if (!peek) continue
+			const util = editor.getShapeUtil(peek.type)
+
+			// If canCull is the default (always-true), skip per-shape subscription entirely.
+			// >99% of shapes hit this path in practice.
+			if (util.canCull === defaultCanCull) {
+				notVisibleIds.add(id)
+				continue
 			}
-			return nextValue
+
+			// Custom canCull — subscribe so prop flips invalidate this derivation.
+			const shape = editor.getShape(id)
+			if (!shape) continue
+			if (!util.canCull(shape)) continue
+			notVisibleIds.add(id)
 		}
 
-		// Subsequent runs: single pass to collect IDs and detect changes
-		const notVisibleIds: TLShapeId[] = []
-		for (let i = 0; i < allShapes.length; i++) {
-			shape = allShapes[i]
-			if (visibleIds.has(shape.id)) continue
-			if (!editor.getShapeUtil(shape.type).canCull(shape)) continue
-			notVisibleIds.push(shape.id)
+		// First run
+		if (isUninitialized(prevValue)) {
+			return notVisibleIds
 		}
 
-		// Check if the result changed
-		if (notVisibleIds.length === prevValue.size) {
+		// Reuse prev set when contents are unchanged
+		if (notVisibleIds.size === prevValue.size) {
 			let same = true
-			for (let i = 0; i < notVisibleIds.length; i++) {
-				if (!prevValue.has(notVisibleIds[i])) {
+			for (const id of notVisibleIds) {
+				if (!prevValue.has(id)) {
 					same = false
 					break
 				}
 			}
 			if (same) return prevValue
 		}
 
-		return new Set(notVisibleIds)
+		return notVisibleIds
 	})
 }

packages/editor/src/lib/editor/managers/SpatialIndexManager/RBushIndex.ts

@@ -113,25 +113,24 @@ export class RBushIndex {
 	}
 
 	/**
-	 * Get all shape IDs currently in the spatial index.
+	 * Get the raw stored element for a shape, without allocating a Box.
+	 * Use when you only need to read the indexed bounds for comparison.
+	 *
+	 * @internal
 	 */
-	getAllShapeIds(): TLShapeId[] {
-		return Array.from(this.elementsInTree.keys())
+	getElement(id: TLShapeId): SpatialElement | undefined {
+		return this.elementsInTree.get(id)
 	}
 
 	/**
-	 * Get the bounds currently stored in the spatial index for a shape.
-	 * Returns undefined if the shape is not in the index.
+	 * Iterate the entries currently in the index. Callers may upsert existing
+	 * keys or remove keys during iteration; current callers do not insert new
+	 * keys.
+	 *
+	 * @internal
 	 */
-	getBounds(id: TLShapeId): Box | undefined {
-		const element = this.elementsInTree.get(id)
-		if (!element) return undefined
-		return new Box(
-			element.minX,
-			element.minY,
-			element.maxX - element.minX,
-			element.maxY - element.minY
-		)
+	entries(): IterableIterator<[TLShapeId, SpatialElement]> {
+		return this.elementsInTree.entries()
 	}
 
 	/**

packages/editor/src/lib/editor/managers/SpatialIndexManager/SpatialIndexManager.ts

@@ -28,51 +28,56 @@ export class SpatialIndexManager {
 	private spatialIndexComputed: Computed<number>
 	private lastPageId: TLPageId | null = null
 
+	// Bumps only when the rbush content may have changed. Consumers subscribe
+	// via the computed; a stable epoch lets prop-only diffs skip downstream
+	// invalidations.
+	private _boundsEpoch = 0
+
 	constructor(public readonly editor: Editor) {
 		this.rbush = new RBushIndex()
 		this.spatialIndexComputed = this.createSpatialIndexComputed()
 	}
 
+	private rebuildAndBumpEpoch(): number {
+		this.buildFromScratch()
+		this._boundsEpoch++
+		return this._boundsEpoch
+	}
+
 	private createSpatialIndexComputed() {
 		const shapeHistory = this.editor.store.query.filterHistory('shape')
 
 		return computed<number>('spatialIndex', (_prevValue, lastComputedEpoch) => {
 			if (isUninitialized(_prevValue)) {
-				return this.buildFromScratch(lastComputedEpoch)
+				return this.rebuildAndBumpEpoch()
 			}
 
 			const shapeDiff = shapeHistory.getDiffSince(lastComputedEpoch)
 
 			if (shapeDiff === RESET_VALUE) {
-				return this.buildFromScratch(lastComputedEpoch)
+				return this.rebuildAndBumpEpoch()
 			}
 
 			const currentPageId = this.editor.getCurrentPageId()
 			if (this.lastPageId !== currentPageId) {
-				return this.buildFromScratch(lastComputedEpoch)
+				return this.rebuildAndBumpEpoch()
 			}
 
-			// No shape changes - index is already up to date
-			if (shapeDiff.length === 0) {
-				return lastComputedEpoch
-			}
-
-			// Process incremental updates
-			this.processIncrementalUpdate(shapeDiff)
+			if (shapeDiff.length === 0) return this._boundsEpoch
 
-			return lastComputedEpoch
+			if (this.processIncrementalUpdate(shapeDiff)) {
+				this._boundsEpoch++
+			}
+			return this._boundsEpoch
 		})
 	}
 
-	private buildFromScratch(epoch: number): number {
+	private buildFromScratch(): void {
 		this.rbush.clear()
 		this.lastPageId = this.editor.getCurrentPageId()
 
-		const shapes = this.editor.getCurrentPageShapes()
 		const elements: SpatialElement[] = []
-
-		// Collect all shape elements for bulk loading
-		for (const shape of shapes) {
+		for (const shape of this.editor.getCurrentPageShapes()) {
 			const bounds = this.editor.getShapePageBounds(shape.id)
 			if (bounds && bounds.isValid()) {
 				elements.push({
@@ -87,36 +92,38 @@ export class SpatialIndexManager {
 
 		// Bulk load for efficiency
 		this.rbush.bulkLoad(elements)
-
-		return epoch
 	}
 
-	private processIncrementalUpdate(shapeDiff: RecordsDiff<TLRecord>[]): void {
-		// Track shapes we've already processed from the diff
+	private processIncrementalUpdate(shapeDiff: RecordsDiff<TLRecord>[]): boolean {
 		const processedShapeIds = new Set<TLShapeId>()
+		let changed = false
 
-		// 1. Process shape additions, removals, and updates from diff
+		// Step 1: apply diff entries directly. `changed` flips only on real
+		// rbush mutations, so prop-only updates and no-op removes (e.g. shapes
+		// from other pages, or never-indexed shapes with invalid bounds) don't
+		// bump the epoch.
 		for (const changes of shapeDiff) {
-			// Handle additions (only for shapes on current page)
 			for (const shape of objectMapValues(changes.added) as TLShape[]) {
 				if (isShape(shape) && this.editor.getAncestorPageId(shape) === this.lastPageId) {
 					const bounds = this.editor.getShapePageBounds(shape.id)
 					if (bounds && bounds.isValid()) {
 						this.rbush.upsert(shape.id, bounds)
+						changed = true
 					}
 					processedShapeIds.add(shape.id)
 				}
 			}
 
-			// Handle removals
 			for (const shape of objectMapValues(changes.removed) as TLShape[]) {
 				if (isShape(shape)) {
-					this.rbush.remove(shape.id)
+					if (this.rbush.has(shape.id)) {
+						this.rbush.remove(shape.id)
+						changed = true
+					}
 					processedShapeIds.add(shape.id)
 				}
 			}
 
-			// Handle updated shapes: page changes and bounds updates
 			for (const [, to] of objectMapValues(changes.updated) as [TLShape, TLShape][]) {
 				if (!isShape(to)) continue
 				processedShapeIds.add(to.id)
@@ -126,35 +133,55 @@ export class SpatialIndexManager {
 				if (isOnPage) {
 					const bounds = this.editor.getShapePageBounds(to.id)
 					if (bounds && bounds.isValid()) {
-						this.rbush.upsert(to.id, bounds)
+						const indexedElement = this.rbush.getElement(to.id)
+						if (!this.areBoundsEqualToSpatialElement(bounds, indexedElement)) {
+							this.rbush.upsert(to.id, bounds)
+							changed = true
+						}
+					} else if (this.rbush.has(to.id)) {
+						this.rbush.remove(to.id)
+						changed = true
 					}
-				} else {
+				} else if (this.rbush.has(to.id)) {
 					this.rbush.remove(to.id)
+					changed = true
 				}
 			}
 		}
 
-		// 2. Check remaining shapes in index for bounds changes
-		// This handles shapes with computed bounds (arrows bound to moved shapes, groups with moved children, etc.)
-		const allShapeIds = this.rbush.getAllShapeIds()
-
-		for (const shapeId of allShapeIds) {
+		// Step 2: must always run. Diff entries can dirty derived bounds —
+		// arrows bound to moved shapes, groups with moved children — without
+		// touching any record visited in step 1. Also catches outline-only
+		// changes (e.g. geo rectangle→ellipse at the same w/h) that shift a
+		// bound arrow's intersection points: step 1 sees the geo's
+		// axis-aligned bounds unchanged and skips, but the dependent arrow's
+		// bounds have moved.
+		//
+		// Iterating the rbush's element map directly avoids allocating a
+		// shape-id array per pointer move. Mutation here is limited to
+		// upserts of existing keys and deletions, both safe during Map
+		// iteration.
+		for (const [shapeId, indexedElement] of this.rbush.entries()) {
 			if (processedShapeIds.has(shapeId)) continue
 
 			const currentBounds = this.editor.getShapePageBounds(shapeId)
-			const indexedBounds = this.rbush.getBounds(shapeId)
+			if (this.areBoundsEqualToSpatialElement(currentBounds, indexedElement)) continue
 
-			if (!this.areBoundsEqual(currentBounds, indexedBounds)) {
-				if (currentBounds && currentBounds.isValid()) {
-					this.rbush.upsert(shapeId, currentBounds)
-				} else {
-					this.rbush.remove(shapeId)
-				}
+			if (currentBounds && currentBounds.isValid()) {
+				this.rbush.upsert(shapeId, currentBounds)
+			} else {
+				this.rbush.remove(shapeId)
 			}
+			changed = true
 		}
+
+		return changed
 	}
 
-	private areBoundsEqual(a: Box | undefined, b: Box | undefined): boolean {
+	private areBoundsEqualToSpatialElement(
+		a: Box | undefined,
+		b: SpatialElement | undefined
+	): boolean {
 		if (!a && !b) return true
 		if (!a || !b) return false
 		return a.minX === b.minX && a.minY === b.minY && a.maxX === b.maxX && a.maxY === b.maxY