Determine which tiles are on screen in a more stable manner

Instead of tracking for each individual tile whether it's visible, just track the total number of tiles that appear on screen. This ought to make the whole thing a lot less dynamic, which is crucial given that our UI renders asynchronously and RxJS doesn't really support cyclic dependencies in any rigorous way.

In particular this ought to make the following kind of situation impossible:
1. There 3 tiles, ABC. A and B are on screen.
2. Now C becomes important. The requested order is now CAB.
3. To reduce the size of the layout shift, the algorithm selects to swap just B and C in the original order, giving ACB. However, the UI is blocked and doesn't render this order yet.
4. For whatever reason, a spurious update of the importance algorithm occurs. It once again requests CAB.
5. Now because the UI was blocked, the layout still thinks that A and B are on screen (rather than A and C). It thinks that C is some weird island of "off-screen territory" in the middle of the tile order. This confuses it into swapping A and C rather than keeping the layout stable.

The reality is that whenever we think N tiles are visible on screen, we're always referring to the first N tiles in the grid. It's best if the code reflects this assumption.

Author: robintown

src/grid/Grid.tsx

@@ -24,7 +24,6 @@ import {
   createContext,
   forwardRef,
   memo,
-  useCallback,
   useContext,
   useEffect,
   useMemo,
@@ -54,7 +53,6 @@ interface Tile<Model> {
   id: string;
   model: Model;
   onDrag: DragCallback | undefined;
-  setVisible: (visible: boolean) => void;
 }
 
 type PlacedTile<Model> = Tile<Model> & Rect;
@@ -88,7 +86,6 @@ interface SlotProps<Model> extends Omit<ComponentProps<"div">, "onDrag"> {
   id: string;
   model: Model;
   onDrag?: DragCallback;
-  onVisibilityChange?: (visible: boolean) => void;
   style?: CSSProperties;
   className?: string;
 }
@@ -115,24 +112,47 @@ function offset(element: HTMLElement, relativeTo: Element): Offset {
   }
 }
 
+export type VisibleTilesCallback = (visibleTiles: number) => void;
+
 interface LayoutContext {
   setGeneration: Dispatch<SetStateAction<number | null>>;
+  setVisibleTilesCallback: Dispatch<
+    SetStateAction<VisibleTilesCallback | null>
+  >;
 }
 
 const LayoutContext = createContext<LayoutContext | null>(null);
 
