From f390c711268e81f93e7aa08ae2d0a55f9f0825a5 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Fri, 3 May 2024 15:13:59 +0100 Subject: [PATCH 1/7] prevent wobble during viewport following --- packages/editor/api-report.md | 4 +- packages/editor/src/lib/constants.ts | 9 +- packages/editor/src/lib/editor/Editor.ts | 182 +++++++++++++---------- 3 files changed, 111 insertions(+), 84 deletions(-) diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index e4262d7ab66..c390679be09 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -1033,7 +1033,9 @@ export class Editor extends EventEmitter { }): this; readonly snaps: SnapManager; stackShapes(shapes: TLShape[] | TLShapeId[], operation: 'horizontal' | 'vertical', gap: number): this; - startFollowingUser(userId: string): this; + startFollowingUser(userId: string, { animateToUser }?: { + animateToUser?: boolean; + }): this; stopCameraAnimation(): this; stopFollowingUser(): this; readonly store: TLStore; diff --git a/packages/editor/src/lib/constants.ts b/packages/editor/src/lib/constants.ts index 7b68334dd80..cb393f96f48 100644 --- a/packages/editor/src/lib/constants.ts +++ b/packages/editor/src/lib/constants.ts @@ -20,15 +20,8 @@ export const DEFAULT_CAMERA_OPTIONS: TLCameraOptions = { zoomSteps: [0.1, 0.25, 0.5, 1, 2, 4, 8], } -export const FOLLOW_CHASE_PROPORTION = 0.5 /** @internal */ -export const FOLLOW_CHASE_PAN_SNAP = 0.1 -/** @internal */ -export const FOLLOW_CHASE_PAN_UNSNAP = 0.2 -/** @internal */ -export const FOLLOW_CHASE_ZOOM_SNAP = 0.005 -/** @internal */ -export const FOLLOW_CHASE_ZOOM_UNSNAP = 0.05 +export const FOLLOW_CHASE_VIEWPORT_SNAP = 2 /** @internal */ export const DOUBLE_CLICK_DURATION = 450 diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 2ff5e8925e6..8bb41daab4b 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -20,6 +20,7 @@ import { TLBinding, TLBindingId, TLBindingPartial, + TLCamera, TLCursor, TLCursorType, TLDOCUMENT_ID, @@ -88,11 +89,7 @@ import { DEFAULT_ANIMATION_OPTIONS, DEFAULT_CAMERA_OPTIONS, DRAG_DISTANCE, - FOLLOW_CHASE_PAN_SNAP, - FOLLOW_CHASE_PAN_UNSNAP, - FOLLOW_CHASE_PROPORTION, - FOLLOW_CHASE_ZOOM_SNAP, - FOLLOW_CHASE_ZOOM_UNSNAP, + FOLLOW_CHASE_VIEWPORT_SNAP, HIT_TEST_MARGIN, INTERNAL_POINTER_IDS, LEFT_MOUSE_BUTTON, @@ -2018,8 +2015,55 @@ export class Editor extends EventEmitter { * * @public */ - @computed getCamera() { - return this.store.get(this.getCameraId())! + @computed getCamera(): TLCamera { + const baseCamera = this.store.get(this.getCameraId())! + if (this._isLockedOnFollowingUser.get()) { + const followingCamera = this.getCameraForFollowing() + if (followingCamera) { + return { ...baseCamera, ...followingCamera } + } + } + return baseCamera + } + + @computed + private getViewportPageBoundsForFollowing(): null | Box { + const followingUserId = this.getInstanceState().followingUserId + if (!followingUserId) return null + const leaderPresence = this.getCollaborators().find((c) => c.userId === followingUserId) + if (!leaderPresence) return null + + // Fit their viewport inside of our screen bounds + // 1. calculate their viewport in page space + const { w: lw, h: lh } = leaderPresence.screenBounds + const { x: lx, y: ly, z: lz } = leaderPresence.camera + const theirViewport = new Box(-lx, -ly, lw / lz, lh / lz) + + // resize our screenBounds to contain their viewport + const ourViewport = this.getViewportScreenBounds().clone() + const ourAspectRatio = ourViewport.width / ourViewport.height + + ourViewport.width = theirViewport.width + ourViewport.height = ourViewport.width / ourAspectRatio + if (ourViewport.height < theirViewport.height) { + ourViewport.height = theirViewport.height + ourViewport.width = ourViewport.height * ourAspectRatio + } + + ourViewport.center = theirViewport.center + return ourViewport + } + + @computed + private getCameraForFollowing(): null | { x: number; y: number; z: number } { + const viewport = this.getViewportPageBoundsForFollowing() + if (!viewport) return null + + return { + x: -viewport.x, + y: -viewport.y, + z: this.getViewportScreenBounds().w / viewport.width, + } } /** @@ -2836,7 +2880,7 @@ export class Editor extends EventEmitter { * editor.zoomToUser(myUserId, { animation: { duration: 200 } }) * ``` * - * @param userId - The id of the user to aniamte to. + * @param userId - The id of the user to animate to. * @param opts - The camera move options. * @public */ @@ -3082,6 +3126,8 @@ export class Editor extends EventEmitter { // Following + private _isLockedOnFollowingUser = atom('isLockedOnFollowingUser', false) + /** * Start viewport-following a user. * @@ -3091,10 +3137,14 @@ export class Editor extends EventEmitter { * ``` * * @param userId - The id of the user to follow. + * @param opts - Options for starting to follow a user. * * @public */ - startFollowingUser(userId: string): this { + startFollowingUser( + userId: string, + { animateToUser = true }: { animateToUser?: boolean } = {} + ): this { const leaderPresences = this._getCollaboratorsQuery() .get() .filter((p) => p.userId === userId) @@ -3116,12 +3166,11 @@ export class Editor extends EventEmitter { }) const cancel = () => { + this._isLockedOnFollowingUser.set(false) this.off('frame', moveTowardsUser) this.off('stop-following', cancel) } - let isCaughtUp = false - const moveTowardsUser = () => { transact(() => { // Stop following if we can't find the user @@ -3132,92 +3181,69 @@ export class Editor extends EventEmitter { return b.lastActivityTimestamp - a.lastActivityTimestamp })[0] - if (!leaderPresence) { - this.stopFollowingUser() - return - } - // Change page if leader is on a different page const isOnSamePage = leaderPresence.currentPageId === this.getCurrentPageId() - const chaseProportion = isOnSamePage ? FOLLOW_CHASE_PROPORTION : 1 if (!isOnSamePage) { this.stopFollowingUser() this.setCurrentPage(leaderPresence.currentPageId) - this.startFollowingUser(userId) + this.startFollowingUser(userId, { animateToUser: false }) return } - // Get the bounds of the follower (me) and the leader (them) - const { center, width, height } = this.getViewportPageBounds() - const leaderScreen = Box.From(leaderPresence.screenBounds) - const leaderWidth = leaderScreen.width / leaderPresence.camera.z - const leaderHeight = leaderScreen.height / leaderPresence.camera.z - const leaderCenter = new Vec( - leaderWidth / 2 - leaderPresence.camera.x, - leaderHeight / 2 - leaderPresence.camera.y - ) - - // At this point, let's check if we're following someone who's following us. - // If so, we can't try to contain their entire viewport - // because that would become a feedback loop where we zoom, they zoom, etc. - const isFollowingFollower = leaderPresence.followingUserId === thisUserId + if (this._isLockedOnFollowingUser.get()) return - // Figure out how much to zoom - const desiredWidth = width + (leaderWidth - width) * chaseProportion - const desiredHeight = height + (leaderHeight - height) * chaseProportion - const ratio = !isFollowingFollower - ? Math.min(width / desiredWidth, height / desiredHeight) - : height / desiredHeight + const animationSpeed = this.user.getAnimationSpeed() - const baseZoom = this.getBaseZoom() - const { zoomSteps } = this.getCameraOptions() - const zoomMin = zoomSteps[0] - const zoomMax = last(zoomSteps)! - const targetZoom = clamp(this.getCamera().z * ratio, zoomMin * baseZoom, zoomMax * baseZoom) - const targetWidth = this.getViewportScreenBounds().w / targetZoom - const targetHeight = this.getViewportScreenBounds().h / targetZoom - - // Figure out where to move the camera - const displacement = leaderCenter.sub(center) - const targetCenter = Vec.Add(center, Vec.Mul(displacement, chaseProportion)) - - // Now let's assess whether we've caught up to the leader or not - const distance = Vec.Sub(targetCenter, center).len() - const zoomChange = Math.abs(targetZoom - this.getCamera().z) - - // If we're chasing the leader... - // Stop chasing if we're close enough - if (distance < FOLLOW_CHASE_PAN_SNAP && zoomChange < FOLLOW_CHASE_ZOOM_SNAP) { - isCaughtUp = true + if (!animateToUser || animationSpeed === 0) { + this._isLockedOnFollowingUser.set(true) return } - // If we're already caught up with the leader... - // Only start moving again if we're far enough away - if ( - isCaughtUp && - distance < FOLLOW_CHASE_PAN_UNSNAP && - zoomChange < FOLLOW_CHASE_ZOOM_UNSNAP - ) { + const targetViewport = this.getViewportPageBoundsForFollowing() + if (!targetViewport) { + this.stopFollowingUser() + return + } + const currentViewport = this.getViewportPageBounds() + + const diffX = + Math.abs(targetViewport.minX - currentViewport.minX) + + Math.abs(targetViewport.maxX - currentViewport.maxX) + const diffY = + Math.abs(targetViewport.minY - currentViewport.minY) + + Math.abs(targetViewport.maxY - currentViewport.maxY) + + // Stop chasing if we're close enough! + if (diffX < FOLLOW_CHASE_VIEWPORT_SNAP && diffY < FOLLOW_CHASE_VIEWPORT_SNAP) { + this._isLockedOnFollowingUser.set(true) return } + const midpointViewport = new Box( + (currentViewport.minX + targetViewport.minX) / 2, + (currentViewport.minY + targetViewport.minY) / 2, + (currentViewport.width + targetViewport.width) / 2, + (currentViewport.height + targetViewport.height) / 2 + ) + + const midpointCamera = new Vec( + -midpointViewport.x, + -midpointViewport.y, + this.getViewportScreenBounds().width / midpointViewport.width + ) + // Update the camera! - isCaughtUp = false this.stopCameraAnimation() - this._setCamera( - new Vec( - -(targetCenter.x - targetWidth / 2), - -(targetCenter.y - targetHeight / 2), - targetZoom - ) - ) + this._setCamera(midpointCamera) }) } this.once('stop-following', cancel) this.on('frame', moveTowardsUser) + // call once to start synchronously + moveTowardsUser() + return this } @@ -3231,8 +3257,14 @@ export class Editor extends EventEmitter { * @public */ stopFollowingUser(): this { - this.updateInstanceState({ followingUserId: null }) - this.emit('stop-following') + this.batch(() => { + // commit the current camera to the store + this._setCamera(this.getCamera()) + // this must happen after the camera is committed + this._isLockedOnFollowingUser.set(false) + this.updateInstanceState({ followingUserId: null }) + this.emit('stop-following') + }) return this } From 58e2a3f732a24431d33ed4d51478f0ebfe72cc72 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Fri, 3 May 2024 16:02:10 +0100 Subject: [PATCH 2/7] handle page creation/deletion --- packages/editor/src/lib/editor/Editor.ts | 63 ++++++++++++++---------- packages/tlsync/src/lib/TLSyncClient.ts | 4 ++ 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index 8bb41daab4b..d4649ae2551 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -1,4 +1,4 @@ -import { EMPTY_ARRAY, atom, computed, transact } from '@tldraw/state' +import { EMPTY_ARRAY, atom, computed, react, transact } from '@tldraw/state' import { ComputedCache, RecordType, @@ -3160,33 +3160,41 @@ export class Editor extends EventEmitter { return this } + const latestLeaderPresence = computed('latestLeaderPresence', () => { + return this.getCollaborators().find((p) => p.userId === userId) + }) + transact(() => { this.stopFollowingUser() this.updateInstanceState({ followingUserId: userId }) - }) - const cancel = () => { - this._isLockedOnFollowingUser.set(false) - this.off('frame', moveTowardsUser) - this.off('stop-following', cancel) - } + const stopUpdatingPage = react('update current page', () => { + const leaderPresence = latestLeaderPresence.get() + if (!leaderPresence) return + if ( + leaderPresence.currentPageId !== this.getCurrentPageId() && + this.getPage(leaderPresence.currentPageId) + ) { + this.history.ignore(() => { + this.store.put([ + { ...this.getInstanceState(), currentPageId: leaderPresence.currentPageId }, + ]) + }) + } + }) + + const cancel = () => { + stopUpdatingPage() + this._isLockedOnFollowingUser.set(false) + this.removeListener('frame', moveTowardsUser) + this.removeListener('stop-following', cancel) + } - const moveTowardsUser = () => { - transact(() => { + const moveTowardsUser = () => { // Stop following if we can't find the user - const leaderPresence = this._getCollaboratorsQuery() - .get() - .filter((p) => p.userId === userId) - .sort((a, b) => { - return b.lastActivityTimestamp - a.lastActivityTimestamp - })[0] - - // Change page if leader is on a different page - const isOnSamePage = leaderPresence.currentPageId === this.getCurrentPageId() - if (!isOnSamePage) { + const leaderPresence = latestLeaderPresence.get() + if (!leaderPresence) { this.stopFollowingUser() - this.setCurrentPage(leaderPresence.currentPageId) - this.startFollowingUser(userId, { animateToUser: false }) return } @@ -3235,14 +3243,14 @@ export class Editor extends EventEmitter { // Update the camera! this.stopCameraAnimation() this._setCamera(midpointCamera) - }) - } + } - this.once('stop-following', cancel) - this.on('frame', moveTowardsUser) + this.once('stop-following', cancel) + this.addListener('frame', moveTowardsUser) - // call once to start synchronously - moveTowardsUser() + // call once to start synchronously + moveTowardsUser() + }) return this } @@ -3257,6 +3265,7 @@ export class Editor extends EventEmitter { * @public */ stopFollowingUser(): this { + console.error('why did i stop') this.batch(() => { // commit the current camera to the store this._setCamera(this.getCamera()) diff --git a/packages/tlsync/src/lib/TLSyncClient.ts b/packages/tlsync/src/lib/TLSyncClient.ts index fe63d687648..df7568cbd8f 100644 --- a/packages/tlsync/src/lib/TLSyncClient.ts +++ b/packages/tlsync/src/lib/TLSyncClient.ts @@ -423,6 +423,10 @@ export class TLSyncClient = Store lastPushedPresenceState: R | null = null private pushPresence(nextPresence: R | null) { + // make sure we push any document changes first + // TODO: need to send presence changes in the same push request as document changes + // in order to not get into weird states + this.store._flushHistory() if (!this.isConnectedToRoom) { // if we're offline, don't do anything return From ca7641927d058692caa915d69eb9b0f644c7cb76 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Tue, 7 May 2024 13:56:39 +0100 Subject: [PATCH 3/7] synchronize presence and document updates --- packages/editor/src/lib/editor/Editor.ts | 1 - packages/tlsync/src/lib/TLSyncClient.ts | 37 +++++++++++++++--------- packages/tlsync/src/lib/TLSyncRoom.ts | 32 +++++++++----------- packages/tlsync/src/lib/protocol.ts | 17 ++++------- 4 files changed, 44 insertions(+), 43 deletions(-) diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index d4649ae2551..c0dd8cf21a6 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -3265,7 +3265,6 @@ export class Editor extends EventEmitter { * @public */ stopFollowingUser(): this { - console.error('why did i stop') this.batch(() => { // commit the current camera to the store this._setCamera(this.getCamera()) diff --git a/packages/tlsync/src/lib/TLSyncClient.ts b/packages/tlsync/src/lib/TLSyncClient.ts index df7568cbd8f..f946404f774 100644 --- a/packages/tlsync/src/lib/TLSyncClient.ts +++ b/packages/tlsync/src/lib/TLSyncClient.ts @@ -431,26 +431,37 @@ export class TLSyncClient = Store // if we're offline, don't do anything return } - let req: TLPushRequest | null = null + + let presence: TLPushRequest['presence'] = undefined if (!this.lastPushedPresenceState && nextPresence) { // we don't have a last presence state, so we need to push the full state - req = { - type: 'push', - presence: [RecordOpType.Put, nextPresence], - clientClock: this.clientClock++, - } + presence = [RecordOpType.Put, nextPresence] } else if (this.lastPushedPresenceState && nextPresence) { // we have a last presence state, so we need to push a diff if there is one const diff = diffRecord(this.lastPushedPresenceState, nextPresence) if (diff) { - req = { - type: 'push', - presence: [RecordOpType.Patch, diff], - clientClock: this.clientClock++, - } + presence = [RecordOpType.Patch, diff] } } + + if (!presence) return this.lastPushedPresenceState = nextPresence + + // if there is a pending push that has not been sent and does not already include a presence update, + // then add this presence update to it + const lastPush = this.pendingPushRequests.at(-1) + if (lastPush && !lastPush.sent && !lastPush.request.presence) { + lastPush.request.presence = presence + return + } + + // otherwise, create a new push request + const req: TLPushRequest = { + type: 'push', + clientClock: this.clientClock++, + presence, + } + if (req) { this.pendingPushRequests.push({ request: req, sent: false }) this.flushPendingPushRequests() @@ -590,7 +601,7 @@ export class TLSyncClient = Store this.pendingPushRequests.shift() } else if (diff.action === 'commit') { const { request } = this.pendingPushRequests.shift()! - if ('diff' in request) { + if ('diff' in request && request.diff) { this.applyNetworkDiff(request.diff, true) } } else { @@ -602,7 +613,7 @@ export class TLSyncClient = Store try { this.speculativeChanges = this.store.extractingChanges(() => { for (const { request } of this.pendingPushRequests) { - if (!('diff' in request)) continue + if (!('diff' in request) || !request.diff) continue this.applyNetworkDiff(request.diff, true) } }) diff --git a/packages/tlsync/src/lib/TLSyncRoom.ts b/packages/tlsync/src/lib/TLSyncRoom.ts index 1c9cc56c0e4..1d46f195a33 100644 --- a/packages/tlsync/src/lib/TLSyncRoom.ts +++ b/packages/tlsync/src/lib/TLSyncRoom.ts @@ -921,7 +921,7 @@ export class TLSyncRoom { const { clientClock } = message - if ('presence' in message) { + if ('presence' in message && message.presence) { // The push request was for the presence scope. const id = session.presenceId const [type, val] = message.presence @@ -944,15 +944,11 @@ export class TLSyncRoom { break } } - this.sendMessage(session.sessionKey, { - type: 'push_result', - clientClock, - action: 'commit', - serverClock: this.clock, - }) - } else { + } + const hasDiff = 'diff' in message && message.diff + if (hasDiff) { // The push request was for the document scope. - for (const [id, op] of Object.entries(message.diff)) { + for (const [id, op] of Object.entries(message.diff!)) { switch (op[0]) { case RecordOpType.Put: { // Try to add the document. @@ -993,24 +989,24 @@ export class TLSyncRoom { } // Let the client know what action to take based on the results of the push - if (!mergedChanges) { - // DISCARD - // Applying the client's changes had no effect, so the client should drop the diff + if (!hasDiff || isEqual(mergedChanges, message.diff)) { + // COMMIT + // Applying the client's changes had the exact same effect on the server as + // they had on the client, so the client should keep the diff this.sendMessage(session.sessionKey, { type: 'push_result', serverClock: this.clock, clientClock, - action: 'discard', + action: 'commit', }) - } else if (isEqual(mergedChanges, message.diff)) { - // COMMIT - // Applying the client's changes had the exact same effect on the server as - // they had on the client, so the client should keep the diff + } else if (!mergedChanges) { + // DISCARD + // Applying the client's changes had no effect, so the client should drop the diff this.sendMessage(session.sessionKey, { type: 'push_result', serverClock: this.clock, clientClock, - action: 'commit', + action: 'discard', }) } else { // REBASE diff --git a/packages/tlsync/src/lib/protocol.ts b/packages/tlsync/src/lib/protocol.ts index 593083a03fd..171b8b0d981 100644 --- a/packages/tlsync/src/lib/protocol.ts +++ b/packages/tlsync/src/lib/protocol.ts @@ -61,17 +61,12 @@ export type TLSocketServerSentDataEvent = } /** @public */ -export type TLPushRequest = - | { - type: 'push' - clientClock: number - presence: [typeof RecordOpType.Patch, ObjectDiff] | [typeof RecordOpType.Put, R] - } - | { - type: 'push' - clientClock: number - diff: NetworkDiff - } +export type TLPushRequest = { + type: 'push' + clientClock: number + diff?: NetworkDiff + presence?: [typeof RecordOpType.Patch, ObjectDiff] | [typeof RecordOpType.Put, R] +} /** @public */ export type TLConnectRequest = { From 070e4b4bdea59e679bc82688758f46d00870aa30 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Wed, 8 May 2024 11:38:02 +0100 Subject: [PATCH 4/7] make sure handlePushRequest responds correctly --- packages/tlsync/src/lib/TLSyncRoom.ts | 135 +++++++++++++++----------- 1 file changed, 76 insertions(+), 59 deletions(-) diff --git a/packages/tlsync/src/lib/TLSyncRoom.ts b/packages/tlsync/src/lib/TLSyncRoom.ts index 1d46f195a33..1669399db70 100644 --- a/packages/tlsync/src/lib/TLSyncRoom.ts +++ b/packages/tlsync/src/lib/TLSyncRoom.ts @@ -814,11 +814,13 @@ export class TLSyncRoom { transaction((rollback) => { // collect actual ops that resulted from the push // these will be broadcast to other users - let mergedChanges: NetworkDiff | null = null + type ActualChanges = { diff: NetworkDiff | null } + const docChanges: ActualChanges = { diff: null } + const presenceChanges: ActualChanges = { diff: null } - const propagateOp = (id: string, op: RecordOp) => { - if (!mergedChanges) mergedChanges = {} - mergedChanges[id] = op + const propagateOp = (changes: ActualChanges, id: string, op: RecordOp) => { + if (!changes.diff) changes.diff = {} + changes.diff[id] = op } const fail = (reason: TLIncompatibilityReason): Result => { @@ -830,7 +832,7 @@ export class TLSyncRoom { return Result.err(undefined) } - const addDocument = (id: string, _state: R): Result => { + const addDocument = (changes: ActualChanges, id: string, _state: R): Result => { const res = this.schema.migratePersistedRecord(_state, session.serializedSchema, 'up') if (res.type === 'error') { return fail( @@ -852,7 +854,7 @@ export class TLSyncRoom { return fail(TLIncompatibilityReason.InvalidRecord) } if (diff.value) { - propagateOp(id, [RecordOpType.Patch, diff.value]) + propagateOp(changes, id, [RecordOpType.Patch, diff.value]) } } else { // Otherwise, if we don't already have a document with this id @@ -861,13 +863,17 @@ export class TLSyncRoom { if (!result.ok) { return fail(TLIncompatibilityReason.InvalidRecord) } - propagateOp(id, [RecordOpType.Put, state]) + propagateOp(changes, id, [RecordOpType.Put, state]) } return Result.ok(undefined) } - const patchDocument = (id: string, patch: ObjectDiff): Result => { + const patchDocument = ( + changes: ActualChanges, + id: string, + patch: ObjectDiff + ): Result => { // if it was already deleted, there's no need to apply the patch const doc = this.getDocument(id) if (!doc) return Result.ok(undefined) @@ -889,7 +895,7 @@ export class TLSyncRoom { return fail(TLIncompatibilityReason.InvalidRecord) } if (diff.value) { - propagateOp(id, [RecordOpType.Patch, diff.value]) + propagateOp(changes, id, [RecordOpType.Patch, diff.value]) } } else { // need to apply the patch to the downgraded version and then upgrade it @@ -912,7 +918,7 @@ export class TLSyncRoom { return fail(TLIncompatibilityReason.InvalidRecord) } if (diff.value) { - propagateOp(id, [RecordOpType.Patch, diff.value]) + propagateOp(changes, id, [RecordOpType.Patch, diff.value]) } } @@ -929,24 +935,25 @@ export class TLSyncRoom { switch (type) { case RecordOpType.Put: { // Try to put the document. If it fails, stop here. - const res = addDocument(id, { ...val, id, typeName }) + const res = addDocument(presenceChanges, id, { ...val, id, typeName }) + // if res.ok is false here then we already called `fail` and we should stop immediately if (!res.ok) return break } case RecordOpType.Patch: { // Try to patch the document. If it fails, stop here. - const res = patchDocument(id, { + const res = patchDocument(presenceChanges, id, { ...val, id: [ValueOpType.Put, id], typeName: [ValueOpType.Put, typeName], }) + // if res.ok is false here then we already called `fail` and we should stop immediately if (!res.ok) return break } } } - const hasDiff = 'diff' in message && message.diff - if (hasDiff) { + if (message.diff) { // The push request was for the document scope. for (const [id, op] of Object.entries(message.diff!)) { switch (op[0]) { @@ -956,13 +963,15 @@ export class TLSyncRoom { if (!this.documentTypes.has(op[1].typeName)) { return fail(TLIncompatibilityReason.InvalidRecord) } - const res = addDocument(id, op[1]) + const res = addDocument(docChanges, id, op[1]) + // if res.ok is false here then we already called `fail` and we should stop immediately if (!res.ok) return break } case RecordOpType.Patch: { // Try to patch the document. If it fails, stop here. - const res = patchDocument(id, op[1]) + const res = patchDocument(docChanges, id, op[1]) + // if res.ok is false here then we already called `fail` and we should stop immediately if (!res.ok) return break } @@ -982,60 +991,68 @@ export class TLSyncRoom { this.removeDocument(id, this.clock) // Schedule a pruneTombstones call to happen on the next call stack setTimeout(this.pruneTombstones, 0) - propagateOp(id, op) + propagateOp(docChanges, id, op) break } } } + } - // Let the client know what action to take based on the results of the push - if (!hasDiff || isEqual(mergedChanges, message.diff)) { - // COMMIT - // Applying the client's changes had the exact same effect on the server as - // they had on the client, so the client should keep the diff - this.sendMessage(session.sessionKey, { - type: 'push_result', - serverClock: this.clock, - clientClock, - action: 'commit', - }) - } else if (!mergedChanges) { - // DISCARD - // Applying the client's changes had no effect, so the client should drop the diff - this.sendMessage(session.sessionKey, { - type: 'push_result', - serverClock: this.clock, - clientClock, - action: 'discard', - }) - } else { - // REBASE - // Applying the client's changes had a different non-empty effect on the server, - // so the client should rebase with our gold-standard / authoritative diff. - // First we need to migrate the diff to the client's version - const migrateResult = this.migrateDiffForSession(session.serializedSchema, mergedChanges) - if (!migrateResult.ok) { - return fail( - migrateResult.error === MigrationFailureReason.TargetVersionTooNew - ? TLIncompatibilityReason.ServerTooOld - : TLIncompatibilityReason.ClientTooOld - ) - } - // If the migration worked, send the rebased diff to the client - this.sendMessage(session.sessionKey, { - type: 'push_result', - serverClock: this.clock, - clientClock, - action: { rebaseWithDiff: migrateResult.value }, - }) + // Let the client know what action to take based on the results of the push + if ( + // if there was only a presence push, the client doesn't need to do anything aside from + // shift the push request. + !message.diff || + isEqual(docChanges.diff, message.diff) + ) { + // COMMIT + // Applying the client's changes had the exact same effect on the server as + // they had on the client, so the client should keep the diff + this.sendMessage(session.sessionKey, { + type: 'push_result', + serverClock: this.clock, + clientClock, + action: 'commit', + }) + } else if (!docChanges.diff) { + // DISCARD + // Applying the client's changes had no effect, so the client should drop the diff + this.sendMessage(session.sessionKey, { + type: 'push_result', + serverClock: this.clock, + clientClock, + action: 'discard', + }) + } else { + // REBASE + // Applying the client's changes had a different non-empty effect on the server, + // so the client should rebase with our gold-standard / authoritative diff. + // First we need to migrate the diff to the client's version + const migrateResult = this.migrateDiffForSession(session.serializedSchema, docChanges.diff) + if (!migrateResult.ok) { + return fail( + migrateResult.error === MigrationFailureReason.TargetVersionTooNew + ? TLIncompatibilityReason.ServerTooOld + : TLIncompatibilityReason.ClientTooOld + ) } + // If the migration worked, send the rebased diff to the client + this.sendMessage(session.sessionKey, { + type: 'push_result', + serverClock: this.clock, + clientClock, + action: { rebaseWithDiff: migrateResult.value }, + }) } // If there are merged changes, broadcast them to all other clients - if (mergedChanges !== null) { + if (docChanges.diff || presenceChanges.diff) { this.broadcastPatch({ sourceSessionKey: session.sessionKey, - diff: mergedChanges, + diff: { + ...docChanges.diff, + ...presenceChanges.diff, + }, }) } From d5dcc9b9ac6b4f4d2924033e13ceee7dc4f23805 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Wed, 8 May 2024 11:47:13 +0100 Subject: [PATCH 5/7] remove unused option --- packages/editor/api-report.md | 4 +--- packages/editor/src/lib/editor/Editor.ts | 7 ++----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index c390679be09..e4262d7ab66 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -1033,9 +1033,7 @@ export class Editor extends EventEmitter { }): this; readonly snaps: SnapManager; stackShapes(shapes: TLShape[] | TLShapeId[], operation: 'horizontal' | 'vertical', gap: number): this; - startFollowingUser(userId: string, { animateToUser }?: { - animateToUser?: boolean; - }): this; + startFollowingUser(userId: string): this; stopCameraAnimation(): this; stopFollowingUser(): this; readonly store: TLStore; diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index c0dd8cf21a6..fd5bd1e0452 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -3141,10 +3141,7 @@ export class Editor extends EventEmitter { * * @public */ - startFollowingUser( - userId: string, - { animateToUser = true }: { animateToUser?: boolean } = {} - ): this { + startFollowingUser(userId: string): this { const leaderPresences = this._getCollaboratorsQuery() .get() .filter((p) => p.userId === userId) @@ -3202,7 +3199,7 @@ export class Editor extends EventEmitter { const animationSpeed = this.user.getAnimationSpeed() - if (!animateToUser || animationSpeed === 0) { + if (animationSpeed === 0) { this._isLockedOnFollowingUser.set(true) return } From 2a01720c8de6a650ec671a692ace233225a7dbe1 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Wed, 8 May 2024 12:25:02 +0100 Subject: [PATCH 6/7] fix stopFollowingUser --- packages/editor/src/lib/editor/Editor.ts | 36 ++++++++++++++++-------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index fd5bd1e0452..f55a2f66f91 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -1,4 +1,4 @@ -import { EMPTY_ARRAY, atom, computed, react, transact } from '@tldraw/state' +import { EMPTY_ARRAY, atom, computed, react, transact, unsafe__withoutCapture } from '@tldraw/state' import { ComputedCache, RecordType, @@ -3142,10 +3142,18 @@ export class Editor extends EventEmitter { * @public */ startFollowingUser(userId: string): this { + // if we were already following someone, stop following them + this.stopFollowingUser() + const leaderPresences = this._getCollaboratorsQuery() .get() .filter((p) => p.userId === userId) + if (!leaderPresences.length) { + console.warn('User not found') + return this + } + const thisUserId = this.user.getId() if (!thisUserId) { @@ -3162,17 +3170,20 @@ export class Editor extends EventEmitter { }) transact(() => { - this.stopFollowingUser() this.updateInstanceState({ followingUserId: userId }) const stopUpdatingPage = react('update current page', () => { const leaderPresence = latestLeaderPresence.get() - if (!leaderPresence) return + if (!leaderPresence) { + this.stopFollowingUser() + return + } if ( leaderPresence.currentPageId !== this.getCurrentPageId() && this.getPage(leaderPresence.currentPageId) ) { this.history.ignore(() => { + // sneaky store.put here, we can't go through setCurrentPage because it calls stopFollowingUser this.store.put([ { ...this.getInstanceState(), currentPageId: leaderPresence.currentPageId }, ]) @@ -3183,8 +3194,8 @@ export class Editor extends EventEmitter { const cancel = () => { stopUpdatingPage() this._isLockedOnFollowingUser.set(false) - this.removeListener('frame', moveTowardsUser) - this.removeListener('stop-following', cancel) + this.off('frame', moveTowardsUser) + this.off('stop-following', cancel) } const moveTowardsUser = () => { @@ -3264,7 +3275,7 @@ export class Editor extends EventEmitter { stopFollowingUser(): this { this.batch(() => { // commit the current camera to the store - this._setCamera(this.getCamera()) + this.store.put([this.getCamera()]) // this must happen after the camera is committed this._isLockedOnFollowingUser.set(false) this.updateInstanceState({ followingUserId: null }) @@ -8304,7 +8315,6 @@ export class Editor extends EventEmitter { const instanceState = this.store.unsafeGetWithoutCapture(TLINSTANCE_ID)! const pageState = this.store.get(this._getCurrentPageStateId())! const cameraOptions = this._cameraOptions.__unsafe__getWithoutCapture()! - const camera = this.store.unsafeGetWithoutCapture(this.getCameraId())! switch (type) { case 'pinch': { @@ -8346,13 +8356,13 @@ export class Editor extends EventEmitter { instanceState.screenBounds.y ) - const { x: cx, y: cy, z: cz } = camera - this.stopCameraAnimation() if (instanceState.followingUserId) { this.stopFollowingUser() } + const { x: cx, y: cy, z: cz } = unsafe__withoutCapture(() => this.getCamera()) + const { panSpeed, zoomSpeed } = cameraOptions this._setCamera( new Vec( @@ -8411,7 +8421,7 @@ export class Editor extends EventEmitter { this.stopFollowingUser() } - const { x: cx, y: cy, z: cz } = camera + const { x: cx, y: cy, z: cz } = unsafe__withoutCapture(() => this.getCamera()) const { x: dx, y: dy, z: dz = 0 } = info.delta let behavior = wheelBehavior @@ -8526,10 +8536,12 @@ export class Editor extends EventEmitter { // If the user is in pen mode, but the pointer is not a pen, stop here. if (!isPen && isPenMode) return + const { x: cx, y: cy, z: cz } = unsafe__withoutCapture(() => this.getCamera()) + + // If we've started panning, then clear any long press timeout if (this.inputs.isPanning && this.inputs.isPointing) { // Handle spacebar / middle mouse button panning const { currentScreenPoint, previousScreenPoint } = this.inputs - const { x: cx, y: cy, z: cz } = camera const { panSpeed } = cameraOptions const offset = Vec.Sub(currentScreenPoint, previousScreenPoint) this.setCamera( @@ -8544,7 +8556,7 @@ export class Editor extends EventEmitter { inputs.isPointing && !inputs.isDragging && Vec.Dist2(originPagePoint, currentPagePoint) > - (instanceState.isCoarsePointer ? COARSE_DRAG_DISTANCE : DRAG_DISTANCE) / camera.z + (instanceState.isCoarsePointer ? COARSE_DRAG_DISTANCE : DRAG_DISTANCE) / cz ) { // Start dragging inputs.isDragging = true From bfc399109109d5a565d50399a636f781ac2c6753 Mon Sep 17 00:00:00 2001 From: David Sheldrick Date: Wed, 8 May 2024 14:05:25 +0100 Subject: [PATCH 7/7] lerp viewports, make page switches instant always --- packages/editor/src/lib/editor/Editor.ts | 35 ++++++++++++++++-------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index f55a2f66f91..4d5b0c83167 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -70,6 +70,7 @@ import { getOwnProperty, hasOwnProperty, last, + lerp, sortById, sortByIndex, structuredClone, @@ -3126,6 +3127,7 @@ export class Editor extends EventEmitter { // Following + // When we are 'locked on' to a user, our camera is derived from their camera. private _isLockedOnFollowingUser = atom('isLockedOnFollowingUser', false) /** @@ -3158,6 +3160,7 @@ export class Editor extends EventEmitter { if (!thisUserId) { console.warn('You should set the userId for the current instance before following a user') + // allow to continue since it's probably fine most of the time. } // If the leader is following us, then we can't follow them @@ -3172,7 +3175,8 @@ export class Editor extends EventEmitter { transact(() => { this.updateInstanceState({ followingUserId: userId }) - const stopUpdatingPage = react('update current page', () => { + // we listen for page changes separately from the 'moveTowardsUser' tick + const dispose = react('update current page', () => { const leaderPresence = latestLeaderPresence.get() if (!leaderPresence) { this.stopFollowingUser() @@ -3182,17 +3186,19 @@ export class Editor extends EventEmitter { leaderPresence.currentPageId !== this.getCurrentPageId() && this.getPage(leaderPresence.currentPageId) ) { + // if the page changed, switch page this.history.ignore(() => { // sneaky store.put here, we can't go through setCurrentPage because it calls stopFollowingUser this.store.put([ { ...this.getInstanceState(), currentPageId: leaderPresence.currentPageId }, ]) + this._isLockedOnFollowingUser.set(true) }) } }) const cancel = () => { - stopUpdatingPage() + dispose() this._isLockedOnFollowingUser.set(false) this.off('frame', moveTowardsUser) this.off('stop-following', cancel) @@ -3235,22 +3241,27 @@ export class Editor extends EventEmitter { return } - const midpointViewport = new Box( - (currentViewport.minX + targetViewport.minX) / 2, - (currentViewport.minY + targetViewport.minY) / 2, - (currentViewport.width + targetViewport.width) / 2, - (currentViewport.height + targetViewport.height) / 2 + // Chase the user's viewport! + // Interpolate between the current viewport and the target viewport based on animation speed. + // This will produce an 'ease-out' effect. + const t = clamp(animationSpeed * 0.5, 0.1, 0.8) + + const nextViewport = new Box( + lerp(currentViewport.minX, targetViewport.minX, t), + lerp(currentViewport.minY, targetViewport.minY, t), + lerp(currentViewport.width, targetViewport.width, t), + lerp(currentViewport.height, targetViewport.height, t) ) - const midpointCamera = new Vec( - -midpointViewport.x, - -midpointViewport.y, - this.getViewportScreenBounds().width / midpointViewport.width + const nextCamera = new Vec( + -nextViewport.x, + -nextViewport.y, + this.getViewportScreenBounds().width / nextViewport.width ) // Update the camera! this.stopCameraAnimation() - this._setCamera(midpointCamera) + this._setCamera(nextCamera) } this.once('stop-following', cancel)