From 51964d2614396f04ea35a99aa8909875f15b465e Mon Sep 17 00:00:00 2001
From: Brent Yi <yibrenth@gmail.com>
Date: Mon, 11 Nov 2024 20:37:13 -0800
Subject: [PATCH] Significantly reduce scene tree remounts, display
 improvements (#328)

* Reduce remounting for scene tree table

* Cleanup

* Remove extra wrapper

* cleanup
---
 .../src/ControlPanel/SceneTreeTable.css.ts    | 30 +++++++++
 .../src/ControlPanel/SceneTreeTable.tsx       | 64 ++++++++-----------
 2 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/src/viser/client/src/ControlPanel/SceneTreeTable.css.ts b/src/viser/client/src/ControlPanel/SceneTreeTable.css.ts
index 8e25fecb..7718d08f 100644
--- a/src/viser/client/src/ControlPanel/SceneTreeTable.css.ts
+++ b/src/viser/client/src/ControlPanel/SceneTreeTable.css.ts
@@ -37,6 +37,36 @@ export const tableRow = style({
   padding: "0 0.25em",
   lineHeight: "2em",
   fontSize: "0.875em",
+  ":hover": {
+    [vars.lightSelector]: {
+      backgroundColor: vars.colors.gray[1],
+    },
+    [vars.darkSelector]: {
+      backgroundColor: vars.colors.dark[6],
+    },
+  },
+});
+
+export const tableHierarchyLine = style({
+  [vars.lightSelector]: {
+    borderColor: vars.colors.gray[2],
+  },
+  [vars.darkSelector]: {
+    borderColor: vars.colors.dark[5],
+  },
+  borderLeft: "0.3em solid",
+  width: "0.2em",
+  marginLeft: "0.375em",
+  height: "2em",
+});
+
+globalStyle(`${tableRow}:hover ${tableHierarchyLine}`, {
+  [vars.lightSelector]: {
+    borderColor: vars.colors.gray[3],
+  },
+  [vars.darkSelector]: {
+    borderColor: vars.colors.dark[4],
+  },
 });
 
 globalStyle(`${tableRow}:hover ${editIconWrapper}`, {
diff --git a/src/viser/client/src/ControlPanel/SceneTreeTable.tsx b/src/viser/client/src/ControlPanel/SceneTreeTable.tsx
index f8c01a79..24aa4876 100644
--- a/src/viser/client/src/ControlPanel/SceneTreeTable.tsx
+++ b/src/viser/client/src/ControlPanel/SceneTreeTable.tsx
@@ -11,6 +11,7 @@ import React from "react";
 import {
   editIconWrapper,
   propsWrapper,
+  tableHierarchyLine,
   tableRow,
   tableWrapper,
 } from "./SceneTreeTable.css";
@@ -24,8 +25,6 @@ import {
   TextInput,
   Tooltip,
   ColorInput,
-  useMantineColorScheme,
-  useMantineTheme,
 } from "@mantine/core";
 
 function EditNodeProps({
@@ -297,43 +296,42 @@ const SceneTreeTableRow = React.memo(function SceneTreeTableRow(props: {
 
   const [expanded, { toggle: toggleExpanded }] = useDisclosure(false);
 
-  function setOverrideVisibility(name: string, visible: boolean | undefined) {
-    const attr = viewer.nodeAttributesFromName.current;
-    attr[name]!.overrideVisibility = visible;
-    rerenderTable();
-  }
   const setLabelVisibility = viewer.useSceneTree(
     (state) => state.setLabelVisibility,
   );
 
-  // For performance, scene node visibility is stored in a ref instead of the
-  // zustand state. This means that re-renders for the table need to be
-  // triggered manually when visibilities are updated.
-  const [, setTime] = React.useState(Date.now());
-  function rerenderTable() {
-    setTime(Date.now());
-  }
+  const pollIsVisible = React.useCallback(() => {
+    const attrs = viewer.nodeAttributesFromName.current[props.nodeName];
+    return (
+      (attrs?.overrideVisibility === undefined
+        ? attrs?.visibility
+        : attrs.overrideVisibility) ?? true
+    );
+  }, [props.nodeName]);
+
+  const [isVisible, setIsVisible] = React.useState(pollIsVisible());
   React.useEffect(() => {
-    const interval = setInterval(rerenderTable, 200);
+    // We put the visibility in a ref, so it needs to be polled. This was for
+    // performance reasons, but we should probably move it into the zustand
+    // store and just be careful to avoid subscribing to it from the r3f
+    // components.
+    const interval = setInterval(() => {
+      const visible = pollIsVisible();
+      if (visible !== isVisible) {
+        setIsVisible(visible);
+      }
+    }, 200);
     return () => {
       clearInterval(interval);
     };
-  }, []);
+  }, [isVisible]);
 
-  const attrs = viewer.nodeAttributesFromName.current[props.nodeName];
-  const isVisible =
-    (attrs?.overrideVisibility === undefined
-      ? attrs?.visibility
-      : attrs.overrideVisibility) ?? true;
   const isVisibleEffective = isVisible && props.isParentVisible;
   const VisibleIcon = isVisible ? IconEye : IconEyeOff;
 
   const [propsPanelOpened, { open: openPropsPanel, close: closePropsPanel }] =
     useDisclosure(false);
 
-  const colorScheme = useMantineColorScheme();
-  const theme = useMantineTheme();
-
   return (
     <>
       <Box
@@ -346,19 +344,7 @@ const SceneTreeTableRow = React.memo(function SceneTreeTableRow(props: {
         onMouseOut={() => setLabelVisibility(props.nodeName, false)}
       >
         {new Array(props.indentCount).fill(null).map((_, i) => (
-          <Box
-            key={i}
-            style={{
-              borderLeft: "0.3em solid",
-              borderColor:
-                colorScheme.colorScheme == "dark"
-                  ? theme.colors.gray[7]
-                  : theme.colors.gray[2],
-              width: "0.2em",
-              marginLeft: "0.375em",
-              height: "2em",
-            }}
-          />
+          <Box className={tableHierarchyLine} key={i} />
         ))}
         <Box
           style={{
@@ -395,7 +381,9 @@ const SceneTreeTableRow = React.memo(function SceneTreeTableRow(props: {
               }}
               onClick={(evt) => {
                 evt.stopPropagation();
-                setOverrideVisibility(props.nodeName, !isVisible);
+                const attr = viewer.nodeAttributesFromName.current;
+                attr[props.nodeName]!.overrideVisibility = !isVisible;
+                setIsVisible(!isVisible);
               }}
             />
           </Tooltip>