Fix lag while panning + translating at the same time (#3186)

Before

![Kapture 2024-03-18 at 09 42
33](https://github.com/tldraw/tldraw/assets/1242537/d27c5852-9514-4e44-8b75-d2cb2571362a)


After

![Kapture 2024-03-18 at 09 41
27](https://github.com/tldraw/tldraw/assets/1242537/f5cbebfd-a45c-48d9-915b-18823f4555ff)

The probelm was manifesting because our camera updates were not
throttled and our render tick was on a different tick timeline to our
tick manager. Fixing the latter gets rid of the lag without requiring us
to throttle the camera updates.

### Change Type

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

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

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

- [x] `bugfix` — Bug fix



### Test Plan

1. Add a step-by-step description of how to test your PR here.
2.

- [ ] Unit Tests
- [ ] End to end tests

### Release Notes

- Add a brief release note for your PR here.

---------

Co-authored-by: Steve Ruiz <steveruizok@gmail.com>

packages/editor/src/lib/editor/Editor.ts

@@ -207,6 +207,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 		this.getContainer = getContainer ?? (() => document.body)
 
 		this.textMeasure = new TextManager(this)
+		this._tickManager = new TickManager(this)
 
 		class NewRoot extends RootState {
 			static override initial = initialState ?? ''
@@ -666,7 +667,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 	readonly disposables = new Set<() => void>()
 
 	/** @internal */
-	private _tickManager = new TickManager(this)
+	private readonly _tickManager
 
 	/**
 	 * A manager for the app's snapping feature.

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

@@ -1,21 +1,31 @@
+import { throttleToNextFrame as _throttleToNextFrame } from '@tldraw/utils'
 import { Vec } from '../../primitives/Vec'
 import { Editor } from '../Editor'
 
+const throttleToNextFrame =
+	typeof process !== 'undefined' && process.env.NODE_ENV === 'test'
+		? // At test time we should use actual raf and not throttle, because throttle was set up to evaluate immediately during tests, which causes stack overflow
+			// for the tick manager since it sets up a raf loop.
+			function mockThrottle(cb: any) {
+				const frame = requestAnimationFrame(cb)
+				return () => cancelAnimationFrame(frame)
+			}
+		: _throttleToNextFrame
+
 export class TickManager {
 	constructor(public editor: Editor) {
 		this.editor.disposables.add(this.dispose)
 		this.start()
 	}
 
-	raf: any
+	cancelRaf?: null | (() => void)
 	isPaused = true
 	last = 0
-	t = 0
 
 	start = () => {
 		this.isPaused = false
-		cancelAnimationFrame(this.raf)
-		this.raf = requestAnimationFrame(this.tick)
+		this.cancelRaf?.()
+		this.cancelRaf = throttleToNextFrame(this.tick)
 		this.last = Date.now()
 	}
 
@@ -27,25 +37,18 @@ export class TickManager {
 		const now = Date.now()
 		const elapsed = now - this.last
 		this.last = now
-		this.t += elapsed
 
-		this.editor.emit('frame', elapsed)
-
-		if (this.t < 16) {
-			this.raf = requestAnimationFrame(this.tick)
-			return
-		}
-
-		this.t -= 16
 		this.updatePointerVelocity(elapsed)
+		this.editor.emit('frame', elapsed)
 		this.editor.emit('tick', elapsed)
-		this.raf = requestAnimationFrame(this.tick)
+		this.cancelRaf = throttleToNextFrame(this.tick)
 	}
 
 	// Clear the listener
 	dispose = () => {
 		this.isPaused = true
-		cancelAnimationFrame(this.raf)
+
+		this.cancelRaf?.()
 	}
 
 	private prevPoint = new Vec()

packages/tldraw/src/test/TestEditor.ts

@@ -3,6 +3,7 @@ import {
 	BoxModel,
 	Editor,
 	HALF_PI,
+	IdOf,
 	Mat,
 	PageRecordType,
 	ROTATE_CORNER_TO_SELECTION_CORNER,
@@ -205,6 +206,8 @@ export class TestEditor extends Editor {
 			y: +camera.y.toFixed(2),
 			z: +camera.z.toFixed(2),
 		}).toCloselyMatchObject({ x, y, z })
+
+		return this
 	}
 
 	expectShapeToMatch = <T extends TLShape = TLShape>(
@@ -218,6 +221,25 @@ export class TestEditor extends Editor {
 		return this
 	}
 
+	expectPageBoundsToBe = <T extends TLShape = TLShape>(id: IdOf<T>, bounds: Partial<BoxModel>) => {
+		const observedBounds = this.getShapePageBounds(id)!
+		expect(observedBounds).toCloselyMatchObject(bounds)
+		return this
+	}
+
+	expectScreenBoundsToBe = <T extends TLShape = TLShape>(
+		id: IdOf<T>,
+		bounds: Partial<BoxModel>
+	) => {
+		const pageBounds = this.getShapePageBounds(id)!
+		const screenPoint = this.pageToScreen(pageBounds.point)
+		const observedBounds = pageBounds.clone()
+		observedBounds.x = screenPoint.x
+		observedBounds.y = screenPoint.y
+		expect(observedBounds).toCloselyMatchObject(bounds)
+		return this
+	}
+
 	/* --------------------- Inputs --------------------- */
 
 	protected getInfo = <T extends TLEventInfo>(info: string | T): T => {
@@ -294,8 +316,9 @@ export class TestEditor extends Editor {
 	You can use this helper to force the tick, which will then process all the updates.
 	*/
 	forceTick = (count = 1) => {
+		const tick = (this as any)._tickManager as { tick(): void }
 		for (let i = 0; i < count; i++) {
-			this.emit('tick', 16)
+			tick.tick()
 		}
 		return this
 	}

packages/tldraw/src/test/translating.test.ts

@@ -1876,3 +1876,129 @@ it('clones a single shape simply', () => {
 	expect(editor.getEditingShape()).toBe(undefined)
 	expect(editor.getHoveredShape()).toBe(sticky2)
 })
+
+describe('Moving the camera while panning', () => {
+	it('moves things while dragging', () => {
+		editor.createShape({
+			type: 'geo',
+			id: ids.box1,
+			x: 0,
+			y: 0,
+			props: { geo: 'rectangle', w: 100, h: 100, fill: 'solid' },
+		})
+
+		editor
+			.expectShapeToMatch({ id: ids.box1, x: 0, y: 0 })
+			.expectToBeIn('select.idle')
+			.pointerMove(40, 40)
+			.pointerDown()
+			.expectToBeIn('select.pointing_shape')
+			.pointerMove(50, 50) // move by 10,10
+			.expectToBeIn('select.translating')
+			.expectShapeToMatch({ id: ids.box1, x: 10, y: 10 })
+			.wheel(-10, -10) // wheel by -10,-10
+			.forceTick() // needed
+			.expectShapeToMatch({ id: ids.box1, x: 20, y: 20 })
+			.wheel(-10, -10) // wheel by -10,-10
+			.forceTick() // needed
+			.expectShapeToMatch({ id: ids.box1, x: 30, y: 30 })
+	})
+
+	it('FAILING EXAMPLE: preserves screen point while dragging', () => {
+		editor.createShape({
+			type: 'geo',
+			id: ids.box1,
+			x: 0,
+			y: 0,
+			props: { geo: 'rectangle', w: 100, h: 100, fill: 'solid' },
+		})
+
+		editor
+			.expectCameraToBe(0, 0, 1)
+			.expectShapeToMatch({ id: ids.box1, x: 0, y: 0 })
+			.expectPageBoundsToBe(ids.box1, { x: 0, y: 0 })
+			.expectScreenBoundsToBe(ids.box1, { x: 0, y: 0 })
+			.expectToBeIn('select.idle')
+			.pointerMove(40, 40)
+			.pointerDown()
+			.expectToBeIn('select.pointing_shape')
+			.pointerMove(50, 50) // move by 10,10
+			.expectToBeIn('select.translating')
+
+			// we haven't moved the camera from origin yet, so the
+			// point / page / screen points should all be identical
+			.expectCameraToBe(0, 0, 1)
+			.expectShapeToMatch({ id: ids.box1, x: 10, y: 10 })
+			.expectPageBoundsToBe(ids.box1, { x: 10, y: 10 })
+			.expectScreenBoundsToBe(ids.box1, { x: 10, y: 10 })
+
+			// now we move the camera by -10,-10
+			// since we're dragging, they should still all move together
+			.wheel(-10, -10)
+
+			// ! This is the problem here—the screen point has changed
+			// ! because the camera moved but the resulting pointer move
+			// ! isn't processed until after the tick
+			.expectCameraToBe(-10, -10, 1)
+			.expectScreenBoundsToBe(ids.box1, { x: 0, y: 0 }) // should be 10,10
+
+			// nothing else has changed yet... until the tick
+			.expectShapeToMatch({ id: ids.box1, x: 10, y: 10 })
+			.expectPageBoundsToBe(ids.box1, { x: 10, y: 10 })
+
+			.forceTick() // needed
+
+			// The camera is still the same...
+			.expectCameraToBe(-10, -10, 1)
+
+			// But we've processed a pointer move, which has changed the shapes
+			.expectShapeToMatch({ id: ids.box1, x: 20, y: 20 })
+			.expectPageBoundsToBe(ids.box1, { x: 20, y: 20 })
+
+			// ! And this has fixed the screen point
+			.expectScreenBoundsToBe(ids.box1, { x: 10, y: 10 })
+	})
+
+	it('Correctly preserves screen point while dragging', async () => {
+		editor.createShape({
+			type: 'geo',
+			id: ids.box1,
+			x: 0,
+			y: 0,
+			props: { geo: 'rectangle', w: 100, h: 100, fill: 'solid' },
+		})
+
+		editor
+			.expectCameraToBe(0, 0, 1)
+			.expectShapeToMatch({ id: ids.box1, x: 0, y: 0 })
+			.expectPageBoundsToBe(ids.box1, { x: 0, y: 0 })
+			.expectScreenBoundsToBe(ids.box1, { x: 0, y: 0 })
+			.expectToBeIn('select.idle')
+			.pointerMove(40, 40)
+			.pointerDown()
+			.expectToBeIn('select.pointing_shape')
+			.pointerMove(50, 50) // move by 10,10
+			.expectToBeIn('select.translating')
+
+			// we haven't moved the camera from origin yet, so the
+			// point / page / screen points should all be identical
+			.expectCameraToBe(0, 0, 1)
+			.expectShapeToMatch({ id: ids.box1, x: 10, y: 10 })
+			.expectPageBoundsToBe(ids.box1, { x: 10, y: 10 })
+			.expectScreenBoundsToBe(ids.box1, { x: 10, y: 10 })
+
+			// now we move the camera by -10,-10
+			// since we're dragging, they should still all move together
+			.wheel(-10, -10)
+			.forceTick()
+			// wait for a tick to allow the tick manager to dispatch to the translating tool
+
+			// The camera has moved
+			.expectCameraToBe(-10, -10, 1)
+			.expectShapeToMatch({ id: ids.box1, x: 20, y: 20 })
+			.expectPageBoundsToBe(ids.box1, { x: 20, y: 20 })
+
+			// Screen bounds / point is still the same as it was before
+			.expectScreenBoundsToBe(ids.box1, { x: 10, y: 10 })
+	})
+})

packages/utils/api-report.md

@@ -315,7 +315,7 @@ export { structuredClone_2 as structuredClone }
 export function throttle<T extends (...args: any) => any>(func: T, limit: number): (...args: Parameters<T>) => ReturnType<T>;
 
 // @internal
-export function throttleToNextFrame(fn: () => void): void;
+export function throttleToNextFrame(fn: () => void): () => void;
 
 // @internal (undocumented)
 export function validateIndexKey(key: string): asserts key is IndexKey;

packages/utils/src/lib/throttle.ts

@@ -74,23 +74,31 @@ export function fpsThrottle(fn: () => void) {
  * Calls the function on the next frame. The target frame rate is 60fps.
  * If the same fn is passed again before the next frame, it will still be called only once.
  * @param fn - the fun to call on the next frame
- * @returns
+ * @returns a function that will cancel the call if called before the next frame
  * @internal
  */
-export function throttleToNextFrame(fn: () => void) {
+export function throttleToNextFrame(fn: () => void): () => void {
 	if (isTest()) {
-		return fn()
+		fn()
+		return () => {
+			// noop
+		}
 	}
 
-	if (fpsQueue.includes(fn)) {
-		return
+	if (!fpsQueue.includes(fn)) {
+		fpsQueue.push(fn)
+		if (!started) {
+			started = true
+			// We set last to Date.now() - targetTimePerFrame - 1 so that the first run will happen immediately
+			last = Date.now() - targetTimePerFrame - 1
+		}
+		tick()
 	}
 
-	fpsQueue.push(fn)
-	if (!started) {
-		started = true
-		// We set last to Date.now() - targetTimePerFrame - 1 so that the first run will happen immediately
-		last = Date.now() - targetTimePerFrame - 1
+	return () => {
+		const index = fpsQueue.indexOf(fn)
+		if (index > -1) {
+			fpsQueue.splice(index, 1)
+		}
 	}
-	tick()
 }