Merge pull request #1906 from robintown/speaker-jumpiness

Fix jumpy speaker tiles in spotlight mode

src/room/InCallView.tsx

@@ -489,8 +489,6 @@ function useParticipantTiles(
   });
 
   const items = useMemo(() => {
-    const hasPresenter =
-      sfuParticipants.find((p) => p.isScreenShareEnabled) !== undefined;
     let allGhosts = true;
 
     const speakActiveTime = new Date();
@@ -498,10 +496,9 @@ function useParticipantTiles(
     // Iterate over SFU participants (those who actually are present from the SFU perspective) and create tiles for them.
     const tiles: TileDescriptor<ItemData>[] = sfuParticipants.flatMap(
       (sfuParticipant) => {
-        const hadSpokedInTime =
-          !hasPresenter && sfuParticipant.lastSpokeAt
-            ? sfuParticipant.lastSpokeAt > speakActiveTime
-            : false;
+        const spokeRecently =
+          sfuParticipant.lastSpokeAt !== undefined &&
+          sfuParticipant.lastSpokeAt > speakActiveTime;
 
         const id = sfuParticipant.identity;
         const member = findMatrixMember(matrixRoom, id);
@@ -519,7 +516,7 @@ function useParticipantTiles(
           focused: false,
           isPresenter: sfuParticipant.isScreenShareEnabled,
           isSpeaker:
-            (sfuParticipant.isSpeaking || hadSpokedInTime) &&
+            (sfuParticipant.isSpeaking || spokeRecently) &&
             !sfuParticipant.isLocal,
           hasVideo: sfuParticipant.isCameraEnabled,
           local: sfuParticipant.isLocal,

src/video-grid/VideoGrid.tsx

@@ -54,7 +54,7 @@ interface TilePosition {
   zIndex: number;
 }
 
-interface Tile<T> {
+export interface Tile<T> {
   key: string;
   order: number;
   item: TileDescriptor<T>;
@@ -728,7 +728,7 @@ function displayedTileCount(
 
 // Sets the 'order' property on tiles based on the layout param and
 // other properties of the tiles, eg. 'focused' and 'presenter'
-function reorderTiles<T>(
+export function reorderTiles<T>(
   tiles: Tile<T>[],
   layout: Layout,
   displayedTile = -1,
@@ -750,7 +750,6 @@ function reorderTiles<T>(
   } else {
     const focusedTiles: Tile<T>[] = [];
     const presenterTiles: Tile<T>[] = [];
-    const speakerTiles: Tile<T>[] = [];
     const onlyVideoTiles: Tile<T>[] = [];
     const otherTiles: Tile<T>[] = [];
 
@@ -763,8 +762,6 @@ function reorderTiles<T>(
         focusedTiles.push(tile);
       } else if (tile.isPresenter) {
         presenterTiles.push(tile);
-      } else if (tile.isSpeaker && displayedTile < tile.order) {
-        speakerTiles.push(tile);
       } else if (tile.hasVideo) {
         if (tile.order === 0 && tile.item.local) {
           firstLocalTile = tile;
@@ -788,13 +785,27 @@ function reorderTiles<T>(
       }
     }
 
-    [
+    const reorderedTiles = [
       ...focusedTiles,
       ...presenterTiles,
-      ...speakerTiles,
       ...onlyVideoTiles,
       ...otherTiles,
-    ].forEach((tile, i) => (tile.order = i));
+    ];
+    let nextSpeakerTileIndex = focusedTiles.length + presenterTiles.length;
+
+    reorderedTiles.forEach((tile, i) => {
+      // If a speaker's natural ordering would place it outside the default
+      // visible area, promote them to the section dedicated to speakers
+      if (tile.isSpeaker && displayedTile <= i && nextSpeakerTileIndex < i) {
+        // Remove the tile from its current section
+        reorderedTiles.splice(i, 1);
+        // Insert it into the speaker section
+        reorderedTiles.splice(nextSpeakerTileIndex, 0, tile);
+        nextSpeakerTileIndex++;
+      }
+    });
+
+    reorderedTiles.forEach((tile, i) => (tile.order = i));
   }
 }
 

test/video-grid/VideoGrid-test.ts

@@ -0,0 +1,72 @@
+/*
+Copyright 2023 New Vector Ltd
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import {
+  Tile,
+  TileDescriptor,
+  reorderTiles,
+} from "../../src/video-grid/VideoGrid";
+
+const alice: Tile<unknown> = {
+  key: "alice",
+  order: 0,
+  item: { local: false } as unknown as TileDescriptor<unknown>,
+  remove: false,
+  focused: false,
+  isPresenter: false,
+  isSpeaker: false,
+  hasVideo: true,
+};
+const bob: Tile<unknown> = {
+  key: "bob",
+  order: 1,
+  item: { local: false } as unknown as TileDescriptor<unknown>,
+  remove: false,
+  focused: false,
+  isPresenter: false,
+  isSpeaker: false,
+  hasVideo: false,
+};
+
+test("reorderTiles does not promote a non-speaker", () => {
+  const tiles = [{ ...alice }, { ...bob }];
+  reorderTiles(tiles, "spotlight", 1);
+  expect(tiles).toEqual([
+    expect.objectContaining({ key: "alice", order: 0 }),
+    expect.objectContaining({ key: "bob", order: 1 }),
+  ]);
+});
+
+test("reorderTiles promotes a speaker into the visible area", () => {
+  const tiles = [{ ...alice }, { ...bob, isSpeaker: true }];
+  reorderTiles(tiles, "spotlight", 1);
+  expect(tiles).toEqual([
+    expect.objectContaining({ key: "alice", order: 1 }),
+    expect.objectContaining({ key: "bob", order: 0 }),
+  ]);
+});
+
+test("reorderTiles keeps a promoted speaker in the visible area", () => {
+  const tiles = [
+    { ...alice, order: 1 },
+    { ...bob, isSpeaker: true, order: 0 },
+  ];
+  reorderTiles(tiles, "spotlight", 1);
+  expect(tiles).toEqual([
+    expect.objectContaining({ key: "alice", order: 1 }),
+    expect.objectContaining({ key: "bob", order: 0 }),
+  ]);
+});