Revert perf changes (#3217)

Step 1 of the master plan 😂 

![CleanShot 2024-03-19 at 16 05
08](https://github.com/tldraw/tldraw/assets/2523721/7d2afed9-7b69-4fdb-8b9f-54a48c61258f)

This:
- Reverts #3186 
- Reverts #3160 (there were some conflicting changes so it's not a
straight revert)
- Reverts most of #2977 


### Change Type

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

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

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

- [ ] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [ ] `improvement` — Improving existing features
- [x] `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
This commit is contained in:
Mitja Bezenšek 2024-03-21 11:05:44 +01:00 committed by GitHub
parent d5dc306314
commit cd02d03d06
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
24 changed files with 179 additions and 360 deletions

View file

@ -29,8 +29,8 @@
"dev": "vite --host", "dev": "vite --host",
"build": "vite build", "build": "vite build",
"lint": "yarn run -T tsx ../../scripts/lint.ts", "lint": "yarn run -T tsx ../../scripts/lint.ts",
"e2e": "NODE_ENV=test && playwright test -c ./e2e/playwright.config.ts", "e2e": "playwright test -c ./e2e/playwright.config.ts",
"e2e-ui": "NODE_ENV=test && playwright test --ui -c ./e2e/playwright.config.ts" "e2e-ui": "playwright test --ui -c ./e2e/playwright.config.ts"
}, },
"dependencies": { "dependencies": {
"@playwright/test": "^1.38.1", "@playwright/test": "^1.38.1",

View file

@ -1,7 +1,7 @@
import { useLayoutReaction, useStateTracking } from '@tldraw/state' import { useQuickReactor, useStateTracking } from '@tldraw/state'
import { IdOf } from '@tldraw/store' import { IdOf } from '@tldraw/store'
import { TLShape, TLShapeId } from '@tldraw/tlschema' import { TLShape, TLShapeId } from '@tldraw/tlschema'
import { memo, useCallback, useLayoutEffect, useRef } from 'react' import { memo, useCallback, useRef } from 'react'
import { ShapeUtil } from '../editor/shapes/ShapeUtil' import { ShapeUtil } from '../editor/shapes/ShapeUtil'
import { useEditor } from '../hooks/useEditor' import { useEditor } from '../hooks/useEditor'
import { useEditorComponents } from '../hooks/useEditorComponents' import { useEditorComponents } from '../hooks/useEditorComponents'
@ -54,60 +54,68 @@ export const Shape = memo(function Shape({
height: 0, height: 0,
}) })
useLayoutReaction('set shape stuff', () => { useQuickReactor(
const shape = editor.getShape(id) 'set shape stuff',
if (!shape) return // probably the shape was just deleted () => {
const shape = editor.getShape(id)
if (!shape) return // probably the shape was just deleted
const prev = memoizedStuffRef.current const prev = memoizedStuffRef.current
// Clip path // Clip path
const clipPath = editor.getShapeClipPath(id) ?? 'none' const clipPath = editor.getShapeClipPath(id) ?? 'none'
if (clipPath !== prev.clipPath) { if (clipPath !== prev.clipPath) {
setStyleProperty(containerRef.current, 'clip-path', clipPath) setStyleProperty(containerRef.current, 'clip-path', clipPath)
setStyleProperty(bgContainerRef.current, 'clip-path', clipPath) setStyleProperty(bgContainerRef.current, 'clip-path', clipPath)
prev.clipPath = clipPath prev.clipPath = clipPath
} }
// Page transform // Page transform
const transform = Mat.toCssString(editor.getShapePageTransform(id)) const transform = Mat.toCssString(editor.getShapePageTransform(id))
if (transform !== prev.transform) { if (transform !== prev.transform) {
setStyleProperty(containerRef.current, 'transform', transform) setStyleProperty(containerRef.current, 'transform', transform)
setStyleProperty(bgContainerRef.current, 'transform', transform) setStyleProperty(bgContainerRef.current, 'transform', transform)
prev.transform = transform prev.transform = transform
} }
// Width / Height // Width / Height
// We round the shape width and height up to the nearest multiple of dprMultiple // We round the shape width and height up to the nearest multiple of dprMultiple
// to avoid the browser making miscalculations when applying the transform. // to avoid the browser making miscalculations when applying the transform.
const bounds = editor.getShapeGeometry(shape).bounds const bounds = editor.getShapeGeometry(shape).bounds
const widthRemainder = bounds.w % dprMultiple const widthRemainder = bounds.w % dprMultiple
const heightRemainder = bounds.h % dprMultiple const heightRemainder = bounds.h % dprMultiple
const width = widthRemainder === 0 ? bounds.w : bounds.w + (dprMultiple - widthRemainder) const width = widthRemainder === 0 ? bounds.w : bounds.w + (dprMultiple - widthRemainder)
const height = heightRemainder === 0 ? bounds.h : bounds.h + (dprMultiple - heightRemainder) const height = heightRemainder === 0 ? bounds.h : bounds.h + (dprMultiple - heightRemainder)
if (width !== prev.width || height !== prev.height) { if (width !== prev.width || height !== prev.height) {
setStyleProperty(containerRef.current, 'width', Math.max(width, dprMultiple) + 'px') setStyleProperty(containerRef.current, 'width', Math.max(width, dprMultiple) + 'px')
setStyleProperty(containerRef.current, 'height', Math.max(height, dprMultiple) + 'px') setStyleProperty(containerRef.current, 'height', Math.max(height, dprMultiple) + 'px')
setStyleProperty(bgContainerRef.current, 'width', Math.max(width, dprMultiple) + 'px') setStyleProperty(bgContainerRef.current, 'width', Math.max(width, dprMultiple) + 'px')
setStyleProperty(bgContainerRef.current, 'height', Math.max(height, dprMultiple) + 'px') setStyleProperty(bgContainerRef.current, 'height', Math.max(height, dprMultiple) + 'px')
prev.width = width prev.width = width
prev.height = height prev.height = height
} }
}) },
[editor]
)
// This stuff changes pretty infrequently, so we can change them together // This stuff changes pretty infrequently, so we can change them together
useLayoutEffect(() => { useQuickReactor(
const container = containerRef.current 'set opacity and z-index',
const bgContainer = bgContainerRef.current () => {
const container = containerRef.current
const bgContainer = bgContainerRef.current
// Opacity // Opacity
setStyleProperty(container, 'opacity', opacity) setStyleProperty(container, 'opacity', opacity)
setStyleProperty(bgContainer, 'opacity', opacity) setStyleProperty(bgContainer, 'opacity', opacity)
// Z-Index // Z-Index
setStyleProperty(container, 'z-index', index) setStyleProperty(container, 'z-index', index)
setStyleProperty(bgContainer, 'z-index', backgroundIndex) setStyleProperty(bgContainer, 'z-index', backgroundIndex)
}, [opacity, index, backgroundIndex]) },
[opacity, index, backgroundIndex]
)
const annotateError = useCallback( const annotateError = useCallback(
(error: any) => editor.annotateError(error, { origin: 'shape', willCrashApp: false }), (error: any) => editor.annotateError(error, { origin: 'shape', willCrashApp: false }),
@ -169,14 +177,18 @@ const CulledShape = function CulledShape<T extends TLShape>({ shapeId }: { shape
const editor = useEditor() const editor = useEditor()
const culledRef = useRef<HTMLDivElement>(null) const culledRef = useRef<HTMLDivElement>(null)
useLayoutReaction('set shape stuff', () => { useQuickReactor(
const bounds = editor.getShapeGeometry(shapeId).bounds 'set shape stuff',
setStyleProperty( () => {
culledRef.current, const bounds = editor.getShapeGeometry(shapeId).bounds
'transform', setStyleProperty(
`translate(${toDomPrecision(bounds.minX)}px, ${toDomPrecision(bounds.minY)}px)` culledRef.current,
) 'transform',
}) `translate(${toDomPrecision(bounds.minX)}px, ${toDomPrecision(bounds.minY)}px)`
)
},
[editor]
)
return <div ref={culledRef} className="tl-shape__culled" /> return <div ref={culledRef} className="tl-shape__culled" />
} }

View file

@ -1,4 +1,4 @@
import { react, useLayoutReaction, useValue } from '@tldraw/state' import { react, useQuickReactor, useValue } from '@tldraw/state'
import { TLHandle, TLShapeId } from '@tldraw/tlschema' import { TLHandle, TLShapeId } from '@tldraw/tlschema'
import { dedupe, modulate, objectMapValues } from '@tldraw/utils' import { dedupe, modulate, objectMapValues } from '@tldraw/utils'
import classNames from 'classnames' import classNames from 'classnames'
@ -43,21 +43,25 @@ export function DefaultCanvas({ className }: TLCanvasComponentProps) {
useGestureEvents(rCanvas) useGestureEvents(rCanvas)
useFixSafariDoubleTapZoomPencilEvents(rCanvas) useFixSafariDoubleTapZoomPencilEvents(rCanvas)
useLayoutReaction('position layers', () => { useQuickReactor(
const { x, y, z } = editor.getCamera() 'position layers',
() => {
const { x, y, z } = editor.getCamera()
// Because the html container has a width/height of 1px, we // Because the html container has a width/height of 1px, we
// need to create a small offset when zoomed to ensure that // need to create a small offset when zoomed to ensure that
// the html container and svg container are lined up exactly. // the html container and svg container are lined up exactly.
const offset = const offset =
z >= 1 ? modulate(z, [1, 8], [0.125, 0.5], true) : modulate(z, [0.1, 1], [-2, 0.125], true) z >= 1 ? modulate(z, [1, 8], [0.125, 0.5], true) : modulate(z, [0.1, 1], [-2, 0.125], true)
const transform = `scale(${toDomPrecision(z)}) translate(${toDomPrecision( const transform = `scale(${toDomPrecision(z)}) translate(${toDomPrecision(
x + offset x + offset
)}px,${toDomPrecision(y + offset)}px)` )}px,${toDomPrecision(y + offset)}px)`
setStyleProperty(rHtmlLayer.current, 'transform', transform) setStyleProperty(rHtmlLayer.current, 'transform', transform)
setStyleProperty(rHtmlLayer2.current, 'transform', transform) setStyleProperty(rHtmlLayer2.current, 'transform', transform)
}) },
[editor]
)
const events = useCanvasEvents() const events = useCanvasEvents()

View file

@ -128,9 +128,6 @@ export function useComputed<Value>(name: string, compute: () => Value, deps: any
// @public (undocumented) // @public (undocumented)
export function useComputed<Value, Diff = unknown>(name: string, compute: () => Value, opts: ComputedOptions<Value, Diff>, deps: any[]): Computed<Value>; export function useComputed<Value, Diff = unknown>(name: string, compute: () => Value, opts: ComputedOptions<Value, Diff>, deps: any[]): Computed<Value>;
// @internal (undocumented)
export function useLayoutReaction(name: string, effect: () => void): void;
// @public (undocumented) // @public (undocumented)
export function useQuickReactor(name: string, reactFn: () => void, deps?: any[]): void; export function useQuickReactor(name: string, reactFn: () => void, deps?: any[]): void;

View file

@ -52,9 +52,6 @@
"node_modules/(?!(nanoid)/)" "node_modules/(?!(nanoid)/)"
] ]
}, },
"dependencies": {
"@tldraw/utils": "workspace:*"
},
"devDependencies": { "devDependencies": {
"@types/lodash": "^4.14.188", "@types/lodash": "^4.14.188",
"@types/react": "^18.2.47", "@types/react": "^18.2.47",

View file

@ -51,7 +51,6 @@ class __EffectScheduler__<Result> {
lastTraversedEpoch = GLOBAL_START_EPOCH lastTraversedEpoch = GLOBAL_START_EPOCH
private lastReactedEpoch = GLOBAL_START_EPOCH private lastReactedEpoch = GLOBAL_START_EPOCH
private hasPendingEffect = true
private _scheduleCount = 0 private _scheduleCount = 0
/** /**
@ -95,7 +94,6 @@ class __EffectScheduler__<Result> {
/** @internal */ /** @internal */
scheduleEffect() { scheduleEffect() {
this._scheduleCount++ this._scheduleCount++
this.hasPendingEffect = true
if (this._scheduleEffect) { if (this._scheduleEffect) {
// if the effect should be deferred (e.g. until a react render), do so // if the effect should be deferred (e.g. until a react render), do so
this._scheduleEffect(this.maybeExecute) this._scheduleEffect(this.maybeExecute)
@ -108,7 +106,7 @@ class __EffectScheduler__<Result> {
/** @internal */ /** @internal */
readonly maybeExecute = () => { readonly maybeExecute = () => {
// bail out if we have been detached before this runs // bail out if we have been detached before this runs
if (!this._isActivelyListening || !this.hasPendingEffect) return if (!this._isActivelyListening) return
this.execute() this.execute()
} }
@ -144,7 +142,6 @@ class __EffectScheduler__<Result> {
try { try {
startCapturingParents(this) startCapturingParents(this)
const result = this.runEffect(this.lastReactedEpoch) const result = this.runEffect(this.lastReactedEpoch)
this.hasPendingEffect = false
this.lastReactedEpoch = getGlobalEpoch() this.lastReactedEpoch = getGlobalEpoch()
return result return result
} finally { } finally {

View file

@ -1,7 +1,6 @@
export { track } from './track' export { track } from './track'
export { useAtom } from './useAtom' export { useAtom } from './useAtom'
export { useComputed } from './useComputed' export { useComputed } from './useComputed'
export { useLayoutReaction } from './useLayoutReaction'
export { useQuickReactor } from './useQuickReactor' export { useQuickReactor } from './useQuickReactor'
export { useReactor } from './useReactor' export { useReactor } from './useReactor'
export { useStateTracking } from './useStateTracking' export { useStateTracking } from './useStateTracking'

View file

@ -1,10 +0,0 @@
import React from 'react'
import { useTrackedScheduler } from './useTrackedScheduler'
/** @internal */
export function useLayoutReaction(name: string, effect: () => void): void {
const scheduler = useTrackedScheduler(name, effect)
// eslint-disable-next-line react-hooks/exhaustive-deps
React.useLayoutEffect(scheduler.maybeExecute)
}

View file

@ -1,6 +1,60 @@
import { useTrackedScheduler } from './useTrackedScheduler' import React from 'react'
import { EffectScheduler } from '../core'
/** @internal */ /** @internal */
export function useStateTracking<T>(name: string, render: () => T): T { export function useStateTracking<T>(name: string, render: () => T): T {
return useTrackedScheduler(name, render).execute() // This hook creates an effect scheduler that will trigger re-renders when its reactive dependencies change, but it
// defers the actual execution of the effect to the consumer of this hook.
// We need the exec fn to always be up-to-date when calling scheduler.execute() but it'd be wasteful to
// instantiate a new EffectScheduler on every render, so we use an immediately-updated ref
// to wrap it
const renderRef = React.useRef(render)
renderRef.current = render
const [scheduler, subscribe, getSnapshot] = React.useMemo(() => {
let scheduleUpdate = null as null | (() => void)
// useSyncExternalStore requires a subscribe function that returns an unsubscribe function
const subscribe = (cb: () => void) => {
scheduleUpdate = cb
return () => {
scheduleUpdate = null
}
}
const scheduler = new EffectScheduler(
`useStateTracking(${name})`,
// this is what `scheduler.execute()` will call
() => renderRef.current?.(),
// this is what will be invoked when @tldraw/state detects a change in an upstream reactive value
{
scheduleEffect() {
scheduleUpdate?.()
},
}
)
// we use an incrementing number based on when this
const getSnapshot = () => scheduler.scheduleCount
return [scheduler, subscribe, getSnapshot]
}, [name])
React.useSyncExternalStore(subscribe, getSnapshot, getSnapshot)
// reactive dependencies are captured when `scheduler.execute()` is called
// and then to make it reactive we wait for a `useEffect` to 'attach'
// this allows us to avoid rendering outside of React's render phase
// and avoid 'zombie' components that try to render with bad/deleted data before
// react has a chance to umount them.
React.useEffect(() => {
scheduler.attach()
// do not execute, we only do that in render
scheduler.maybeScheduleEffect()
return () => {
scheduler.detach()
}
}, [scheduler])
return scheduler.execute()
} }

View file

@ -1,61 +0,0 @@
import { fpsThrottle } from '@tldraw/utils'
import React from 'react'
import { EffectScheduler } from '../core'
/** @internal */
export function useTrackedScheduler<T>(name: string, exec: () => T): EffectScheduler<T> {
// This hook creates an effect scheduler that will trigger re-renders when its reactive dependencies change, but it
// defers the actual execution of the effect to the consumer of this hook.
// We need the exec fn to always be up-to-date when calling scheduler.execute() but it'd be wasteful to
// instantiate a new EffectScheduler on every render, so we use an immediately-updated ref
// to wrap it
const execRef = React.useRef(exec)
execRef.current = exec
const [scheduler, subscribe, getSnapshot] = React.useMemo(() => {
let scheduleUpdate = null as null | (() => void)
// useSyncExternalStore requires a subscribe function that returns an unsubscribe function
const subscribe = (cb: () => void) => {
scheduleUpdate = cb
return () => {
scheduleUpdate = null
}
}
const scheduler = new EffectScheduler(
`useStateTracking(${name})`,
// this is what `scheduler.execute()` will call
() => execRef.current?.(),
// this is what will be invoked when @tldraw/state detects a change in an upstream reactive value
{
scheduleEffect: fpsThrottle(() => {
scheduleUpdate?.()
}),
}
)
// we use an incrementing number based on when this
const getSnapshot = () => scheduler.scheduleCount
return [scheduler, subscribe, getSnapshot]
}, [name])
React.useSyncExternalStore(subscribe, getSnapshot, getSnapshot)
// reactive dependencies are captured when `scheduler.execute()` is called
// and then to make it reactive we wait for a `useEffect` to 'attach'
// this allows us to avoid rendering outside of React's render phase
// and avoid 'zombie' components that try to render with bad/deleted data before
// react has a chance to umount them.
React.useEffect(() => {
scheduler.attach()
// do not execute, we only do that in render
scheduler.maybeScheduleEffect()
return () => {
scheduler.detach()
}
}, [scheduler])
return scheduler
}

View file

@ -1,5 +1,4 @@
/* eslint-disable prefer-rest-params */ /* eslint-disable prefer-rest-params */
import { throttleToNextFrame } from '@tldraw/utils'
import { useMemo, useRef, useSyncExternalStore } from 'react' import { useMemo, useRef, useSyncExternalStore } from 'react'
import { Signal, computed, react } from '../core' import { Signal, computed, react } from '../core'
@ -82,16 +81,10 @@ export function useValue() {
const { subscribe, getSnapshot } = useMemo(() => { const { subscribe, getSnapshot } = useMemo(() => {
return { return {
subscribe: (listen: () => void) => { subscribe: (listen: () => void) => {
return react( return react(`useValue(${name})`, () => {
`useValue(${name})`, $val.get()
() => { listen()
$val.get() })
listen()
},
{
scheduleEffect: throttleToNextFrame,
}
)
}, },
getSnapshot: () => $val.get(), getSnapshot: () => $val.get(),
} }

View file

@ -2,7 +2,6 @@
"extends": "../../config/tsconfig.base.json", "extends": "../../config/tsconfig.base.json",
"include": ["src"], "include": ["src"],
"exclude": ["node_modules", "dist", ".tsbuild*"], "exclude": ["node_modules", "dist", ".tsbuild*"],
"references": [{ "path": "../utils" }],
"compilerOptions": { "compilerOptions": {
"outDir": "./.tsbuild", "outDir": "./.tsbuild",
"rootDir": "src" "rootDir": "src"

View file

@ -51,18 +51,11 @@ export class Drawing extends StateNode {
markId = null as null | string 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<DrawableShape> | null = null
override onEnter = (info: TLPointerEventInfo) => { override onEnter = (info: TLPointerEventInfo) => {
this.markId = null this.markId = null
this.info = info this.info = info
this.canDraw = !this.editor.getIsMenuOpen() this.canDraw = !this.editor.getIsMenuOpen()
this.lastRecordedPoint = this.editor.inputs.currentPagePoint.clone() this.lastRecordedPoint = this.editor.inputs.currentPagePoint.clone()
this.shapePartial = null
this.isDirty = false
if (this.canDraw) { if (this.canDraw) {
this.startShape() this.startShape()
} }
@ -106,18 +99,10 @@ export class Drawing extends StateNode {
this.mergeNextPoint = false this.mergeNextPoint = false
} }
this.processUpdates() this.updateShapes()
} }
} }
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) => { override onKeyDown: TLEventHandlers['onKeyDown'] = (info) => {
if (info.key === 'Shift') { if (info.key === 'Shift') {
switch (this.segmentMode) { switch (this.segmentMode) {
@ -132,7 +117,7 @@ export class Drawing extends StateNode {
} }
} }
} }
this.processUpdates() this.updateShapes()
} }
override onKeyUp: TLEventHandlers['onKeyUp'] = (info) => { override onKeyUp: TLEventHandlers['onKeyUp'] = (info) => {
@ -154,7 +139,7 @@ export class Drawing extends StateNode {
} }
} }
this.processUpdates() this.updateShapes()
} }
override onExit? = () => { override onExit? = () => {
@ -296,12 +281,7 @@ export class Drawing extends StateNode {
this.initialShape = this.editor.getShape<DrawableShape>(id) this.initialShape = this.editor.getShape<DrawableShape>(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 { inputs } = this.editor
const { initialShape } = this const { initialShape } = this
@ -316,16 +296,12 @@ export class Drawing extends StateNode {
if (!shape) return if (!shape) return
// We default to the partial, as it might have some segments / points that the editor const { segments } = shape.props
// 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 { x, y, z } = this.editor.getPointInShapeSpace(shape, inputs.currentPagePoint).toFixed()
const newPoint = { x, y, z: this.isPen ? +(z! * 1.25).toFixed(2) : 0.5 } const newPoint = { x, y, z: this.isPen ? +(z! * 1.25).toFixed(2) : 0.5 }
this.isDirty = true
switch (this.segmentMode) { switch (this.segmentMode) {
case 'starting_straight': { case 'starting_straight': {
const { pagePointWhereNextSegmentChanged } = this const { pagePointWhereNextSegmentChanged } = this
@ -394,7 +370,9 @@ export class Drawing extends StateNode {
) )
} }
this.shapePartial = shapePartial this.editor.updateShapes<TLDrawShape | TLHighlightShape>([shapePartial], {
squashing: true,
})
} }
break break
} }
@ -452,7 +430,7 @@ export class Drawing extends StateNode {
) )
} }
this.shapePartial = shapePartial this.editor.updateShapes([shapePartial], { squashing: true })
} }
break break
@ -594,7 +572,7 @@ export class Drawing extends StateNode {
) )
} }
this.shapePartial = shapePartial this.editor.updateShapes([shapePartial], { squashing: true })
break break
} }
@ -639,19 +617,13 @@ 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) { if (newPoints.length > 500) {
// It's easier to just apply this change directly, so we will mark that the shape is no longer dirty. this.editor.updateShapes([{ id, type: this.shapeType, props: { isComplete: true } }])
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
if (shapePartial?.props) { const { currentPagePoint } = this.editor.inputs
shapePartial.props.isComplete = true
this.editor.updateShapes([shapePartial])
}
const { currentPagePoint } = inputs
const newShapeId = createShapeId() const newShapeId = createShapeId()
@ -675,10 +647,8 @@ export class Drawing extends StateNode {
this.initialShape = structuredClone(this.editor.getShape<DrawableShape>(newShapeId)!) this.initialShape = structuredClone(this.editor.getShape<DrawableShape>(newShapeId)!)
this.mergeNextPoint = false this.mergeNextPoint = false
this.lastRecordedPoint = inputs.currentPagePoint.clone() this.lastRecordedPoint = this.editor.inputs.currentPagePoint.clone()
this.currentLineLength = 0 this.currentLineLength = 0
} else {
this.shapePartial = shapePartial
} }
break break

View file

@ -27,7 +27,6 @@ export class Brushing extends StateNode {
brush = new Box() brush = new Box()
initialSelectedShapeIds: TLShapeId[] = [] initialSelectedShapeIds: TLShapeId[] = []
excludedShapeIds = new Set<TLShapeId>() excludedShapeIds = new Set<TLShapeId>()
isDirty = false
isWrapMode = false isWrapMode = false
// The shape that the brush started on // The shape that the brush started on
@ -55,7 +54,6 @@ export class Brushing extends StateNode {
) )
this.info = info this.info = info
this.isDirty = false
this.initialSelectedShapeIds = this.editor.getSelectedShapeIds().slice() this.initialSelectedShapeIds = this.editor.getSelectedShapeIds().slice()
this.initialStartShape = this.editor.getShapesAtPoint(currentPagePoint)[0] this.initialStartShape = this.editor.getShapesAtPoint(currentPagePoint)[0]
this.hitTestShapes() this.hitTestShapes()
@ -68,14 +66,10 @@ export class Brushing extends StateNode {
override onTick = () => { override onTick = () => {
moveCameraWhenCloseToEdge(this.editor) moveCameraWhenCloseToEdge(this.editor)
if (this.isDirty) {
this.isDirty = false
this.hitTestShapes()
}
} }
override onPointerMove = () => { override onPointerMove = () => {
this.isDirty = true this.hitTestShapes()
} }
override onPointerUp: TLEventHandlers['onPointerUp'] = () => { override onPointerUp: TLEventHandlers['onPointerUp'] = () => {
@ -105,7 +99,6 @@ export class Brushing extends StateNode {
private complete() { private complete() {
this.hitTestShapes() this.hitTestShapes()
this.isDirty = false
this.parent.transition('idle') this.parent.transition('idle')
} }

View file

@ -26,7 +26,6 @@ export class Cropping extends StateNode {
} }
markId = '' markId = ''
isDirty = false
private snapshot = {} as any as Snapshot private snapshot = {} as any as Snapshot
@ -41,19 +40,11 @@ export class Cropping extends StateNode {
this.markId = 'cropping' this.markId = 'cropping'
this.editor.mark(this.markId) this.editor.mark(this.markId)
this.snapshot = this.createSnapshot() this.snapshot = this.createSnapshot()
this.isDirty = false
this.updateShapes() this.updateShapes()
} }
override onTick = () => {
if (this.isDirty) {
this.isDirty = false
this.updateShapes()
}
}
override onPointerMove: TLEventHandlers['onPointerMove'] = () => { override onPointerMove: TLEventHandlers['onPointerMove'] = () => {
this.isDirty = true this.updateShapes()
} }
override onPointerUp: TLEventHandlers['onPointerUp'] = () => { override onPointerUp: TLEventHandlers['onPointerUp'] = () => {
@ -215,7 +206,6 @@ export class Cropping extends StateNode {
private complete() { private complete() {
this.updateShapes() this.updateShapes()
this.isDirty = false
if (this.info.onInteractionEnd) { if (this.info.onInteractionEnd) {
this.editor.setCurrentTool(this.info.onInteractionEnd, this.info) this.editor.setCurrentTool(this.info.onInteractionEnd, this.info)
} else { } else {

View file

@ -39,7 +39,6 @@ export class DraggingHandle extends StateNode {
isPrecise = false isPrecise = false
isPreciseId = null as TLShapeId | null isPreciseId = null as TLShapeId | null
pointingId = null as TLShapeId | null pointingId = null as TLShapeId | null
isDirty = false
override onEnter: TLEnterEventHandler = ( override onEnter: TLEnterEventHandler = (
info: TLPointerEventInfo & { info: TLPointerEventInfo & {
@ -51,7 +50,6 @@ export class DraggingHandle extends StateNode {
) => { ) => {
const { shape, isCreating, handle } = info const { shape, isCreating, handle } = info
this.info = info this.info = info
this.isDirty = false
this.parent.setCurrentToolIdMask(info.onInteractionEnd) this.parent.setCurrentToolIdMask(info.onInteractionEnd)
this.shapeId = shape.id this.shapeId = shape.id
this.markId = isCreating ? `creating:${shape.id}` : 'dragging handle' this.markId = isCreating ? `creating:${shape.id}` : 'dragging handle'
@ -167,15 +165,8 @@ export class DraggingHandle extends StateNode {
} }
} }
override onTick = () => {
if (this.isDirty) {
this.isDirty = false
this.update()
}
}
override onPointerMove: TLEventHandlers['onPointerMove'] = () => { override onPointerMove: TLEventHandlers['onPointerMove'] = () => {
this.isDirty = true this.update()
} }
override onKeyDown: TLKeyboardEvent | undefined = () => { override onKeyDown: TLKeyboardEvent | undefined = () => {
@ -192,7 +183,6 @@ export class DraggingHandle extends StateNode {
override onComplete: TLEventHandlers['onComplete'] = () => { override onComplete: TLEventHandlers['onComplete'] = () => {
this.update() this.update()
this.isDirty = false
this.complete() this.complete()
} }

View file

@ -75,15 +75,10 @@ export class Resizing extends StateNode {
override onTick = () => { override onTick = () => {
moveCameraWhenCloseToEdge(this.editor) moveCameraWhenCloseToEdge(this.editor)
if (!this.isDirty) return
this.isDirty = false
this.updateShapes()
} }
isDirty = false
override onPointerMove: TLEventHandlers['onPointerMove'] = () => { override onPointerMove: TLEventHandlers['onPointerMove'] = () => {
this.isDirty = true this.updateShapes()
} }
override onKeyDown: TLEventHandlers['onKeyDown'] = () => { override onKeyDown: TLEventHandlers['onKeyDown'] = () => {

View file

@ -22,7 +22,6 @@ export class Rotating extends StateNode {
info = {} as Extract<TLPointerEventInfo, { target: 'selection' }> & { onInteractionEnd?: string } info = {} as Extract<TLPointerEventInfo, { target: 'selection' }> & { onInteractionEnd?: string }
markId = '' markId = ''
isDirty = false
override onEnter = ( override onEnter = (
info: TLPointerEventInfo & { target: 'selection'; onInteractionEnd?: string } info: TLPointerEventInfo & { target: 'selection'; onInteractionEnd?: string }
@ -66,15 +65,8 @@ export class Rotating extends StateNode {
this.snapshot = {} as TLRotationSnapshot this.snapshot = {} as TLRotationSnapshot
} }
override onTick = () => {
if (this.isDirty) {
this.isDirty = false
this.update()
}
}
override onPointerMove = () => { override onPointerMove = () => {
this.isDirty = true this.update()
} }
override onKeyDown = () => { override onKeyDown = () => {

View file

@ -24,8 +24,6 @@ export class ScribbleBrushing extends StateNode {
initialSelectedShapeIds = new Set<TLShapeId>() initialSelectedShapeIds = new Set<TLShapeId>()
newlySelectedShapeIds = new Set<TLShapeId>() newlySelectedShapeIds = new Set<TLShapeId>()
isDirty = false
override onEnter = () => { override onEnter = () => {
this.initialSelectedShapeIds = new Set<TLShapeId>( this.initialSelectedShapeIds = new Set<TLShapeId>(
this.editor.inputs.shiftKey ? this.editor.getSelectedShapeIds() : [] this.editor.inputs.shiftKey ? this.editor.getSelectedShapeIds() : []
@ -33,7 +31,6 @@ export class ScribbleBrushing extends StateNode {
this.newlySelectedShapeIds = new Set<TLShapeId>() this.newlySelectedShapeIds = new Set<TLShapeId>()
this.size = 0 this.size = 0
this.hits.clear() this.hits.clear()
this.isDirty = false
const scribbleItem = this.editor.scribbles.addScribble({ const scribbleItem = this.editor.scribbles.addScribble({
color: 'selection-stroke', color: 'selection-stroke',
@ -54,15 +51,8 @@ export class ScribbleBrushing extends StateNode {
this.editor.scribbles.stop(this.scribbleId) this.editor.scribbles.stop(this.scribbleId)
} }
override onTick = () => {
if (this.isDirty) {
this.isDirty = false
this.updateScribbleSelection(true)
}
}
override onPointerMove = () => { override onPointerMove = () => {
this.isDirty = true this.updateScribbleSelection(true)
} }
override onPointerUp = () => { override onPointerUp = () => {
@ -168,7 +158,6 @@ export class ScribbleBrushing extends StateNode {
private complete() { private complete() {
this.updateScribbleSelection(true) this.updateScribbleSelection(true)
this.isDirty = false
this.parent.transition('idle') this.parent.transition('idle')
} }

View file

@ -35,7 +35,6 @@ export class Translating extends StateNode {
isCloning = false isCloning = false
isCreating = false isCreating = false
isDirty = false
onCreate: (shape: TLShape | null) => void = () => void null onCreate: (shape: TLShape | null) => void = () => void null
dragAndDropManager = new DragAndDropManager(this.editor) dragAndDropManager = new DragAndDropManager(this.editor)
@ -51,7 +50,6 @@ export class Translating extends StateNode {
const { isCreating = false, onCreate = () => void null } = info const { isCreating = false, onCreate = () => void null } = info
this.info = info this.info = info
this.isDirty = false
this.parent.setCurrentToolIdMask(info.onInteractionEnd) this.parent.setCurrentToolIdMask(info.onInteractionEnd)
this.isCreating = isCreating this.isCreating = isCreating
this.onCreate = onCreate this.onCreate = onCreate
@ -100,14 +98,10 @@ export class Translating extends StateNode {
this.updateParentTransforms this.updateParentTransforms
) )
moveCameraWhenCloseToEdge(this.editor) moveCameraWhenCloseToEdge(this.editor)
if (this.isDirty) {
this.isDirty = false
this.updateShapes()
}
} }
override onPointerMove = () => { override onPointerMove = () => {
this.isDirty = true this.updateShapes()
} }
override onKeyDown = () => { override onKeyDown = () => {
@ -172,7 +166,6 @@ export class Translating extends StateNode {
protected complete() { protected complete() {
this.updateShapes() this.updateShapes()
this.isDirty = false
this.dragAndDropManager.dropShapes(this.snapshot.movingShapes) this.dragAndDropManager.dropShapes(this.snapshot.movingShapes)
this.handleEnd() this.handleEnd()

View file

@ -311,18 +311,6 @@ export class TestEditor extends Editor {
/* ------------------ Input Events ------------------ */ /* ------------------ 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) => {
const tick = (this as any)._tickManager as { tick(): void }
for (let i = 0; i < count; i++) {
tick.tick()
}
return this
}
pointerMove = ( pointerMove = (
x = this.inputs.currentScreenPoint.x, x = this.inputs.currentScreenPoint.x,
y = this.inputs.currentScreenPoint.y, y = this.inputs.currentScreenPoint.y,
@ -332,7 +320,7 @@ export class TestEditor extends Editor {
this.dispatch({ this.dispatch({
...this.getPointerEventInfo(x, y, options, modifiers), ...this.getPointerEventInfo(x, y, options, modifiers),
name: 'pointer_move', name: 'pointer_move',
}).forceTick() })
return this return this
} }

View file

@ -211,7 +211,6 @@ for (const toolType of ['draw', 'highlight'] as const) {
expect(point1.x).toBe(1) expect(point1.x).toBe(1)
editor.keyDown('Meta') editor.keyDown('Meta')
editor.forceTick()
const shape2 = editor.getCurrentPageShapes()[0] as DrawableShape const shape2 = editor.getCurrentPageShapes()[0] as DrawableShape
const segment2 = last(shape2.props.segments)! const segment2 = last(shape2.props.segments)!
const point2 = last(segment2.points)! const point2 = last(segment2.points)!
@ -237,7 +236,6 @@ for (const toolType of ['draw', 'highlight'] as const) {
expect(point1.x).toBe(1) expect(point1.x).toBe(1)
editor.keyDown('Meta') editor.keyDown('Meta')
editor.forceTick()
const shape2 = editor.getCurrentPageShapes()[0] as DrawableShape const shape2 = editor.getCurrentPageShapes()[0] as DrawableShape
const segment2 = last(shape2.props.segments)! const segment2 = last(shape2.props.segments)!
const point2 = last(segment2.points)! const point2 = last(segment2.points)!

View file

@ -137,14 +137,14 @@ describe('When translating...', () => {
const before = editor.getShape<TLGeoShape>(ids.box1)! const before = editor.getShape<TLGeoShape>(ids.box1)!
editor.forceTick(5) jest.advanceTimersByTime(100)
editor editor
// The change is bigger than expected because the camera moves // The change is bigger than expected because the camera moves
.expectShapeToMatch({ id: ids.box1, x: -160, y: 10 }) .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. // 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. // The speed in the y position is smaller since we are further away from the edge.
.pointerMove(0, 25) .pointerMove(0, 25)
editor.forceTick(2) jest.advanceTimersByTime(100)
editor.pointerUp() editor.pointerUp()
const after = editor.getShape<TLGeoShape>(ids.box1)! const after = editor.getShape<TLGeoShape>(ids.box1)!
@ -159,16 +159,16 @@ describe('When translating...', () => {
editor.user.updateUserPreferences({ edgeScrollSpeed: 1 }) editor.user.updateUserPreferences({ edgeScrollSpeed: 1 })
editor.pointerDown(50, 50, ids.box1).pointerMove(1080, 50) editor.pointerDown(50, 50, ids.box1).pointerMove(1080, 50)
jest.advanceTimersByTime(100)
editor editor
.forceTick(4)
// The change is bigger than expected because the camera moves // The change is bigger than expected because the camera moves
.expectShapeToMatch({ id: ids.box1, x: 1140, y: 10 }) .expectShapeToMatch({ id: ids.box1, x: 1160, y: 10 })
.pointerMove(1080, 800) .pointerMove(1080, 800)
.forceTick(6) jest.advanceTimersByTime(100)
editor editor
.expectShapeToMatch({ id: ids.box1, x: 1280, y: 845.68 }) .expectShapeToMatch({ id: ids.box1, x: 1300, y: 845.68 })
.pointerUp() .pointerUp()
.expectShapeToMatch({ id: ids.box1, x: 1280, y: 845.68 }) .expectShapeToMatch({ id: ids.box1, x: 1300, y: 845.68 })
}) })
it('translates multiple shapes', () => { it('translates multiple shapes', () => {
@ -1897,68 +1897,11 @@ describe('Moving the camera while panning', () => {
.expectToBeIn('select.translating') .expectToBeIn('select.translating')
.expectShapeToMatch({ id: ids.box1, x: 10, y: 10 }) .expectShapeToMatch({ id: ids.box1, x: 10, y: 10 })
.wheel(-10, -10) // wheel by -10,-10 .wheel(-10, -10) // wheel by -10,-10
.forceTick() // needed
.expectShapeToMatch({ id: ids.box1, x: 20, y: 20 }) .expectShapeToMatch({ id: ids.box1, x: 20, y: 20 })
.wheel(-10, -10) // wheel by -10,-10 .wheel(-10, -10) // wheel by -10,-10
.forceTick() // needed
.expectShapeToMatch({ id: ids.box1, x: 30, y: 30 }) .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 () => { it('Correctly preserves screen point while dragging', async () => {
editor.createShape({ editor.createShape({
type: 'geo', type: 'geo',
@ -1990,8 +1933,6 @@ describe('Moving the camera while panning', () => {
// now we move the camera by -10,-10 // now we move the camera by -10,-10
// since we're dragging, they should still all move together // since we're dragging, they should still all move together
.wheel(-10, -10) .wheel(-10, -10)
.forceTick()
// wait for a tick to allow the tick manager to dispatch to the translating tool
// The camera has moved // The camera has moved
.expectCameraToBe(-10, -10, 1) .expectCameraToBe(-10, -10, 1)

View file

@ -7399,7 +7399,6 @@ __metadata:
version: 0.0.0-use.local version: 0.0.0-use.local
resolution: "@tldraw/state@workspace:packages/state" resolution: "@tldraw/state@workspace:packages/state"
dependencies: dependencies:
"@tldraw/utils": "workspace:*"
"@types/lodash": "npm:^4.14.188" "@types/lodash": "npm:^4.14.188"
"@types/react": "npm:^18.2.47" "@types/react": "npm:^18.2.47"
"@types/react-test-renderer": "npm:^18.0.0" "@types/react-test-renderer": "npm:^18.0.0"