From 6dfa89e846df4b51319ae8d91b7382c003b3c293 Mon Sep 17 00:00:00 2001 From: David Luzar <5153846+dwelle@users.noreply.github.com> Date: Sat, 16 Dec 2023 17:32:54 +0100 Subject: [PATCH] fix: emitted visible scene bounds not accounting for offsets (#7450) --- excalidraw-app/app_constants.ts | 5 +- excalidraw-app/collab/Collab.tsx | 52 +++++++++----------- excalidraw-app/collab/Portal.tsx | 14 +++--- excalidraw-app/data/index.ts | 24 ++++----- packages/excalidraw/CHANGELOG.md | 2 + packages/excalidraw/actions/actionCanvas.tsx | 6 +-- packages/excalidraw/element/bounds.ts | 31 +++++++++++- packages/excalidraw/index.tsx | 2 +- 8 files changed, 82 insertions(+), 54 deletions(-) diff --git a/excalidraw-app/app_constants.ts b/excalidraw-app/app_constants.ts index a20a2350..3402bf10 100644 --- a/excalidraw-app/app_constants.ts +++ b/excalidraw-app/app_constants.ts @@ -20,9 +20,12 @@ export const WS_EVENTS = { } as const; export enum WS_SUBTYPES { + INVALID_RESPONSE = "INVALID_RESPONSE", INIT = "SCENE_INIT", UPDATE = "SCENE_UPDATE", - USER_VIEWPORT_BOUNDS = "USER_VIEWPORT_BOUNDS", + MOUSE_LOCATION = "MOUSE_LOCATION", + IDLE_STATUS = "IDLE_STATUS", + USER_VISIBLE_SCENE_BOUNDS = "USER_VISIBLE_SCENE_BOUNDS", } export const FIREBASE_STORAGE_PREFIXES = { diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx index d55ec5f3..a0cdc277 100644 --- a/excalidraw-app/collab/Collab.tsx +++ b/excalidraw-app/collab/Collab.tsx @@ -18,10 +18,10 @@ import { } from "../../packages/excalidraw/index"; import { Collaborator, Gesture } from "../../packages/excalidraw/types"; import { + assertNever, preventUnload, resolvablePromise, throttleRAF, - viewportCoordsToSceneCoords, withBatchedUpdates, } from "../../packages/excalidraw/utils"; import { @@ -81,7 +81,8 @@ import { resetBrowserStateVersions } from "../data/tabSync"; import { LocalData } from "../data/LocalData"; import { atom, useAtom } from "jotai"; import { appJotaiStore } from "../app-jotai"; -import { Mutable } from "../../packages/excalidraw/utility-types"; +import { Mutable, ValueOf } from "../../packages/excalidraw/utility-types"; +import { getVisibleSceneBounds } from "../../packages/excalidraw/element/bounds"; export const collabAPIAtom = atom(null); export const collabDialogShownAtom = atom(false); @@ -174,7 +175,7 @@ class Collab extends PureComponent { this.portal.socket && this.portal.broadcastUserFollowed(payload); }); const throttledRelayUserViewportBounds = throttleRAF( - this.relayUserViewportBounds, + this.relayVisibleSceneBounds, ); const unsubOnScrollChange = this.excalidrawAPI.onScrollChange(() => throttledRelayUserViewportBounds(), @@ -384,7 +385,7 @@ class Collab extends PureComponent { iv: Uint8Array, encryptedData: ArrayBuffer, decryptionKey: string, - ) => { + ): Promise> => { try { const decrypted = await decryptData(iv, encryptedData, decryptionKey); @@ -396,7 +397,7 @@ class Collab extends PureComponent { window.alert(t("alerts.decryptFailed")); console.error(error); return { - type: "INVALID_RESPONSE", + type: WS_SUBTYPES.INVALID_RESPONSE, }; } }; @@ -512,7 +513,7 @@ class Collab extends PureComponent { ); switch (decryptedData.type) { - case "INVALID_RESPONSE": + case WS_SUBTYPES.INVALID_RESPONSE: return; case WS_SUBTYPES.INIT: { if (!this.portal.socketInitialized) { @@ -535,7 +536,7 @@ class Collab extends PureComponent { this.reconcileElements(decryptedData.payload.elements), ); break; - case "MOUSE_LOCATION": { + case WS_SUBTYPES.MOUSE_LOCATION: { const { pointer, button, username, selectedElementIds } = decryptedData.payload; @@ -554,8 +555,8 @@ class Collab extends PureComponent { break; } - case WS_SUBTYPES.USER_VIEWPORT_BOUNDS: { - const { bounds, socketId } = decryptedData.payload; + case WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS: { + const { sceneBounds, socketId } = decryptedData.payload; const appState = this.excalidrawAPI.getAppState(); @@ -579,7 +580,7 @@ class Collab extends PureComponent { this.excalidrawAPI.updateScene({ appState: zoomToFitBounds({ appState, - bounds, + bounds: sceneBounds, fitToViewport: true, viewportZoomFactor: 1, }).appState, @@ -588,7 +589,7 @@ class Collab extends PureComponent { break; } - case "IDLE_STATUS": { + case WS_SUBTYPES.IDLE_STATUS: { const { userState, socketId, username } = decryptedData.payload; this.updateCollaborator(socketId, { userState, @@ -596,6 +597,10 @@ class Collab extends PureComponent { }); break; } + + default: { + assertNever(decryptedData, null); + } } }, ); @@ -618,7 +623,7 @@ class Collab extends PureComponent { appState: { followedBy: new Set(followedBy) }, }); - this.relayUserViewportBounds({ shouldPerform: true }); + this.relayVisibleSceneBounds({ force: true }); }, ); @@ -848,25 +853,14 @@ class Collab extends PureComponent { CURSOR_SYNC_TIMEOUT, ); - relayUserViewportBounds = (props?: { shouldPerform: boolean }) => { + relayVisibleSceneBounds = (props?: { force: boolean }) => { const appState = this.excalidrawAPI.getAppState(); - if ( - this.portal.socket && - (appState.followedBy.size > 0 || props?.shouldPerform) - ) { - const { x: x1, y: y1 } = viewportCoordsToSceneCoords( - { clientX: 0, clientY: 0 }, - appState, - ); - - const { x: x2, y: y2 } = viewportCoordsToSceneCoords( - { clientX: appState.width, clientY: appState.height }, - appState, - ); - - this.portal.broadcastUserViewportBounds( - { bounds: [x1, y1, x2, y2] }, + if (this.portal.socket && (appState.followedBy.size > 0 || props?.force)) { + this.portal.broadcastVisibleSceneBounds( + { + sceneBounds: getVisibleSceneBounds(appState), + }, `follow@${this.portal.socket.id}`, ); } diff --git a/excalidraw-app/collab/Portal.tsx b/excalidraw-app/collab/Portal.tsx index 74ef27a0..66dd8a56 100644 --- a/excalidraw-app/collab/Portal.tsx +++ b/excalidraw-app/collab/Portal.tsx @@ -184,7 +184,7 @@ class Portal { broadcastIdleChange = (userState: UserIdleState) => { if (this.socket?.id) { const data: SocketUpdateDataSource["IDLE_STATUS"] = { - type: "IDLE_STATUS", + type: WS_SUBTYPES.IDLE_STATUS, payload: { socketId: this.socket.id, userState, @@ -204,7 +204,7 @@ class Portal { }) => { if (this.socket?.id) { const data: SocketUpdateDataSource["MOUSE_LOCATION"] = { - type: "MOUSE_LOCATION", + type: WS_SUBTYPES.MOUSE_LOCATION, payload: { socketId: this.socket.id, pointer: payload.pointer, @@ -222,19 +222,19 @@ class Portal { } }; - broadcastUserViewportBounds = ( + broadcastVisibleSceneBounds = ( payload: { - bounds: [number, number, number, number]; + sceneBounds: SocketUpdateDataSource["USER_VISIBLE_SCENE_BOUNDS"]["payload"]["sceneBounds"]; }, roomId: string, ) => { if (this.socket?.id) { - const data: SocketUpdateDataSource["USER_VIEWPORT_BOUNDS"] = { - type: WS_SUBTYPES.USER_VIEWPORT_BOUNDS, + const data: SocketUpdateDataSource["USER_VISIBLE_SCENE_BOUNDS"] = { + type: WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS, payload: { socketId: this.socket.id, username: this.collab.state.username, - bounds: payload.bounds, + sceneBounds: payload.sceneBounds, }, }; diff --git a/excalidraw-app/data/index.ts b/excalidraw-app/data/index.ts index b162da9a..fe44dc13 100644 --- a/excalidraw-app/data/index.ts +++ b/excalidraw-app/data/index.ts @@ -10,6 +10,7 @@ import { import { serializeAsJSON } from "../../packages/excalidraw/data/json"; import { restore } from "../../packages/excalidraw/data/restore"; import { ImportedDataState } from "../../packages/excalidraw/data/types"; +import { SceneBounds } from "../../packages/excalidraw/element/bounds"; import { isInvisiblySmallElement } from "../../packages/excalidraw/element/sizeHelpers"; import { isInitializedImageElement } from "../../packages/excalidraw/element/typeChecks"; import { @@ -28,6 +29,7 @@ import { DELETED_ELEMENT_TIMEOUT, FILE_UPLOAD_MAX_BYTES, ROOM_ID_BYTES, + WS_SUBTYPES, } from "../app_constants"; import { encodeFilesForUpload } from "./FileManager"; import { saveFilesToFirebase } from "./firebase"; @@ -97,20 +99,23 @@ export type EncryptedData = { }; export type SocketUpdateDataSource = { + INVALID_RESPONSE: { + type: WS_SUBTYPES.INVALID_RESPONSE; + }; SCENE_INIT: { - type: "SCENE_INIT"; + type: WS_SUBTYPES.INIT; payload: { elements: readonly ExcalidrawElement[]; }; }; SCENE_UPDATE: { - type: "SCENE_UPDATE"; + type: WS_SUBTYPES.UPDATE; payload: { elements: readonly ExcalidrawElement[]; }; }; MOUSE_LOCATION: { - type: "MOUSE_LOCATION"; + type: WS_SUBTYPES.MOUSE_LOCATION; payload: { socketId: string; pointer: { x: number; y: number; tool: "pointer" | "laser" }; @@ -119,16 +124,16 @@ export type SocketUpdateDataSource = { username: string; }; }; - USER_VIEWPORT_BOUNDS: { - type: "USER_VIEWPORT_BOUNDS"; + USER_VISIBLE_SCENE_BOUNDS: { + type: WS_SUBTYPES.USER_VISIBLE_SCENE_BOUNDS; payload: { socketId: string; username: string; - bounds: [number, number, number, number]; + sceneBounds: SceneBounds; }; }; IDLE_STATUS: { - type: "IDLE_STATUS"; + type: WS_SUBTYPES.IDLE_STATUS; payload: { socketId: string; userState: UserIdleState; @@ -138,10 +143,7 @@ export type SocketUpdateDataSource = { }; export type SocketUpdateDataIncoming = - | SocketUpdateDataSource[keyof SocketUpdateDataSource] - | { - type: "INVALID_RESPONSE"; - }; + SocketUpdateDataSource[keyof SocketUpdateDataSource]; export type SocketUpdateData = SocketUpdateDataSource[keyof SocketUpdateDataSource] & { diff --git a/packages/excalidraw/CHANGELOG.md b/packages/excalidraw/CHANGELOG.md index de8ce722..652c7784 100644 --- a/packages/excalidraw/CHANGELOG.md +++ b/packages/excalidraw/CHANGELOG.md @@ -13,6 +13,8 @@ Please add the latest change on the top under the correct section. ## Unreleased +- Expose `getVisibleSceneBounds` helper to get scene bounds of visible canvas area. [#7450](https://github.com/excalidraw/excalidraw/pull/7450) + ### Breaking Changes - `appState.openDialog` type was changed from `null | string` to `null | { name: string }`. [#7336](https://github.com/excalidraw/excalidraw/pull/7336) diff --git a/packages/excalidraw/actions/actionCanvas.tsx b/packages/excalidraw/actions/actionCanvas.tsx index 7d57c64a..ab5f8cfd 100644 --- a/packages/excalidraw/actions/actionCanvas.tsx +++ b/packages/excalidraw/actions/actionCanvas.tsx @@ -20,7 +20,7 @@ import { isHandToolActive, } from "../appState"; import { DEFAULT_CANVAS_BACKGROUND_PICKS } from "../colors"; -import { Bounds } from "../element/bounds"; +import { SceneBounds } from "../element/bounds"; import { setCursor } from "../cursor"; export const actionChangeViewBackgroundColor = register({ @@ -211,7 +211,7 @@ export const actionResetZoom = register({ }); const zoomValueToFitBoundsOnViewport = ( - bounds: Bounds, + bounds: SceneBounds, viewportDimensions: { width: number; height: number }, ) => { const [x1, y1, x2, y2] = bounds; @@ -235,7 +235,7 @@ export const zoomToFitBounds = ({ fitToViewport = false, viewportZoomFactor = 0.7, }: { - bounds: readonly [number, number, number, number]; + bounds: SceneBounds; appState: Readonly; /** whether to fit content to viewport (beyond >100%) */ fitToViewport: boolean; diff --git a/packages/excalidraw/element/bounds.ts b/packages/excalidraw/element/bounds.ts index f8d8223f..292fc995 100644 --- a/packages/excalidraw/element/bounds.ts +++ b/packages/excalidraw/element/bounds.ts @@ -9,7 +9,7 @@ import { import { distance2d, rotate, rotatePoint } from "../math"; import rough from "roughjs/bin/rough"; import { Drawable, Op } from "roughjs/bin/core"; -import { Point } from "../types"; +import { AppState, Point } from "../types"; import { generateRoughOptions } from "../scene/Shape"; import { isArrowElement, @@ -35,7 +35,9 @@ export type RectangleBox = { type MaybeQuadraticSolution = [number | null, number | null] | false; -// x and y position of top left corner, x and y position of bottom right corner +/** + * x and y position of top left corner, x and y position of bottom right corner + */ export type Bounds = readonly [ minX: number, minY: number, @@ -43,6 +45,13 @@ export type Bounds = readonly [ maxY: number, ]; +export type SceneBounds = readonly [ + sceneX: number, + sceneY: number, + sceneX2: number, + sceneY2: number, +]; + export class ElementBounds { private static boundsCache = new WeakMap< ExcalidrawElement, @@ -879,3 +888,21 @@ export const getCommonBoundingBox = ( midY: (minY + maxY) / 2, }; }; + +/** + * returns scene coords of user's editor viewport (visible canvas area) bounds + */ +export const getVisibleSceneBounds = ({ + scrollX, + scrollY, + width, + height, + zoom, +}: AppState): SceneBounds => { + return [ + -scrollX, + -scrollY, + -scrollX + width / zoom.value, + -scrollY + height / zoom.value, + ]; +}; diff --git a/packages/excalidraw/index.tsx b/packages/excalidraw/index.tsx index 74aa6d8d..915836cb 100644 --- a/packages/excalidraw/index.tsx +++ b/packages/excalidraw/index.tsx @@ -249,7 +249,7 @@ export { TTDDialogTrigger } from "./components/TTDDialog/TTDDialogTrigger"; export { normalizeLink } from "./data/url"; export { zoomToFitBounds } from "./actions/actionCanvas"; export { convertToExcalidrawElements } from "./data/transform"; -export { getCommonBounds } from "./element/bounds"; +export { getCommonBounds, getVisibleSceneBounds } from "./element/bounds"; export { elementsOverlappingBBox,