diff --git a/excalidraw-app/collab/Portal.tsx b/excalidraw-app/collab/Portal.tsx index 401b83ec51..82181f9c19 100644 --- a/excalidraw-app/collab/Portal.tsx +++ b/excalidraw-app/collab/Portal.tsx @@ -18,7 +18,7 @@ import throttle from "lodash.throttle"; import { newElementWith } from "../../src/element/mutateElement"; import { BroadcastedExcalidrawElement } from "./reconciliation"; import { encryptData } from "../../src/data/encryption"; -import { PRECEDING_ELEMENT_KEY } from "../../src/constants"; +import { normalizeFractionalIndexing } from "../../src/zindex"; class Portal { collab: TCollabClass; @@ -150,11 +150,7 @@ class Portal { this.broadcastedElementVersions.get(element.id)!) && isSyncableElement(element) ) { - acc.push({ - ...element, - // z-index info for the reconciler - [PRECEDING_ELEMENT_KEY]: idx === 0 ? "^" : elements[idx - 1]?.id, - }); + acc.push(element); } return acc; }, @@ -164,7 +160,7 @@ class Portal { const data: SocketUpdateDataSource[typeof updateType] = { type: updateType, payload: { - elements: syncableElements, + elements: normalizeFractionalIndexing(syncableElements), }, }; diff --git a/excalidraw-app/collab/reconciliation.ts b/excalidraw-app/collab/reconciliation.ts index 1efc5db460..8c873f1023 100644 --- a/excalidraw-app/collab/reconciliation.ts +++ b/excalidraw-app/collab/reconciliation.ts @@ -1,15 +1,13 @@ -import { PRECEDING_ELEMENT_KEY } from "../../src/constants"; import { ExcalidrawElement } from "../../src/element/types"; import { AppState } from "../../src/types"; -import { arrayToMapWithIndex } from "../../src/utils"; +import { arrayToMap, arrayToMapWithIndex } from "../../src/utils"; +import { orderByFractionalIndex } from "../../src/zindex"; export type ReconciledElements = readonly ExcalidrawElement[] & { _brand: "reconciledElements"; }; -export type BroadcastedExcalidrawElement = ExcalidrawElement & { - [PRECEDING_ELEMENT_KEY]?: string; -}; +export type BroadcastedExcalidrawElement = ExcalidrawElement; const shouldDiscardRemoteElement = ( localAppState: AppState, @@ -39,116 +37,43 @@ export const reconcileElements = ( remoteElements: readonly BroadcastedExcalidrawElement[], localAppState: AppState, ): ReconciledElements => { - const localElementsData = - arrayToMapWithIndex(localElements); - - const reconciledElements: ExcalidrawElement[] = localElements.slice(); - - const duplicates = new WeakMap(); + const localElementsData = arrayToMap(localElements); + const reconciledElements: ExcalidrawElement[] = []; + const added = new Set(); - let cursor = 0; - let offset = 0; - - let remoteElementIdx = -1; + // process remote elements for (const remoteElement of remoteElements) { - remoteElementIdx++; - - const local = localElementsData.get(remoteElement.id); + if (localElementsData.has(remoteElement.id)) { + const localElement = localElementsData.get(remoteElement.id); - if (shouldDiscardRemoteElement(localAppState, local?.[0], remoteElement)) { - if (remoteElement[PRECEDING_ELEMENT_KEY]) { - delete remoteElement[PRECEDING_ELEMENT_KEY]; - } - - continue; - } - - // Mark duplicate for removal as it'll be replaced with the remote element - if (local) { - // Unless the remote and local elements are the same element in which case - // we need to keep it as we'd otherwise discard it from the resulting - // array. - if (local[0] === remoteElement) { + if ( + localElement && + shouldDiscardRemoteElement(localAppState, localElement, remoteElement) + ) { continue; - } - duplicates.set(local[0], true); - } - - // parent may not be defined in case the remote client is running an older - // excalidraw version - const parent = - remoteElement[PRECEDING_ELEMENT_KEY] || - remoteElements[remoteElementIdx - 1]?.id || - null; - - if (parent != null) { - delete remoteElement[PRECEDING_ELEMENT_KEY]; - - // ^ indicates the element is the first in elements array - if (parent === "^") { - offset++; - if (cursor === 0) { - reconciledElements.unshift(remoteElement); - localElementsData.set(remoteElement.id, [ - remoteElement, - cursor - offset, - ]); - } else { - reconciledElements.splice(cursor + 1, 0, remoteElement); - localElementsData.set(remoteElement.id, [ - remoteElement, - cursor + 1 - offset, - ]); - cursor++; - } } else { - let idx = localElementsData.has(parent) - ? localElementsData.get(parent)![1] - : null; - if (idx != null) { - idx += offset; - } - if (idx != null && idx >= cursor) { - reconciledElements.splice(idx + 1, 0, remoteElement); - offset++; - localElementsData.set(remoteElement.id, [ - remoteElement, - idx + 1 - offset, - ]); - cursor = idx + 1; - } else if (idx != null) { - reconciledElements.splice(cursor + 1, 0, remoteElement); - offset++; - localElementsData.set(remoteElement.id, [ - remoteElement, - cursor + 1 - offset, - ]); - cursor++; - } else { + if (!added.has(remoteElement.id)) { reconciledElements.push(remoteElement); - localElementsData.set(remoteElement.id, [ - remoteElement, - reconciledElements.length - 1 - offset, - ]); + added.add(remoteElement.id); } } - // no parent z-index information, local element exists → replace in place - } else if (local) { - reconciledElements[local[1]] = remoteElement; - localElementsData.set(remoteElement.id, [remoteElement, local[1]]); - // otherwise push to the end } else { - reconciledElements.push(remoteElement); - localElementsData.set(remoteElement.id, [ - remoteElement, - reconciledElements.length - 1 - offset, - ]); + if (!added.has(remoteElement.id)) { + reconciledElements.push(remoteElement); + added.add(remoteElement.id); + } } } - const ret: readonly ExcalidrawElement[] = reconciledElements.filter( - (element) => !duplicates.has(element), - ); + // process local elements + for (const localElement of localElements) { + if (!added.has(localElement.id)) { + reconciledElements.push(localElement); + added.add(localElement.id); + } + } - return ret as ReconciledElements; + return orderByFractionalIndex( + reconciledElements, + ) as readonly ExcalidrawElement[] as ReconciledElements; }; diff --git a/excalidraw-app/tests/reconciliation.test.ts b/excalidraw-app/tests/reconciliation.test.ts index 26fb5b8ca0..1d73c3cef0 100644 --- a/excalidraw-app/tests/reconciliation.test.ts +++ b/excalidraw-app/tests/reconciliation.test.ts @@ -1,5 +1,4 @@ import { expect } from "chai"; -import { PRECEDING_ELEMENT_KEY } from "../../src/constants"; import { ExcalidrawElement } from "../../src/element/types"; import { BroadcastedExcalidrawElement, @@ -15,7 +14,6 @@ type ElementLike = { id: string; version: number; versionNonce: number; - [PRECEDING_ELEMENT_KEY]?: string | null; }; type Cache = Record; @@ -44,7 +42,6 @@ const createElement = (opts: { uid: string } | ElementLike) => { id, version, versionNonce: versionNonce || randomInteger(), - [PRECEDING_ELEMENT_KEY]: parent || null, }; }; @@ -53,20 +50,15 @@ const idsToElements = ( cache: Cache = {}, ): readonly ExcalidrawElement[] => { return ids.reduce((acc, _uid, idx) => { - const { - uid, - id, - version, - [PRECEDING_ELEMENT_KEY]: parent, - versionNonce, - } = createElement(typeof _uid === "string" ? { uid: _uid } : _uid); + const { uid, id, version, versionNonce } = createElement( + typeof _uid === "string" ? { uid: _uid } : _uid, + ); const cached = cache[uid]; const elem = { id, version: version ?? 0, versionNonce, ...cached, - [PRECEDING_ELEMENT_KEY]: parent, } as BroadcastedExcalidrawElement; // @ts-ignore cache[uid] = elem; @@ -77,7 +69,6 @@ const idsToElements = ( const addParents = (elements: BroadcastedExcalidrawElement[]) => { return elements.map((el, idx, els) => { - el[PRECEDING_ELEMENT_KEY] = els[idx - 1]?.id || "^"; return el; }); }; @@ -389,13 +380,11 @@ describe("elements reconciliation", () => { id: "A", version: 1, versionNonce: 1, - [PRECEDING_ELEMENT_KEY]: null, }, { id: "B", version: 1, versionNonce: 1, - [PRECEDING_ELEMENT_KEY]: null, }, ]; @@ -408,13 +397,11 @@ describe("elements reconciliation", () => { id: "A", version: 1, versionNonce: 1, - [PRECEDING_ELEMENT_KEY]: null, }; const el2 = { id: "B", version: 1, versionNonce: 1, - [PRECEDING_ELEMENT_KEY]: null, }; testIdentical([el1, el2], [el2, el1], ["A", "B"]); }); diff --git a/src/constants.ts b/src/constants.ts index 902706201f..6cf75fb59e 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -302,10 +302,6 @@ export const ROUNDNESS = { ADAPTIVE_RADIUS: 3, } as const; -/** key containt id of precedeing elemnt id we use in reconciliation during - * collaboration */ -export const PRECEDING_ELEMENT_KEY = "__precedingElement__"; - export const ROUGHNESS = { architect: 0, artist: 1,