Просмотр исходного кода

perf: Improved pointer events related performance when the sidebar is docked with a large library open (#9086)

Co-authored-by: dwelle <[email protected]>
tothatt81 6 месяцев назад
Родитель
Сommit
4f64372506
3 измененных файлов с 211 добавлено и 151 удалено
  1. 18 7
      packages/excalidraw/components/App.tsx
  2. 190 144
      packages/excalidraw/components/LibraryMenu.tsx
  3. 3 0
      setupTests.ts

+ 18 - 7
packages/excalidraw/components/App.tsx

@@ -1522,13 +1522,17 @@ class App extends React.Component<AppProps, AppState> {
     const allElementsMap = this.scene.getNonDeletedElementsMap();
 
     const shouldBlockPointerEvents =
-      this.state.selectionElement ||
-      this.state.newElement ||
-      this.state.selectedElementsAreBeingDragged ||
-      this.state.resizingElement ||
-      (this.state.activeTool.type === "laser" &&
-        // technically we can just test on this once we make it more safe
-        this.state.cursorButton === "down");
+      // default back to `--ui-pointerEvents` flow if setPointerCapture
+      // not supported
+      "setPointerCapture" in HTMLElement.prototype
+        ? false
+        : this.state.selectionElement ||
+          this.state.newElement ||
+          this.state.selectedElementsAreBeingDragged ||
+          this.state.resizingElement ||
+          (this.state.activeTool.type === "laser" &&
+            // technically we can just test on this once we make it more safe
+            this.state.cursorButton === "down");
 
     const firstSelectedElement = selectedElements[0];
 
@@ -6295,6 +6299,13 @@ class App extends React.Component<AppProps, AppState> {
   private handleCanvasPointerDown = (
     event: React.PointerEvent<HTMLElement>,
   ) => {
+    const target = event.target as HTMLElement;
+    // capture subsequent pointer events to the canvas
+    // this makes other elements non-interactive until pointer up
+    if (target.setPointerCapture) {
+      target.setPointerCapture(event.pointerId);
+    }
+
     this.maybeCleanupAfterMissingPointerUp(event.nativeEvent);
     this.maybeUnfollowRemoteUser();
 

+ 190 - 144
packages/excalidraw/components/LibraryMenu.tsx

@@ -2,8 +2,9 @@ import React, {
   useState,
   useCallback,
   useMemo,
-  useRef,
   useEffect,
+  memo,
+  useRef,
 } from "react";
 import type Library from "../data/library";
 import {
@@ -17,6 +18,7 @@ import type {
   LibraryItem,
   ExcalidrawProps,
   UIAppState,
+  AppClassProperties,
 } from "../types";
 import LibraryMenuItems from "./LibraryMenuItems";
 import { trackEvent } from "../analytics";
@@ -33,9 +35,12 @@ import { useUIAppState } from "../context/ui-appState";
 
 import "./LibraryMenu.scss";
 import { LibraryMenuControlButtons } from "./LibraryMenuControlButtons";
-import { isShallowEqual } from "../utils";
-import type { NonDeletedExcalidrawElement } from "../element/types";
+import type {
+  ExcalidrawElement,
+  NonDeletedExcalidrawElement,
+} from "../element/types";
 import { LIBRARY_DISABLED_TYPES } from "../constants";
+import { isShallowEqual } from "../utils";
 
 export const isLibraryMenuOpenAtom = atom(false);
 
@@ -43,170 +48,215 @@ const LibraryMenuWrapper = ({ children }: { children: React.ReactNode }) => {
   return <div className="layer-ui__library">{children}</div>;
 };
 
-export const LibraryMenuContent = ({
-  onInsertLibraryItems,
-  pendingElements,
-  onAddToLibrary,
-  setAppState,
-  libraryReturnUrl,
-  library,
-  id,
-  theme,
-  selectedItems,
-  onSelectItems,
-}: {
-  pendingElements: LibraryItem["elements"];
-  onInsertLibraryItems: (libraryItems: LibraryItems) => void;
-  onAddToLibrary: () => void;
-  setAppState: React.Component<any, UIAppState>["setState"];
-  libraryReturnUrl: ExcalidrawProps["libraryReturnUrl"];
-  library: Library;
-  id: string;
-  theme: UIAppState["theme"];
-  selectedItems: LibraryItem["id"][];
-  onSelectItems: (id: LibraryItem["id"][]) => void;
-}) => {
-  const [libraryItemsData] = useAtom(libraryItemsAtom);
-
-  const _onAddToLibrary = useCallback(
-    (elements: LibraryItem["elements"]) => {
-      const addToLibrary = async (
-        processedElements: LibraryItem["elements"],
-        libraryItems: LibraryItems,
-      ) => {
-        trackEvent("element", "addToLibrary", "ui");
-        for (const type of LIBRARY_DISABLED_TYPES) {
-          if (processedElements.some((element) => element.type === type)) {
-            return setAppState({
-              errorMessage: t(`errors.libraryElementTypeError.${type}`),
-            });
+const LibraryMenuContent = memo(
+  ({
+    onInsertLibraryItems,
+    pendingElements,
+    onAddToLibrary,
+    setAppState,
+    libraryReturnUrl,
+    library,
+    id,
+    theme,
+    selectedItems,
+    onSelectItems,
+  }: {
+    pendingElements: LibraryItem["elements"];
+    onInsertLibraryItems: (libraryItems: LibraryItems) => void;
+    onAddToLibrary: () => void;
+    setAppState: React.Component<any, UIAppState>["setState"];
+    libraryReturnUrl: ExcalidrawProps["libraryReturnUrl"];
+    library: Library;
+    id: string;
+    theme: UIAppState["theme"];
+    selectedItems: LibraryItem["id"][];
+    onSelectItems: (id: LibraryItem["id"][]) => void;
+  }) => {
+    const [libraryItemsData] = useAtom(libraryItemsAtom);
+
+    const _onAddToLibrary = useCallback(
+      (elements: LibraryItem["elements"]) => {
+        const addToLibrary = async (
+          processedElements: LibraryItem["elements"],
+          libraryItems: LibraryItems,
+        ) => {
+          trackEvent("element", "addToLibrary", "ui");
+          for (const type of LIBRARY_DISABLED_TYPES) {
+            if (processedElements.some((element) => element.type === type)) {
+              return setAppState({
+                errorMessage: t(`errors.libraryElementTypeError.${type}`),
+              });
+            }
           }
-        }
-        const nextItems: LibraryItems = [
-          {
-            status: "unpublished",
-            elements: processedElements,
-            id: randomId(),
-            created: Date.now(),
-          },
-          ...libraryItems,
-        ];
-        onAddToLibrary();
-        library.setLibrary(nextItems).catch(() => {
-          setAppState({ errorMessage: t("alerts.errorAddingToLibrary") });
-        });
-      };
-      addToLibrary(elements, libraryItemsData.libraryItems);
-    },
-    [onAddToLibrary, library, setAppState, libraryItemsData.libraryItems],
-  );
+          const nextItems: LibraryItems = [
+            {
+              status: "unpublished",
+              elements: processedElements,
+              id: randomId(),
+              created: Date.now(),
+            },
+            ...libraryItems,
+          ];
+          onAddToLibrary();
+          library.setLibrary(nextItems).catch(() => {
+            setAppState({ errorMessage: t("alerts.errorAddingToLibrary") });
+          });
+        };
+        addToLibrary(elements, libraryItemsData.libraryItems);
+      },
+      [onAddToLibrary, library, setAppState, libraryItemsData.libraryItems],
+    );
 
-  const libraryItems = useMemo(
-    () => libraryItemsData.libraryItems,
-    [libraryItemsData],
-  );
+    const libraryItems = useMemo(
+      () => libraryItemsData.libraryItems,
+      [libraryItemsData],
+    );
 
-  if (
-    libraryItemsData.status === "loading" &&
-    !libraryItemsData.isInitialized
-  ) {
-    return (
-      <LibraryMenuWrapper>
-        <div className="layer-ui__library-message">
-          <div>
-            <Spinner size="2em" />
-            <span>{t("labels.libraryLoadingMessage")}</span>
+    if (
+      libraryItemsData.status === "loading" &&
+      !libraryItemsData.isInitialized
+    ) {
+      return (
+        <LibraryMenuWrapper>
+          <div className="layer-ui__library-message">
+            <div>
+              <Spinner size="2em" />
+              <span>{t("labels.libraryLoadingMessage")}</span>
+            </div>
           </div>
-        </div>
-      </LibraryMenuWrapper>
-    );
-  }
+        </LibraryMenuWrapper>
+      );
+    }
 
-  const showBtn =
-    libraryItemsData.libraryItems.length > 0 || pendingElements.length > 0;
+    const showBtn =
+      libraryItemsData.libraryItems.length > 0 || pendingElements.length > 0;
 
-  return (
-    <LibraryMenuWrapper>
-      <LibraryMenuItems
-        isLoading={libraryItemsData.status === "loading"}
-        libraryItems={libraryItems}
-        onAddToLibrary={_onAddToLibrary}
-        onInsertLibraryItems={onInsertLibraryItems}
-        pendingElements={pendingElements}
-        id={id}
-        libraryReturnUrl={libraryReturnUrl}
-        theme={theme}
-        onSelectItems={onSelectItems}
-        selectedItems={selectedItems}
-      />
-      {showBtn && (
-        <LibraryMenuControlButtons
-          className="library-menu-control-buttons--at-bottom"
-          style={{ padding: "16px 12px 0 12px" }}
+    return (
+      <LibraryMenuWrapper>
+        <LibraryMenuItems
+          isLoading={libraryItemsData.status === "loading"}
+          libraryItems={libraryItems}
+          onAddToLibrary={_onAddToLibrary}
+          onInsertLibraryItems={onInsertLibraryItems}
+          pendingElements={pendingElements}
           id={id}
           libraryReturnUrl={libraryReturnUrl}
           theme={theme}
+          onSelectItems={onSelectItems}
+          selectedItems={selectedItems}
         />
-      )}
-    </LibraryMenuWrapper>
-  );
-};
+        {showBtn && (
+          <LibraryMenuControlButtons
+            className="library-menu-control-buttons--at-bottom"
+            style={{ padding: "16px 12px 0 12px" }}
+            id={id}
+            libraryReturnUrl={libraryReturnUrl}
+            theme={theme}
+          />
+        )}
+      </LibraryMenuWrapper>
+    );
+  },
+);
+
+const getPendingElements = (
+  elements: readonly NonDeletedExcalidrawElement[],
+  selectedElementIds: UIAppState["selectedElementIds"],
+) => ({
+  elements,
+  pending: getSelectedElements(
+    elements,
+    { selectedElementIds },
+    {
+      includeBoundTextElement: true,
+      includeElementsInFrames: true,
+    },
+  ),
+  selectedElementIds,
+});
 
 const usePendingElementsMemo = (
   appState: UIAppState,
-  elements: readonly NonDeletedExcalidrawElement[],
+  app: AppClassProperties,
 ) => {
-  const create = useCallback(
-    (appState: UIAppState, elements: readonly NonDeletedExcalidrawElement[]) =>
-      getSelectedElements(elements, appState, {
-        includeBoundTextElement: true,
-        includeElementsInFrames: true,
-      }),
-    [],
+  const elements = useExcalidrawElements();
+  const [state, setState] = useState(() =>
+    getPendingElements(elements, appState.selectedElementIds),
+  );
+
+  const selectedElementVersions = useRef(
+    new Map<ExcalidrawElement["id"], ExcalidrawElement["version"]>(),
   );
 
-  const val = useRef(create(appState, elements));
-  const prevAppState = useRef<UIAppState>(appState);
-  const prevElements = useRef(elements);
+  useEffect(() => {
+    for (const element of state.pending) {
+      selectedElementVersions.current.set(element.id, element.version);
+    }
+  }, [state.pending]);
 
-  const update = useCallback(() => {
+  useEffect(() => {
     if (
-      !isShallowEqual(
-        appState.selectedElementIds,
-        prevAppState.current.selectedElementIds,
-      ) ||
-      !isShallowEqual(elements, prevElements.current)
+      // Only update once pointer is released.
+      // Reading directly from app.state to make it clear it's not reactive
+      // (hence, there's potential for stale state)
+      app.state.cursorButton === "up" &&
+      app.state.activeTool.type === "selection"
     ) {
-      val.current = create(appState, elements);
-      prevAppState.current = appState;
-      prevElements.current = elements;
+      setState((prev) => {
+        // if selectedElementIds changed, we don't have to compare versions
+        // ---------------------------------------------------------------------
+        if (
+          !isShallowEqual(prev.selectedElementIds, appState.selectedElementIds)
+        ) {
+          selectedElementVersions.current.clear();
+          return getPendingElements(elements, appState.selectedElementIds);
+        }
+        // otherwise we need to check whether selected elements changed
+        // ---------------------------------------------------------------------
+        const elementsMap = app.scene.getNonDeletedElementsMap();
+        for (const id of Object.keys(appState.selectedElementIds)) {
+          const currVersion = elementsMap.get(id)?.version;
+          if (
+            currVersion &&
+            currVersion !== selectedElementVersions.current.get(id)
+          ) {
+            // we can't update the selectedElementVersions in here
+            // because of double render in StrictMode which would overwrite
+            // the state in the second pass with the old `prev` state.
+            // Thus, we update versions in a separate effect. May create
+            // a race condition since current effect is not fully reactive.
+            return getPendingElements(elements, appState.selectedElementIds);
+          }
+        }
+        // nothing changed
+        // ---------------------------------------------------------------------
+        return prev;
+      });
     }
-  }, [create, appState, elements]);
-
-  return useMemo(
-    () => ({
-      update,
-      value: val.current,
-    }),
-    [update, val],
-  );
+  }, [
+    app,
+    app.state.cursorButton,
+    app.state.activeTool.type,
+    appState.selectedElementIds,
+    elements,
+  ]);
+
+  return state.pending;
 };
 
 /**
  * This component is meant to be rendered inside <Sidebar.Tab/> inside our
  * <DefaultSidebar/> or host apps Sidebar components.
  */
-export const LibraryMenu = () => {
-  const { library, id, onInsertElements } = useApp();
+export const LibraryMenu = memo(() => {
+  const app = useApp();
+  const { onInsertElements } = app;
   const appProps = useAppProps();
   const appState = useUIAppState();
-  const app = useApp();
   const setAppState = useExcalidrawSetAppState();
-  const elements = useExcalidrawElements();
   const [selectedItems, setSelectedItems] = useState<LibraryItem["id"][]>([]);
-  const memoizedLibrary = useMemo(() => library, [library]);
-  // BUG: pendingElements are still causing some unnecessary rerenders because clicking into canvas returns some ids even when no element is selected.
-  const pendingElements = usePendingElementsMemo(appState, elements);
+  const memoizedLibrary = useMemo(() => app.library, [app.library]);
+  const pendingElements = usePendingElementsMemo(appState, app);
 
   const onInsertLibraryItems = useCallback(
     (libraryItems: LibraryItems) => {
@@ -223,22 +273,18 @@ export const LibraryMenu = () => {
     });
   }, [setAppState]);
 
-  useEffect(() => {
-    return app.onPointerUpEmitter.on(() => pendingElements.update());
-  }, [app, pendingElements]);
-
   return (
     <LibraryMenuContent
-      pendingElements={pendingElements.value}
+      pendingElements={pendingElements}
       onInsertLibraryItems={onInsertLibraryItems}
       onAddToLibrary={deselectItems}
       setAppState={setAppState}
       libraryReturnUrl={appProps.libraryReturnUrl}
       library={memoizedLibrary}
-      id={id}
+      id={app.id}
       theme={appState.theme}
       selectedItems={selectedItems}
       onSelectItems={setSelectedItems}
     />
   );
-};
+});

+ 3 - 0
setupTests.ts

@@ -7,6 +7,9 @@ import polyfill from "./packages/excalidraw/polyfill";
 import { testPolyfills } from "./packages/excalidraw/tests/helpers/polyfills";
 import { yellow } from "./packages/excalidraw/tests/helpers/colorize";
 
+// mock for pep.js not working with setPointerCapture()
+HTMLElement.prototype.setPointerCapture = vi.fn();
+
 Object.assign(globalThis, testPolyfills);
 
 require("fake-indexeddb/auto");