+function useLayoutContext(): LayoutContext {
+  const context = useContext(LayoutContext);
+  if (context === null)
+    throw new Error("useUpdateLayout called outside a Grid layout context");
+  return context;
+}
+
 /**
  * Enables Grid to react to layout changes. You must call this in your Layout
  * component or else Grid will not be reactive.
  */
 export function useUpdateLayout(): void {
-  const context = useContext(LayoutContext);
-  if (context === null)
-    throw new Error("useUpdateLayout called outside a Grid layout context");
-
+  const { setGeneration } = useLayoutContext();
   // On every render, tell Grid that the layout may have changed
-  useEffect(() =>
-    context.setGeneration((prev) => (prev === null ? 0 : prev + 1)),
+  useEffect(() => setGeneration((prev) => (prev === null ? 0 : prev + 1)));
+}
+
+/**
+ * Asks Grid to call a callback whenever the number of visible tiles may have
+ * changed.
+ */
+export function useVisibleTiles(callback: VisibleTilesCallback): void {
+  const { setVisibleTilesCallback } = useLayoutContext();
+  useEffect(
+    () => setVisibleTilesCallback(() => callback),
+    [callback, setVisibleTilesCallback],
+  );
+  useEffect(
+    () => (): void => setVisibleTilesCallback(null),
+    [setVisibleTilesCallback],
   );
 }
 
@@ -245,39 +265,20 @@ export function Grid<
   const windowHeight = useObservableEagerState(windowHeightObservable);
   const [layoutRoot, setLayoutRoot] = useState<HTMLElement | null>(null);
   const [generation, setGeneration] = useState<number | null>(null);
+  const [visibleTilesCallback, setVisibleTilesCallback] =
+    useState<VisibleTilesCallback | null>(null);
   const tiles = useInitial(() => new Map<string, Tile<TileModel>>());
   const prefersReducedMotion = usePrefersReducedMotion();
 
   const Slot: FC<SlotProps<TileModel>> = useMemo(
     () =>
-      function Slot({
-        id,
-        model,
-        onDrag,
-        onVisibilityChange,
-        style,
-        className,
-        ...props
-      }) {
+      function Slot({ id, model, onDrag, style, className, ...props }) {
         const ref = useRef<HTMLDivElement | null>(null);
-        const prevVisible = useRef<boolean | null>(null);
-        const setVisible = useCallback(
-          (visible: boolean) => {
-            if (
-              onVisibilityChange !== undefined &&
-              visible !== prevVisible.current
-            ) {
-              onVisibilityChange(visible);
-              prevVisible.current = visible;
-            }
-          },
-          [onVisibilityChange],
-        );
 
         useEffect(() => {
-          tiles.set(id, { id, model, onDrag, setVisible });
+          tiles.set(id, { id, model, onDrag });
           return (): void => void tiles.delete(id);
-        }, [id, model, onDrag, setVisible]);
+        }, [id, model, onDrag]);
 
         return (
           <div
@@ -307,7 +308,10 @@ export function Grid<
     [],
   );
 
-  const context: LayoutContext = useMemo(() => ({ setGeneration }), []);
+  const context: LayoutContext = useMemo(
+    () => ({ setGeneration, setVisibleTilesCallback }),
+    [setVisibleTilesCallback],
+  );
 
   // Combine the tile definitions and slots together to create placed tiles
   const placedTiles = useMemo(() => {
@@ -342,9 +346,11 @@ export function Grid<
   );
 
   useEffect(() => {
-    for (const tile of placedTiles)
-      tile.setVisible(tile.y + tile.height <= visibleHeight);
-  }, [placedTiles, visibleHeight]);
+    visibleTilesCallback?.(
+      placedTiles.filter((tile) => tile.y + tile.height <= visibleHeight)
+        .length,
+    );
+  }, [placedTiles, visibleTilesCallback, visibleHeight]);
 
   // Drag state is stored in a ref rather than component state, because we use
   // react-spring's imperative API during gestures to improve responsiveness

src/grid/GridLayout.tsx

@@ -13,7 +13,7 @@ import { type GridLayout as GridLayoutModel } from "../state/CallViewModel";
 import styles from "./GridLayout.module.css";
 import { useInitial } from "../useInitial";
 import { type CallLayout, arrangeTiles } from "./CallLayout";
-import { type DragCallback, useUpdateLayout } from "./Grid";
+import { type DragCallback, useUpdateLayout, useVisibleTiles } from "./Grid";
 
 interface GridCSSProperties extends CSSProperties {
   "--gap": string;
@@ -73,6 +73,7 @@ export const makeGridLayout: CallLayout<GridLayoutModel> = ({
   // The scrolling part of the layout is where all the grid tiles live
   scrolling: forwardRef(function GridLayout({ model, Slot }, ref) {
     useUpdateLayout();
+    useVisibleTiles(model.setVisibleTiles);
     const { width, height: minHeight } = useObservableEagerState(minBounds);
     const { gap, tileWidth, tileHeight } = useMemo(
       () => arrangeTiles(width, minHeight, model.grid.length),
@@ -93,13 +94,7 @@ export const makeGridLayout: CallLayout<GridLayoutModel> = ({
         }
       >
         {model.grid.map((m) => (
-          <Slot
-            key={m.id}
-            className={styles.slot}
-            id={m.id}
-            model={m}
-            onVisibilityChange={m.setVisible}
-          />
+          <Slot key={m.id} className={styles.slot} id={m.id} model={m} />
         ))}
       </div>
     );

src/grid/OneOnOneLayout.tsx

@@ -52,7 +52,6 @@ export const makeOneOnOneLayout: CallLayout<OneOnOneLayoutModel> = ({
         <Slot
           id={model.remote.id}
           model={model.remote}
-          onVisibilityChange={model.remote.setVisible}
           className={styles.container}
           style={{ width: tileWidth, height: tileHeight }}
         >
@@ -61,7 +60,6 @@ export const makeOneOnOneLayout: CallLayout<OneOnOneLayoutModel> = ({
             id={model.local.id}
             model={model.local}
             onDrag={onDragLocalTile}
-            onVisibilityChange={model.local.setVisible}
             data-block-alignment={pipAlignmentValue.block}
             data-inline-alignment={pipAlignmentValue.inline}
           />

src/grid/SpotlightExpandedLayout.tsx

@@ -63,7 +63,6 @@ export const makeSpotlightExpandedLayout: CallLayout<
             id={model.pip.id}
             model={model.pip}
             onDrag={onDragPip}
-            onVisibilityChange={model.pip.setVisible}
             data-block-alignment={pipAlignmentValue.block}
             data-inline-alignment={pipAlignmentValue.inline}
           />

src/grid/SpotlightLandscapeLayout.tsx

@@ -12,7 +12,7 @@ import classNames from "classnames";
 import { type CallLayout } from "./CallLayout";
 import { type SpotlightLandscapeLayout as SpotlightLandscapeLayoutModel } from "../state/CallViewModel";
 import styles from "./SpotlightLandscapeLayout.module.css";
-import { useUpdateLayout } from "./Grid";
+import { useUpdateLayout, useVisibleTiles } from "./Grid";
 
 /**
  * An implementation of the "spotlight landscape" layout, in which the spotlight
@@ -50,6 +50,7 @@ export const makeSpotlightLandscapeLayout: CallLayout<
     ref,
   ) {
     useUpdateLayout();
+    useVisibleTiles(model.setVisibleTiles);
     useObservableEagerState(minBounds);
     const withIndicators =
       useObservableEagerState(model.spotlight.media).length > 1;
@@ -63,13 +64,7 @@ export const makeSpotlightLandscapeLayout: CallLayout<
         />
         <div className={styles.grid}>
           {model.grid.map((m) => (
-            <Slot
-              key={m.id}
-              className={styles.slot}
-              id={m.id}
-              model={m}
-              onVisibilityChange={m.setVisible}
-            />
+            <Slot key={m.id} className={styles.slot} id={m.id} model={m} />
           ))}
         </div>
       </div>

src/grid/SpotlightPortraitLayout.tsx

@@ -12,7 +12,7 @@ import classNames from "classnames";
 import { type CallLayout, arrangeTiles } from "./CallLayout";
 import { type SpotlightPortraitLayout as SpotlightPortraitLayoutModel } from "../state/CallViewModel";
 import styles from "./SpotlightPortraitLayout.module.css";
-import { useUpdateLayout } from "./Grid";
+import { useUpdateLayout, useVisibleTiles } from "./Grid";
 
 interface GridCSSProperties extends CSSProperties {
   "--grid-gap": string;
@@ -54,6 +54,7 @@ export const makeSpotlightPortraitLayout: CallLayout<
     ref,
   ) {
     useUpdateLayout();
+    useVisibleTiles(model.setVisibleTiles);
     const { width } = useObservableEagerState(minBounds);
     const { gap, tileWidth, tileHeight } = arrangeTiles(
       width,
@@ -84,13 +85,7 @@ export const makeSpotlightPortraitLayout: CallLayout<
         />
         <div className={styles.grid}>
           {model.grid.map((m) => (
-            <Slot
-              key={m.id}
-              className={styles.slot}
-              id={m.id}
-              model={m}
-              onVisibilityChange={m.setVisible}
-            />
+            <Slot key={m.id} className={styles.slot} id={m.id} model={m} />
           ))}
         </div>
       </div>

src/state/CallViewModel.test.ts

@@ -402,10 +402,7 @@ test("participants stay in the same order unless to appear/disappear", () => {
             // We imagine that only three tiles (the first three) will be visible
             // on screen at a time
             vm.layout.subscribe((layout) => {
-              if (layout.type === "grid") {
-                for (let i = 0; i < layout.grid.length; i++)
-                  layout.grid[i].setVisible(i < 3);
-              }
+              if (layout.type === "grid") layout.setVisibleTiles(3);
             });
           },
         });