From b5aff00c8964a3513954fad7ca296c0b8c3bd4cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mitja=20Bezen=C5=A1ek?= Date: Mon, 11 Mar 2024 14:17:31 +0100 Subject: [PATCH] Performance improvements (#2977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR does a few things to help with performance: 1. Instead of doing changes on raf we now do them 60 times per second. This limits the number of updates on high refresh rate screens like the iPad. With the current code this only applied to the history updates (so when you subscribed to the updates), but the next point takes this a bit futher. 2. We now trigger react updates 60 times per second. This is a change in `useValue` and `useStateTracking` hooks. 3. We now throttle the inputs (like the `pointerMove`) in state nodes. This means we batch multiple inputs and only apply them at most 60 times per second. We had to adjust our own tests to pass after this change so I marked this as major as it might require the users of the library to do the same. Few observations: - The browser calls the raf callbacks when it can. If it gets overwhelmed it will call them further and further apart. As things call down it will start calling them more frequently again. You can clearly see this in the drawing example. When fps gets to a certain level we start to get fewer updates, then fps can recover a bit. This makes the experience quite janky. The updates can be kinda ok one second (dropping frames, but consistently) and then they can completely stop and you have to let go of the mouse to make them happen again. With the new logic it seems everything is a lot more consistent. - We might look into variable refresh rates to prevent this overtaxing of the browser. Like when we see that the times between our updates are getting higher we could make the updates less frequent. If we then see that they are happening more often we could ramp them back up. I had an [experiment for this here](https://github.com/tldraw/tldraw/pull/2977/commits/48348639669e556798296eee82fc53ca8ef444f2#diff-318e71563d7c47173f89ec084ca44417cf70fc72faac85b96f48b856a8aec466L30-L35). Few tests below. Used 6x slowdown for these. # Resizing ### Before https://github.com/tldraw/tldraw/assets/2523721/798a033f-5dfa-419e-9a2d-fd8908272ba0 ### After https://github.com/tldraw/tldraw/assets/2523721/45870a0c-c310-4be0-b63c-6c92c20ca037 # Drawing Comparison is not 100% fair, we don't store the intermediate inputs right now. That said, tick should still only produce once update so I do think we can get a sense of the differences. ### Before https://github.com/tldraw/tldraw/assets/2523721/2e8ac8c5-bbdf-484b-bb0c-70c967f4541c ### After https://github.com/tldraw/tldraw/assets/2523721/8f54b7a8-9a0e-4a39-b168-482caceb0149 ### Change Type - [ ] `patch` — Bug fix - [ ] `minor` — New feature - [x] `major` — Breaking change - [ ] `dependencies` — Changes to package dependencies[^1] - [ ] `documentation` — Changes to the documentation only[^2] - [ ] `tests` — Changes to any test code only[^2] - [ ] `internal` — Any other changes that don't affect the published package[^2] - [ ] I don't know [^1]: publishes a `patch` release, for devDependencies use `internal` [^2]: will not publish a new version ### Release Notes - Improves the performance of rendering. --------- Co-authored-by: Steve Ruiz --- apps/examples/package.json | 4 +- packages/state/package.json | 3 + .../state/src/lib/react/useStateTracking.ts | 5 +- packages/state/src/lib/react/useValue.ts | 15 ++- packages/state/tsconfig.json | 7 +- packages/store/src/lib/Store.ts | 4 +- .../src/lib/shapes/draw/toolStates/Drawing.ts | 64 +++++++++---- .../tools/SelectTool/childStates/Brushing.ts | 12 ++- .../tools/SelectTool/childStates/Cropping.ts | 13 ++- .../SelectTool/childStates/DraggingHandle.tsx | 13 ++- .../tools/SelectTool/childStates/Resizing.ts | 7 +- .../tools/SelectTool/childStates/Rotating.ts | 50 +++++----- .../childStates/ScribbleBrushing.ts | 14 ++- .../SelectTool/childStates/Translating.ts | 9 +- packages/tldraw/src/test/TestEditor.ts | 13 ++- packages/tldraw/src/test/drawing.test.ts | 4 +- packages/tldraw/src/test/translating.test.ts | 13 ++- packages/tlsync/src/lib/TLSyncClient.ts | 6 +- packages/utils/api-report.md | 8 +- packages/utils/src/index.ts | 2 +- packages/utils/src/lib/raf.ts | 67 ------------- packages/utils/src/lib/throttle.ts | 96 +++++++++++++++++++ yarn.lock | 1 + 23 files changed, 290 insertions(+), 140 deletions(-) delete mode 100644 packages/utils/src/lib/raf.ts create mode 100644 packages/utils/src/lib/throttle.ts diff --git a/apps/examples/package.json b/apps/examples/package.json index bdd90648c..468f01258 100644 --- a/apps/examples/package.json +++ b/apps/examples/package.json @@ -29,8 +29,8 @@ "dev": "vite --host", "build": "vite build", "lint": "yarn run -T tsx ../../scripts/lint.ts", - "e2e": "playwright test -c ./e2e/playwright.config.ts", - "e2e-ui": "playwright test --ui -c ./e2e/playwright.config.ts" + "e2e": "NODE_ENV=test && playwright test -c ./e2e/playwright.config.ts", + "e2e-ui": "NODE_ENV=test && playwright test --ui -c ./e2e/playwright.config.ts" }, "dependencies": { "@playwright/test": "^1.38.1", diff --git a/packages/state/package.json b/packages/state/package.json index 003a5ddd2..c6037198c 100644 --- a/packages/state/package.json +++ b/packages/state/package.json @@ -52,6 +52,9 @@ "node_modules/(?!(nanoid)/)" ] }, + "dependencies": { + "@tldraw/utils": "workspace:*" + }, "devDependencies": { "@types/lodash": "^4.14.188", "@types/react": "^18.2.47", diff --git a/packages/state/src/lib/react/useStateTracking.ts b/packages/state/src/lib/react/useStateTracking.ts index f46786b34..fbd60fe5c 100644 --- a/packages/state/src/lib/react/useStateTracking.ts +++ b/packages/state/src/lib/react/useStateTracking.ts @@ -1,3 +1,4 @@ +import { fpsThrottle } from '@tldraw/utils' import React from 'react' import { EffectScheduler } from '../core' @@ -26,9 +27,9 @@ export function useStateTracking(name: string, render: () => T): T { () => renderRef.current?.(), // this is what will be invoked when @tldraw/state detects a change in an upstream reactive value { - scheduleEffect() { + scheduleEffect: fpsThrottle(() => { scheduleUpdate?.() - }, + }), } ) diff --git a/packages/state/src/lib/react/useValue.ts b/packages/state/src/lib/react/useValue.ts index 8d4384a5b..4e8d7fa1f 100644 --- a/packages/state/src/lib/react/useValue.ts +++ b/packages/state/src/lib/react/useValue.ts @@ -1,4 +1,5 @@ /* eslint-disable prefer-rest-params */ +import { throttleToNextFrame } from '@tldraw/utils' import { useMemo, useRef, useSyncExternalStore } from 'react' import { Signal, computed, react } from '../core' @@ -81,10 +82,16 @@ export function useValue() { const { subscribe, getSnapshot } = useMemo(() => { return { subscribe: (listen: () => void) => { - return react(`useValue(${name})`, () => { - $val.get() - listen() - }) + return react( + `useValue(${name})`, + () => { + $val.get() + listen() + }, + { + scheduleEffect: throttleToNextFrame, + } + ) }, getSnapshot: () => $val.get(), } diff --git a/packages/state/tsconfig.json b/packages/state/tsconfig.json index 662a89b2d..be43fd20f 100644 --- a/packages/state/tsconfig.json +++ b/packages/state/tsconfig.json @@ -5,5 +5,10 @@ "compilerOptions": { "outDir": "./.tsbuild", "rootDir": "src" - } + }, + "references": [ + { + "path": "../utils" + } + ] } diff --git a/packages/store/src/lib/Store.ts b/packages/store/src/lib/Store.ts index b665b6751..2d24ceec6 100644 --- a/packages/store/src/lib/Store.ts +++ b/packages/store/src/lib/Store.ts @@ -5,7 +5,7 @@ import { objectMapFromEntries, objectMapKeys, objectMapValues, - throttledRaf, + throttleToNextFrame, } from '@tldraw/utils' import { nanoid } from 'nanoid' import { IdOf, RecordId, UnknownRecord } from './BaseRecord' @@ -205,7 +205,7 @@ export class Store { // If we have accumulated history, flush it and update listeners this._flushHistory() }, - { scheduleEffect: (cb) => throttledRaf(cb) } + { scheduleEffect: (cb) => throttleToNextFrame(cb) } ) this.scopedTypes = { document: new Set( diff --git a/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts b/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts index 81762cee0..8269718f3 100644 --- a/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts +++ b/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts @@ -51,11 +51,18 @@ export class Drawing extends StateNode { markId = null as null | string + // Used to track whether we have changes that have not yet been pushed to the Editor. + isDirty = false + // The changes that have not yet been pushed to the Editor. + shapePartial: TLShapePartial | null = null + override onEnter = (info: TLPointerEventInfo) => { this.markId = null this.info = info this.canDraw = !this.editor.getIsMenuOpen() this.lastRecordedPoint = this.editor.inputs.currentPagePoint.clone() + this.shapePartial = null + this.isDirty = false if (this.canDraw) { this.startShape() } @@ -84,8 +91,8 @@ export class Drawing extends StateNode { } if (this.canDraw) { - // Don't update the shape if we haven't moved far enough from the last time we recorded a point if (inputs.isPen) { + // Don't update the shape if we haven't moved far enough from the last time we recorded a point if ( Vec.Dist(inputs.currentPagePoint, this.lastRecordedPoint) >= 1 / this.editor.getZoomLevel() @@ -99,10 +106,18 @@ export class Drawing extends StateNode { this.mergeNextPoint = false } - this.updateShapes() + this.processUpdates() } } + override onTick = () => { + if (!this.isDirty) return + this.isDirty = false + if (!this.shapePartial) return + + this.editor.updateShapes([this.shapePartial], { squashing: true }) + } + override onKeyDown: TLEventHandlers['onKeyDown'] = (info) => { if (info.key === 'Shift') { switch (this.segmentMode) { @@ -117,7 +132,7 @@ export class Drawing extends StateNode { } } } - this.updateShapes() + this.processUpdates() } override onKeyUp: TLEventHandlers['onKeyUp'] = (info) => { @@ -139,7 +154,7 @@ export class Drawing extends StateNode { } } - this.updateShapes() + this.processUpdates() } override onExit? = () => { @@ -281,7 +296,12 @@ export class Drawing extends StateNode { this.initialShape = this.editor.getShape(id) } - private updateShapes() { + /** + * This function is called to process user actions like moving the mouse or pressing a key. + * The updates are not directly propagated to the Editor. Instead they are stored in the `shapePartial` + * and only sent to the Editor on the next tick. + */ + private processUpdates() { const { inputs } = this.editor const { initialShape } = this @@ -296,12 +316,16 @@ export class Drawing extends StateNode { if (!shape) return - const { segments } = shape.props + // We default to the partial, as it might have some segments / points that the editor + // does not know about yet. + const segments = this.shapePartial?.props?.segments || shape.props.segments const { x, y, z } = this.editor.getPointInShapeSpace(shape, inputs.currentPagePoint).toFixed() const newPoint = { x, y, z: this.isPen ? +(z! * 1.25).toFixed(2) : 0.5 } + this.isDirty = true + switch (this.segmentMode) { case 'starting_straight': { const { pagePointWhereNextSegmentChanged } = this @@ -370,9 +394,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { - squashing: true, - }) + this.shapePartial = shapePartial } break } @@ -430,7 +452,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { squashing: true }) + this.shapePartial = shapePartial } break @@ -572,7 +594,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { squashing: true }) + this.shapePartial = shapePartial break } @@ -617,13 +639,19 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { squashing: true }) - - // Set a maximum length for the lines array; after 200 points, complete the line. if (newPoints.length > 500) { - this.editor.updateShapes([{ id, type: this.shapeType, props: { isComplete: true } }]) + // It's easier to just apply this change directly, so we will mark that the shape is no longer dirty. + this.isDirty = false + // Also clear the changes as they were flushed. + // The next pointerMove will establish a new partial from the new shape created below. + this.shapePartial = null - const { currentPagePoint } = this.editor.inputs + if (shapePartial?.props) { + shapePartial.props.isComplete = true + this.editor.updateShapes([shapePartial]) + } + + const { currentPagePoint } = inputs const newShapeId = createShapeId() @@ -647,8 +675,10 @@ export class Drawing extends StateNode { this.initialShape = structuredClone(this.editor.getShape(newShapeId)!) this.mergeNextPoint = false - this.lastRecordedPoint = this.editor.inputs.currentPagePoint.clone() + this.lastRecordedPoint = inputs.currentPagePoint.clone() this.currentLineLength = 0 + } else { + this.shapePartial = shapePartial } break diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts index 91fd887ae..4258dbd0e 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts @@ -28,6 +28,7 @@ export class Brushing extends StateNode { brush = new Box() initialSelectedShapeIds: TLShapeId[] = [] excludedShapeIds = new Set() + isDirty = false isWrapMode = false // The shape that the brush started on @@ -55,9 +56,10 @@ export class Brushing extends StateNode { ) this.info = info + this.isDirty = false this.initialSelectedShapeIds = this.editor.getSelectedShapeIds().slice() this.initialStartShape = this.editor.getShapesAtPoint(currentPagePoint)[0] - this.onPointerMove() + this.hitTestShapes() } override onExit = () => { @@ -67,10 +69,14 @@ export class Brushing extends StateNode { override onTick: TLTickEventHandler = () => { moveCameraWhenCloseToEdge(this.editor) + if (this.isDirty) { + this.isDirty = false + this.hitTestShapes() + } } override onPointerMove = () => { - this.hitTestShapes() + this.isDirty = true } override onPointerUp: TLEventHandlers['onPointerUp'] = () => { @@ -99,6 +105,8 @@ export class Brushing extends StateNode { } private complete() { + this.hitTestShapes() + this.isDirty = false this.parent.transition('idle') } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts index 64348c289..167253693 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts @@ -26,6 +26,7 @@ export class Cropping extends StateNode { } markId = '' + isDirty = false private snapshot = {} as any as Snapshot @@ -40,11 +41,19 @@ export class Cropping extends StateNode { this.markId = 'cropping' this.editor.mark(this.markId) this.snapshot = this.createSnapshot() + this.isDirty = false this.updateShapes() } + override onTick = () => { + if (this.isDirty) { + this.isDirty = false + this.updateShapes() + } + } + override onPointerMove: TLEventHandlers['onPointerMove'] = () => { - this.updateShapes() + this.isDirty = true } override onPointerUp: TLEventHandlers['onPointerUp'] = () => { @@ -205,6 +214,8 @@ export class Cropping extends StateNode { } private complete() { + this.updateShapes() + this.isDirty = false if (this.info.onInteractionEnd) { this.editor.setCurrentTool(this.info.onInteractionEnd, this.info) } else { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx b/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx index 471ca1275..e4fc689f0 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx @@ -39,6 +39,7 @@ export class DraggingHandle extends StateNode { isPrecise = false isPreciseId = null as TLShapeId | null pointingId = null as TLShapeId | null + isDirty = false override onEnter: TLEnterEventHandler = ( info: TLPointerEventInfo & { @@ -50,6 +51,7 @@ export class DraggingHandle extends StateNode { ) => { const { shape, isCreating, handle } = info this.info = info + this.isDirty = false this.parent.setCurrentToolIdMask(info.onInteractionEnd) this.shapeId = shape.id this.markId = isCreating ? `creating:${shape.id}` : 'dragging handle' @@ -165,8 +167,15 @@ export class DraggingHandle extends StateNode { } } + override onTick = () => { + if (this.isDirty) { + this.isDirty = false + this.update() + } + } + override onPointerMove: TLEventHandlers['onPointerMove'] = () => { - this.update() + this.isDirty = true } override onKeyDown: TLKeyboardEvent | undefined = () => { @@ -182,6 +191,8 @@ export class DraggingHandle extends StateNode { } override onComplete: TLEventHandlers['onComplete'] = () => { + this.update() + this.isDirty = false this.complete() } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts index 5860d1ea9..6c9a76280 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts @@ -76,10 +76,15 @@ export class Resizing extends StateNode { override onTick: TLTickEventHandler = () => { moveCameraWhenCloseToEdge(this.editor) + if (!this.isDirty) return + this.isDirty = false + this.updateShapes() } + isDirty = false + override onPointerMove: TLEventHandlers['onPointerMove'] = () => { - this.updateShapes() + this.isDirty = true } override onKeyDown: TLEventHandlers['onKeyDown'] = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts index 0378d147d..4527301e8 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts @@ -22,6 +22,7 @@ export class Rotating extends StateNode { info = {} as Extract & { onInteractionEnd?: string } markId = '' + isDirty = false override onEnter = ( info: TLPointerEventInfo & { target: 'selection'; onInteractionEnd?: string } @@ -38,7 +39,24 @@ export class Rotating extends StateNode { this.snapshot = snapshot // Trigger a pointer move - this.handleStart() + const newSelectionRotation = this._getRotationFromPointerPosition({ + snapToNearestDegree: false, + }) + + applyRotationToSnapshotShapes({ + editor: this.editor, + delta: this._getRotationFromPointerPosition({ snapToNearestDegree: false }), + snapshot: this.snapshot, + stage: 'start', + }) + + // Update cursor + this.editor.updateInstanceState({ + cursor: { + type: CursorTypeMap[this.info.handle as RotateCorner], + rotation: newSelectionRotation + this.snapshot.initialSelectionRotation, + }, + }) } override onExit = () => { @@ -48,8 +66,15 @@ export class Rotating extends StateNode { this.snapshot = {} as TLRotationSnapshot } + override onTick = () => { + if (this.isDirty) { + this.isDirty = false + this.update() + } + } + override onPointerMove = () => { - this.update() + this.isDirty = true } override onKeyDown = () => { @@ -118,27 +143,6 @@ export class Rotating extends StateNode { } } - protected handleStart() { - const newSelectionRotation = this._getRotationFromPointerPosition({ - snapToNearestDegree: false, - }) - - applyRotationToSnapshotShapes({ - editor: this.editor, - delta: this._getRotationFromPointerPosition({ snapToNearestDegree: false }), - snapshot: this.snapshot, - stage: 'start', - }) - - // Update cursor - this.editor.updateInstanceState({ - cursor: { - type: CursorTypeMap[this.info.handle as RotateCorner], - rotation: newSelectionRotation + this.snapshot.initialSelectionRotation, - }, - }) - } - _getRotationFromPointerPosition({ snapToNearestDegree }: { snapToNearestDegree: boolean }) { const selectionRotation = this.editor.getSelectionRotation() const selectionBounds = this.editor.getSelectionRotatedPageBounds() diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts index 7de3e5b85..75e83c1ca 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts @@ -24,6 +24,8 @@ export class ScribbleBrushing extends StateNode { initialSelectedShapeIds = new Set() newlySelectedShapeIds = new Set() + isDirty = false + override onEnter = () => { this.initialSelectedShapeIds = new Set( this.editor.inputs.shiftKey ? this.editor.getSelectedShapeIds() : [] @@ -31,6 +33,7 @@ export class ScribbleBrushing extends StateNode { this.newlySelectedShapeIds = new Set() this.size = 0 this.hits.clear() + this.isDirty = false const scribbleItem = this.editor.scribbles.addScribble({ color: 'selection-stroke', @@ -51,8 +54,15 @@ export class ScribbleBrushing extends StateNode { this.editor.scribbles.stop(this.scribbleId) } + override onTick = () => { + if (this.isDirty) { + this.isDirty = false + this.updateScribbleSelection(true) + } + } + override onPointerMove = () => { - this.updateScribbleSelection(true) + this.isDirty = true } override onPointerUp = () => { @@ -157,6 +167,8 @@ export class ScribbleBrushing extends StateNode { } private complete() { + this.updateScribbleSelection(true) + this.isDirty = false this.parent.transition('idle') } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts index dc3c7e1d3..2952c3857 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts @@ -36,6 +36,7 @@ export class Translating extends StateNode { isCloning = false isCreating = false + isDirty = false onCreate: (shape: TLShape | null) => void = () => void null dragAndDropManager = new DragAndDropManager(this.editor) @@ -51,6 +52,7 @@ export class Translating extends StateNode { const { isCreating = false, onCreate = () => void null } = info this.info = info + this.isDirty = false this.parent.setCurrentToolIdMask(info.onInteractionEnd) this.isCreating = isCreating this.onCreate = onCreate @@ -99,10 +101,14 @@ export class Translating extends StateNode { this.updateParentTransforms ) moveCameraWhenCloseToEdge(this.editor) + if (this.isDirty) { + this.isDirty = false + this.updateShapes() + } } override onPointerMove = () => { - this.updateShapes() + this.isDirty = true } override onKeyDown = () => { @@ -167,6 +173,7 @@ export class Translating extends StateNode { protected complete() { this.updateShapes() + this.isDirty = false this.dragAndDropManager.dropShapes(this.snapshot.movingShapes) this.handleEnd() diff --git a/packages/tldraw/src/test/TestEditor.ts b/packages/tldraw/src/test/TestEditor.ts index e103951a1..bb8e8a2a0 100644 --- a/packages/tldraw/src/test/TestEditor.ts +++ b/packages/tldraw/src/test/TestEditor.ts @@ -289,6 +289,17 @@ export class TestEditor extends Editor { /* ------------------ Input Events ------------------ */ + /** + Some of our updates are not synchronous any longer. For example, drawing happens on tick instead of on pointer move. + You can use this helper to force the tick, which will then process all the updates. + */ + forceTick = (count = 1) => { + for (let i = 0; i < count; i++) { + this.emit('tick', 16) + } + return this + } + pointerMove = ( x = this.inputs.currentScreenPoint.x, y = this.inputs.currentScreenPoint.y, @@ -298,7 +309,7 @@ export class TestEditor extends Editor { this.dispatch({ ...this.getPointerEventInfo(x, y, options, modifiers), name: 'pointer_move', - }) + }).forceTick() return this } diff --git a/packages/tldraw/src/test/drawing.test.ts b/packages/tldraw/src/test/drawing.test.ts index 8cd9a48e2..76bfd06cf 100644 --- a/packages/tldraw/src/test/drawing.test.ts +++ b/packages/tldraw/src/test/drawing.test.ts @@ -203,7 +203,7 @@ for (const toolType of ['draw', 'highlight'] as const) { .pointerDown(10, 0) .pointerUp() .pointerDown(10, 0) - .pointerMove(1, 0) + .pointerMove(1, 0) // very close to first point const shape1 = editor.getCurrentPageShapes()[0] as DrawableShape const segment1 = last(shape1.props.segments)! @@ -211,6 +211,7 @@ for (const toolType of ['draw', 'highlight'] as const) { expect(point1.x).toBe(1) editor.keyDown('Meta') + editor.forceTick() const shape2 = editor.getCurrentPageShapes()[0] as DrawableShape const segment2 = last(shape2.props.segments)! const point2 = last(segment2.points)! @@ -236,6 +237,7 @@ for (const toolType of ['draw', 'highlight'] as const) { expect(point1.x).toBe(1) editor.keyDown('Meta') + editor.forceTick() const shape2 = editor.getCurrentPageShapes()[0] as DrawableShape const segment2 = last(shape2.props.segments)! const point2 = last(segment2.points)! diff --git a/packages/tldraw/src/test/translating.test.ts b/packages/tldraw/src/test/translating.test.ts index b3e87a957..7d038c216 100644 --- a/packages/tldraw/src/test/translating.test.ts +++ b/packages/tldraw/src/test/translating.test.ts @@ -119,12 +119,15 @@ describe('When translating...', () => { it('translates a single shape', () => { editor - .pointerDown(50, 50, ids.box1) + .pointerDown(50, 50, ids.box1) // point = [10, 10] .pointerMove(50, 40) // [0, -10] + .expectToBeIn('select.translating') .expectShapeToMatch({ id: ids.box1, x: 10, y: 0 }) .pointerMove(100, 100) // [50, 50] + .expectToBeIn('select.translating') .expectShapeToMatch({ id: ids.box1, x: 60, y: 60 }) .pointerUp() + .expectToBeIn('select.idle') .expectShapeToMatch({ id: ids.box1, x: 60, y: 60 }) }) @@ -134,14 +137,14 @@ describe('When translating...', () => { const before = editor.getShape(ids.box1)! - jest.advanceTimersByTime(100) + editor.forceTick(5) editor // The change is bigger than expected because the camera moves .expectShapeToMatch({ id: ids.box1, x: -160, y: 10 }) // We'll continue moving in the x postion, but now we'll also move in the y position. // The speed in the y position is smaller since we are further away from the edge. .pointerMove(0, 25) - jest.advanceTimersByTime(100) + editor.forceTick(2) editor.pointerUp() const after = editor.getShape(ids.box1)! @@ -156,12 +159,12 @@ describe('When translating...', () => { editor.user.updateUserPreferences({ edgeScrollSpeed: 1 }) editor.pointerDown(50, 50, ids.box1).pointerMove(1080, 50) - jest.advanceTimersByTime(100) editor + .forceTick(4) // The change is bigger than expected because the camera moves .expectShapeToMatch({ id: ids.box1, x: 1140, y: 10 }) .pointerMove(1080, 800) - jest.advanceTimersByTime(100) + .forceTick(6) editor .expectShapeToMatch({ id: ids.box1, x: 1280, y: 845.68 }) .pointerUp() diff --git a/packages/tlsync/src/lib/TLSyncClient.ts b/packages/tlsync/src/lib/TLSyncClient.ts index df7e7ae99..b370553b0 100644 --- a/packages/tlsync/src/lib/TLSyncClient.ts +++ b/packages/tlsync/src/lib/TLSyncClient.ts @@ -7,7 +7,7 @@ import { reverseRecordsDiff, squashRecordDiffs, } from '@tldraw/store' -import { exhaustiveSwitchError, objectMapEntries, rafThrottle } from '@tldraw/utils' +import { exhaustiveSwitchError, fpsThrottle, objectMapEntries } from '@tldraw/utils' import isEqual from 'lodash.isequal' import { nanoid } from 'nanoid' import { NetworkDiff, RecordOpType, applyObjectDiff, diffRecord, getNetworkDiff } from './diff' @@ -461,7 +461,7 @@ export class TLSyncClient = Store } /** Send any unsent push requests to the server */ - private flushPendingPushRequests = rafThrottle(() => { + private flushPendingPushRequests = fpsThrottle(() => { this.debug('flushing pending push requests', { isConnectedToRoom: this.isConnectedToRoom, pendingPushRequests: this.pendingPushRequests, @@ -587,5 +587,5 @@ export class TLSyncClient = Store } } - private scheduleRebase = rafThrottle(this.rebase) + private scheduleRebase = fpsThrottle(this.rebase) } diff --git a/packages/utils/api-report.md b/packages/utils/api-report.md index 911cc0e5f..ab0a93439 100644 --- a/packages/utils/api-report.md +++ b/packages/utils/api-report.md @@ -74,6 +74,9 @@ export function filterEntries(object: { [K in Key]: Value; }; +// @internal +export function fpsThrottle(fn: () => void): () => void; + // @internal (undocumented) export function getErrorAnnotations(error: Error): ErrorAnnotations; @@ -264,9 +267,6 @@ export function promiseWithResolve(): Promise & { reject: (reason?: any) => void; }; -// @internal -export function rafThrottle(fn: () => void): () => void; - // @public (undocumented) export type RecursivePartial = { [P in keyof T]?: RecursivePartial; @@ -315,7 +315,7 @@ export { structuredClone_2 as structuredClone } export function throttle any>(func: T, limit: number): (...args: Parameters) => ReturnType; // @internal -export function throttledRaf(fn: () => void): void; +export function throttleToNextFrame(fn: () => void): void; // @internal (undocumented) export function validateIndexKey(key: string): asserts key is IndexKey; diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 3ac1d8712..222d78917 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -38,7 +38,6 @@ export { objectMapValues, } from './lib/object' export { PngHelpers } from './lib/png' -export { rafThrottle, throttledRaf } from './lib/raf' export { type IndexKey } from './lib/reordering/IndexKey' export { ZERO_INDEX_KEY, @@ -63,6 +62,7 @@ export { setInLocalStorage, setInSessionStorage, } from './lib/storage' +export { fpsThrottle, throttleToNextFrame } from './lib/throttle' export type { Expand, RecursivePartial, Required } from './lib/types' export { isDefined, isNonNull, isNonNullish, structuredClone } from './lib/value' export { warnDeprecatedGetter } from './lib/warnDeprecatedGetter' diff --git a/packages/utils/src/lib/raf.ts b/packages/utils/src/lib/raf.ts deleted file mode 100644 index 8078b6191..000000000 --- a/packages/utils/src/lib/raf.ts +++ /dev/null @@ -1,67 +0,0 @@ -const isTest = () => - typeof process !== 'undefined' && - process.env.NODE_ENV === 'test' && - // @ts-expect-error - !globalThis.__FORCE_RAF_IN_TESTS__ - -const rafQueue: Array<() => void> = [] - -const tick = () => { - const queue = rafQueue.splice(0, rafQueue.length) - for (const fn of queue) { - fn() - } -} - -let frame: number | undefined - -function raf() { - if (frame) { - return - } - - frame = requestAnimationFrame(() => { - frame = undefined - tick() - }) -} - -/** - * Returns a throttled version of the function that will only be called max once per frame. - * @param fn - the fun to return a throttled version of - * @returns - * @internal - */ -export function rafThrottle(fn: () => void) { - if (isTest()) { - return fn - } - - return () => { - if (rafQueue.includes(fn)) { - return - } - rafQueue.push(fn) - raf() - } -} - -/** - * Calls the function on the next frame. - * 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 animation frame - * @returns - * @internal - */ -export function throttledRaf(fn: () => void) { - if (isTest()) { - return fn() - } - - if (rafQueue.includes(fn)) { - return - } - - rafQueue.push(fn) - raf() -} diff --git a/packages/utils/src/lib/throttle.ts b/packages/utils/src/lib/throttle.ts new file mode 100644 index 000000000..c39eca537 --- /dev/null +++ b/packages/utils/src/lib/throttle.ts @@ -0,0 +1,96 @@ +const isTest = () => + typeof process !== 'undefined' && + process.env.NODE_ENV === 'test' && + // @ts-expect-error + !globalThis.__FORCE_RAF_IN_TESTS__ + +const fpsQueue: Array<() => void> = [] +const targetFps = 60 +const targetTimePerFrame = 1000 / targetFps +let frame: number | undefined +let time = 0 +let last = 0 + +const flush = () => { + const queue = fpsQueue.splice(0, fpsQueue.length) + for (const fn of queue) { + fn() + } +} + +function tick() { + if (frame) { + return + } + const now = Date.now() + const elapsed = now - last + + if (time + elapsed < targetTimePerFrame) { + frame = requestAnimationFrame(() => { + frame = undefined + tick() + }) + return + } + frame = requestAnimationFrame(() => { + frame = undefined + last = now + // If we fall behind more than 10 frames, we'll just reset the time so we don't try to update a number of times + // This can happen if we don't interact with the page for a while + time = Math.min(time + elapsed - targetTimePerFrame, targetTimePerFrame * 10) + flush() + }) +} + +let started = false + +/** + * Returns a throttled version of the function that will only be called max once per frame. + * The target frame rate is 60fps. + * @param fn - the fun to return a throttled version of + * @returns + * @internal + */ +export function fpsThrottle(fn: () => void) { + if (isTest()) { + return fn + } + + return () => { + if (fpsQueue.includes(fn)) { + return + } + 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() + } +} + +/** + * 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 + * @internal + */ +export function throttleToNextFrame(fn: () => void) { + if (isTest()) { + return fn() + } + + if (fpsQueue.includes(fn)) { + return + } + + 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() +} diff --git a/yarn.lock b/yarn.lock index c53bbc5fd..b81c7452e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7356,6 +7356,7 @@ __metadata: version: 0.0.0-use.local resolution: "@tldraw/state@workspace:packages/state" dependencies: + "@tldraw/utils": "workspace:*" "@types/lodash": "npm:^4.14.188" "@types/react": "npm:^18.2.47" "@types/react-test-renderer": "npm:^18.0.0"