From 8b8d6fd0e089d3b7d00282651b360556eb75cf9d Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sat, 17 Jun 2023 15:21:38 -0400 Subject: [PATCH 1/8] Push large tiles upwards back into the grid --- src/array-utils.ts | 38 ++++++++++ src/video-grid/model.ts | 136 +++++++++++++++++++++++++++++----- test/video-grid/model-test.ts | 44 +++++++++++ 3 files changed, 199 insertions(+), 19 deletions(-) create mode 100644 src/array-utils.ts diff --git a/src/array-utils.ts b/src/array-utils.ts new file mode 100644 index 0000000..cf37e4c --- /dev/null +++ b/src/array-utils.ts @@ -0,0 +1,38 @@ +/* +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. +*/ + +/** + * Gets the index of the last element in the array to satsify the given + * predicate. + */ +// TODO: remove this once TypeScript recognizes the existence of +// Array.prototype.findLastIndex +export function findLastIndex( + array: T[], + predicate: (item: T) => boolean +): number | null { + for (let i = array.length - 1; i >= 0; i--) { + if (predicate(array[i])) return i; + } + + return null; +} + +/** + * Counts the number of elements in an array that satsify the given predicate. + */ +export const count = (array: T[], predicate: (item: T) => boolean): number => + array.reduce((acc, item) => (predicate(item) ? acc + 1 : acc), 0); diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 48f19b3..14bd87e 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -17,6 +17,7 @@ limitations under the License. import TinyQueue from "tinyqueue"; import { TileDescriptor } from "./TileDescriptor"; +import { count, findLastIndex } from "../array-utils"; /** * A 1×1 cell in a grid which belongs to a tile. @@ -105,17 +106,6 @@ export function getPaths(dest: number, g: Grid): (number | null)[] { return edges as (number | null)[]; } -function findLastIndex( - array: T[], - predicate: (item: T) => boolean -): number | null { - for (let i = array.length - 1; i >= 0; i--) { - if (predicate(array[i])) return i; - } - - return null; -} - const findLast1By1Index = (g: Grid): number | null => findLastIndex(g.cells, (c) => c?.rows === 1 && c?.columns === 1); @@ -209,11 +199,117 @@ function getNextGap(g: Grid): number | null { return null; } +/** + * Gets the index of the origin of the tile to which the given cell belongs. + */ +function getOrigin(g: Grid, index: number): number { + const initialColumn = column(index, g); + + for ( + let i = index; + i >= 0; + i = column(i, g) === 0 ? i - g.columns + initialColumn : i - 1 + ) { + const cell = g.cells[i]; + if ( + cell !== undefined && + cell.origin && + inArea(index, i, areaEnd(i, cell.columns, cell.rows, g), g) + ) + return i; + } + + throw new Error("Tile is broken"); +} + +/** + * Moves the tile at index "from" over to index "to", displacing other tiles + * along the way. + * Precondition: the destination area must consist of only 1×1 tiles. + */ +function moveTile(g: Grid, from: number, to: number) { + const tile = g.cells[from]!; + const fromEnd = areaEnd(from, tile.columns, tile.rows, g); + const toEnd = areaEnd(to, tile.columns, tile.rows, g); + + const displacedTiles: Cell[] = []; + forEachCellInArea(to, toEnd, g, (c, i) => { + if (c !== undefined && !inArea(i, from, fromEnd, g)) displacedTiles.push(c); + }); + + const movingCells: Cell[] = []; + forEachCellInArea(from, fromEnd, g, (c, i) => { + movingCells.push(c!); + g.cells[i] = undefined; + }); + + forEachCellInArea( + to, + toEnd, + g, + (_c, i) => (g.cells[i] = movingCells.shift()) + ); + forEachCellInArea( + from, + fromEnd, + g, + (_c, i) => (g.cells[i] ??= displacedTiles.shift()) + ); +} + +/** + * Attempts to push a tile upwards by one row, displacing 1×1 tiles and shifting + * enlarged tiles around when necessary. + * @returns Whether the tile was actually pushed + */ +function pushTileUp(g: Grid, from: number): boolean { + const tile = g.cells[from]!; + + // TODO: pushing large tiles sideways might be more successful in some + // situations + const cellsAboveAreDisplacable = + from - g.columns >= 0 && + allCellsInArea( + from - g.columns, + from - g.columns + tile.columns - 1, + g, + (c, i) => + c === undefined || + (c.columns === 1 && c.rows === 1) || + pushTileUp(g, getOrigin(g, i)) + ); + + if (cellsAboveAreDisplacable) { + moveTile(g, from, from - g.columns); + return true; + } else { + return false; + } +} + /** * Backfill any gaps in the grid. */ export function fillGaps(g: Grid): Grid { const result: Grid = { ...g, cells: [...g.cells] }; + + // This will hopefully be the size of the grid after we're done here, assuming + // that we can pack the large tiles tightly enough + const idealLength = count(result.cells, (c) => c !== undefined); + + // Step 1: Take any large tiles hanging off the bottom of the grid, and push + // them upwards + for (let i = result.cells.length - 1; i >= idealLength; i--) { + const cell = result.cells[i]; + if (cell !== undefined && (cell.columns > 1 || cell.rows > 1)) { + const originIndex = + i - (cell.columns - 1) - result.columns * (cell.rows - 1); + // If it's not possible to pack the large tiles any tighter, give up + if (!pushTileUp(result, originIndex)) break; + } + } + + // Step 2: Fill all 1×1 gaps let gap = getNextGap(result); if (gap !== null) { @@ -263,9 +359,6 @@ export function fillGaps(g: Grid): Grid { } while (gap !== null); } - // TODO: If there are any large tiles on the last row, shuffle them back - // upwards into a full row - // Shrink the array to remove trailing gaps const finalLength = (findLastIndex(result.cells, (c) => c !== undefined) ?? -1) + 1; @@ -381,13 +474,18 @@ export function cycleTileSize(tileId: string, g: Grid): Grid { // Copy tiles from the original grid to the new one, with the new rows // inserted at the target location - g.cells.forEach((c, src) => { + g.cells.forEach((c, from) => { if (c?.origin && c.item.id !== tileId) { const offset = - row(src, g) > toRow + candidateHeight - 1 ? g.columns * newRows : 0; - forEachCellInArea(src, areaEnd(src, c.columns, c.rows, g), g, (c, i) => { - gappyGrid.cells[i + offset] = c; - }); + row(from, g) > toRow + candidateHeight - 1 ? g.columns * newRows : 0; + forEachCellInArea( + from, + areaEnd(from, c.columns, c.rows, g), + g, + (c, i) => { + gappyGrid.cells[i + offset] = c; + } + ); } }); diff --git a/test/video-grid/model-test.ts b/test/video-grid/model-test.ts index cc7741d..d8cfdd8 100644 --- a/test/video-grid/model-test.ts +++ b/test/video-grid/model-test.ts @@ -169,6 +169,50 @@ dddd iegh` ); +testFillGaps( + "keeps a large tile from hanging off the bottom", + ` +abcd +efgh + +ii +ii`, + ` +abcd +iigh +iief` +); + +testFillGaps( + "pushes a chain of large tiles upwards", + ` +abcd +e fg +hh +hh + ii + ii`, + ` +hhcd +hhfg +aiib +eii` +); + +testFillGaps( + "gives up on pushing large tiles upwards when not possible", + ` +aabb +aabb +cc +cc`, + ` +aabb +aabb +cc +cc` +); + function testCycleTileSize( title: string, tileId: string, From 4f582c6ad746fc19198c4816b6940032ab52a290 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sat, 17 Jun 2023 17:28:54 -0400 Subject: [PATCH 2/8] Don't change tile size when dragging --- src/video-grid/NewVideoGrid.module.css | 5 +-- src/video-grid/NewVideoGrid.tsx | 48 +++++++++++++------- src/video-grid/model.ts | 37 +++++++++++++++- test/video-grid/model-test.ts | 61 ++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 20 deletions(-) diff --git a/src/video-grid/NewVideoGrid.module.css b/src/video-grid/NewVideoGrid.module.css index 75ee707..7e34a2d 100644 --- a/src/video-grid/NewVideoGrid.module.css +++ b/src/video-grid/NewVideoGrid.module.css @@ -18,7 +18,7 @@ limitations under the License. contain: strict; position: relative; flex-grow: 1; - padding: 0 20px; + padding: 0 20px var(--footerHeight); overflow-y: auto; overflow-x: hidden; } @@ -28,7 +28,6 @@ limitations under the License. display: grid; grid-auto-rows: 163px; gap: 8px; - padding-bottom: var(--footerHeight); } .slot { @@ -37,7 +36,7 @@ limitations under the License. @media (min-width: 800px) { .grid { - padding: 0 22px; + padding: 0 22px var(--footerHeight); } .slotGrid { diff --git a/src/video-grid/NewVideoGrid.tsx b/src/video-grid/NewVideoGrid.tsx index e27dea5..80f1314 100644 --- a/src/video-grid/NewVideoGrid.tsx +++ b/src/video-grid/NewVideoGrid.tsx @@ -44,6 +44,7 @@ import { forEachCellInArea, cycleTileSize, appendItems, + tryMoveTile, } from "./model"; import { TileWrapper } from "./TileWrapper"; @@ -280,6 +281,8 @@ export const NewVideoGrid: FC = ({ const animateDraggedTile = (endOfGesture: boolean) => { const { tileId, tileX, tileY, cursorX, cursorY } = dragState.current!; const tile = tiles.find((t) => t.item.id === tileId)!; + const originIndex = grid!.cells.findIndex((c) => c?.item.id === tileId); + const originCell = grid!.cells[originIndex]!; springRef.current .find((c) => (c.item as Tile).item.id === tileId) @@ -310,23 +313,36 @@ export const NewVideoGrid: FC = ({ } ); - const overTile = tiles.find( - (t) => - cursorX >= t.x && - cursorX < t.x + t.width && - cursorY >= t.y && - cursorY < t.y + t.height + const columns = grid!.columns; + const rows = row(grid!.cells.length - 1, grid!) + 1; + + const cursorColumn = Math.floor( + (cursorX / slotGrid!.clientWidth) * columns ); - if (overTile !== undefined && overTile.item.id !== tileId) { - setGrid((g) => ({ - ...g!, - cells: g!.cells.map((c) => { - if (c?.item === overTile.item) return { ...c, item: tile.item }; - if (c?.item === tile.item) return { ...c, item: overTile.item }; - return c; - }), - })); - } + const cursorRow = Math.floor((cursorY / slotGrid!.clientHeight) * rows); + + const cursorColumnOnTile = Math.floor( + ((cursorX - tileX) / tile.width) * originCell.columns + ); + const cursorRowOnTile = Math.floor( + ((cursorY - tileY) / tile.height) * originCell.rows + ); + + const dest = + Math.max( + 0, + Math.min( + columns - originCell.columns, + cursorColumn - cursorColumnOnTile + ) + ) + + grid!.columns * + Math.max( + 0, + Math.min(rows - originCell.rows, cursorRow - cursorRowOnTile) + ); + + if (dest !== originIndex) setGrid((g) => tryMoveTile(g, originIndex, dest)); }; // Callback for useDrag. We could call useDrag here, but the default diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 14bd87e..4a6cc32 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -175,6 +175,8 @@ const areaEnd = ( g: Grid ): number => start + columns - 1 + g.columns * (rows - 1); +const cloneGrid = (g: Grid): Grid => ({ ...g, cells: [...g.cells] }); + /** * Gets the index of the next gap in the grid that should be backfilled by 1×1 * tiles. @@ -257,6 +259,39 @@ function moveTile(g: Grid, from: number, to: number) { ); } +/** + * Moves the tile at index "from" over to index "to", if there is space. + */ +export function tryMoveTile(g: Grid, from: number, to: number): Grid { + const tile = g.cells[from]!; + + if ( + to > 0 && + to < g.cells.length && + column(to, g) <= g.columns - tile.columns + ) { + const fromEnd = areaEnd(from, tile.columns, tile.rows, g); + const toEnd = areaEnd(to, tile.columns, tile.rows, g); + + // The contents of a given cell are 'displaceable' if it's empty, holds a + // 1×1 tile, or is part of the original tile we're trying to reposition + const displaceable = (c: Cell | undefined, i: number): boolean => + c === undefined || + (c.columns === 1 && c.rows === 1) || + inArea(i, from, fromEnd, g); + + if (allCellsInArea(to, toEnd, g, displaceable)) { + // The target space is free; move + const gClone = cloneGrid(g); + moveTile(gClone, from, to); + return gClone; + } + } + + // The target space isn't free; don't move + return g; +} + /** * Attempts to push a tile upwards by one row, displacing 1×1 tiles and shifting * enlarged tiles around when necessary. @@ -291,7 +326,7 @@ function pushTileUp(g: Grid, from: number): boolean { * Backfill any gaps in the grid. */ export function fillGaps(g: Grid): Grid { - const result: Grid = { ...g, cells: [...g.cells] }; + const result = cloneGrid(g); // This will hopefully be the size of the grid after we're done here, assuming // that we can pack the large tiles tightly enough diff --git a/test/video-grid/model-test.ts b/test/video-grid/model-test.ts index d8cfdd8..5765915 100644 --- a/test/video-grid/model-test.ts +++ b/test/video-grid/model-test.ts @@ -22,6 +22,7 @@ import { forEachCellInArea, Grid, row, + tryMoveTile, } from "../../src/video-grid/model"; import { TileDescriptor } from "../../src/video-grid/TileDescriptor"; @@ -325,3 +326,63 @@ def`; ); expect(showGrid(appendItems(newItems, mkGrid(grid1)))).toBe(grid2); }); + +function testTryMoveTile( + title: string, + from: number, + to: number, + input: string, + output: string +): void { + test(`tryMoveTile ${title}`, () => { + expect(showGrid(tryMoveTile(mkGrid(input), from, to))).toBe(output); + }); +} + +testTryMoveTile( + "refuses to move a tile too far to the left", + 1, + -1, + ` +abc`, + ` +abc` +); + +testTryMoveTile( + "refuses to move a tile too far to the right", + 1, + 3, + ` +abc`, + ` +abc` +); + +testTryMoveTile( + "moves a large tile to an unoccupied space", + 3, + 1, + ` +a b +ccd +cce`, + ` +acc +bcc +d e` +); + +testTryMoveTile( + "refuses to move a large tile to an occupied space", + 3, + 1, + ` +abb +ccd +cce`, + ` +abb +ccd +cce` +); From afbcea7b66772f0464b98bc634d5a655493def2b Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sat, 17 Jun 2023 21:33:54 -0400 Subject: [PATCH 3/8] Allow the grid to resize with the window width --- src/video-grid/NewVideoGrid.tsx | 20 ++++++------- src/video-grid/model.ts | 52 +++++++++++++++++++++++++++++++-- test/video-grid/model-test.ts | 50 +++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 13 deletions(-) diff --git a/src/video-grid/NewVideoGrid.tsx b/src/video-grid/NewVideoGrid.tsx index 80f1314..8922f6a 100644 --- a/src/video-grid/NewVideoGrid.tsx +++ b/src/video-grid/NewVideoGrid.tsx @@ -45,6 +45,7 @@ import { cycleTileSize, appendItems, tryMoveTile, + resize, } from "./model"; import { TileWrapper } from "./TileWrapper"; @@ -82,8 +83,11 @@ const useGridState = ( }), }; - // Step 2: Backfill gaps left behind by removed tiles - const grid2 = fillGaps(grid1); + // Step 2: Resize the grid if necessary and backfill gaps left behind by + // removed tiles + // Resizing already takes care of backfilling gaps + const grid2 = + columns !== grid1.columns ? resize(grid1, columns!) : fillGaps(grid1); // Step 3: Add new tiles to the end of the grid const existingItemIds = new Set( @@ -205,14 +209,10 @@ export const NewVideoGrid: FC = ({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [slotGrid, slotGridGeneration, gridBounds]); - const [columns] = useReactiveState( - // Since grid resizing isn't implemented yet, pick a column count on mount - // and stick to it - (prevColumns) => - prevColumns !== undefined && prevColumns !== null - ? prevColumns - : // The grid bounds might not be known yet - gridBounds.width === 0 + const columns = useMemo( + () => + // The grid bounds might not be known yet + gridBounds.width === 0 ? null : Math.max(2, Math.floor(gridBounds.width * 0.0045)), [gridBounds] diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 4a6cc32..6ca352e 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -418,6 +418,11 @@ export function appendItems(items: TileDescriptor[], g: Grid): Grid { }; } +const largeTileDimensions = (g: Grid): [number, number] => [ + Math.min(3, Math.max(2, g.columns - 1)), + 2, +]; + /** * Changes the size of a tile, rearranging the grid to make space. * @param tileId The ID of the tile to modify. @@ -433,9 +438,7 @@ export function cycleTileSize(tileId: string, g: Grid): Grid { // The target dimensions, which toggle between 1×1 and larger than 1×1 const [toWidth, toHeight] = - fromWidth === 1 && fromHeight === 1 - ? [Math.min(3, Math.max(2, g.columns - 1)), 2] - : [1, 1]; + fromWidth === 1 && fromHeight === 1 ? largeTileDimensions(g) : [1, 1]; // If we're expanding the tile, we want to create enough new rows at the // tile's target position such that every new unit of grid area created during @@ -547,3 +550,46 @@ export function cycleTileSize(tileId: string, g: Grid): Grid { // Fill any gaps that remain return fillGaps(gappyGrid); } + +/** + * Resizes the grid to a new column width. + */ +export function resize(g: Grid, columns: number): Grid { + const result: Grid = { columns, cells: [] }; + const [largeColumns, largeRows] = largeTileDimensions(result); + + // Copy each tile from the old grid to the resized one in the same order + + // The next index in the result grid to copy a tile to + let next = 0; + + for (const cell of g.cells) { + if (cell?.origin) { + const [nextColumns, nextRows] = + cell.columns > 1 || cell.rows > 1 ? [largeColumns, largeRows] : [1, 1]; + + // If there isn't enough space left on this row, jump to the next row + if (columns - column(next, result) < nextColumns) + next = columns * (Math.floor(next / columns) + 1); + const nextEnd = areaEnd(next, nextColumns, nextRows, result); + + // Expand the cells array as necessary + if (result.cells.length <= nextEnd) + result.cells.push(...new Array(nextEnd + 1 - result.cells.length)); + + // Copy the tile into place + forEachCellInArea(next, nextEnd, result, (_c, i) => { + result.cells[i] = { + item: cell.item, + origin: i === next, + columns: nextColumns, + rows: nextRows, + }; + }); + + next = nextEnd + 1; + } + } + + return fillGaps(result); +} diff --git a/test/video-grid/model-test.ts b/test/video-grid/model-test.ts index 5765915..dd25e8a 100644 --- a/test/video-grid/model-test.ts +++ b/test/video-grid/model-test.ts @@ -21,6 +21,7 @@ import { fillGaps, forEachCellInArea, Grid, + resize, row, tryMoveTile, } from "../../src/video-grid/model"; @@ -386,3 +387,52 @@ abb ccd cce` ); + +function testResize( + title: string, + columns: number, + input: string, + output: string +): void { + test(`resize ${title}`, () => { + expect(showGrid(resize(mkGrid(input), columns))).toBe(output); + }); +} + +testResize( + "contracts the grid", + 2, + ` +abbb +cbbb +ddde +dddf +gh`, + ` +af +bb +bb +ch +dd +dd +eg` +); + +testResize( + "expands the grid", + 4, + ` +af +bb +bb +ch +dd +dd +eg`, + ` +bbbc +bbbf +addd +hddd +ge` +); From 3e56d0a656312760644501e040600869bdf5f286 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sun, 18 Jun 2023 00:28:08 -0400 Subject: [PATCH 4/8] Make it possible again to drag a tile into the top left corner --- src/video-grid/model.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 6ca352e..b2d7984 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -266,7 +266,7 @@ export function tryMoveTile(g: Grid, from: number, to: number): Grid { const tile = g.cells[from]!; if ( - to > 0 && + to >= 0 && to < g.cells.length && column(to, g) <= g.columns - tile.columns ) { From 391ba5196cbaaa04a84504f3f26b48aea65a9c54 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sun, 18 Jun 2023 00:47:37 -0400 Subject: [PATCH 5/8] Make screenshares appear near the presenter's tile and be larger --- src/room/InCallView.tsx | 4 + src/video-grid/NewVideoGrid.tsx | 4 +- src/video-grid/TileDescriptor.tsx | 2 + src/video-grid/model.ts | 151 ++++++++++++++++++++++-------- test/video-grid/model-test.ts | 56 ++++++++--- 5 files changed, 166 insertions(+), 51 deletions(-) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index bf38693..6c2a709 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -216,6 +216,7 @@ export function InCallView({ focused: screenshareFeeds.length === 0 && callFeed === activeSpeaker, isLocal: member.userId === localUserId && deviceId === localDeviceId, presenter, + largeBaseSize: false, connectionState, }); } @@ -228,6 +229,7 @@ export function InCallView({ // Add the screenshares too for (const screenshareFeed of screenshareFeeds) { const member = screenshareFeed.getMember()!; + const deviceId = screenshareFeed.deviceId!; const connectionState = participants .get(member) ?.get(screenshareFeed.deviceId!)?.connectionState; @@ -242,6 +244,8 @@ export function InCallView({ focused: true, isLocal: screenshareFeed.isLocal(), presenter: false, + largeBaseSize: true, + placeNear: `${member.userId} ${deviceId}`, connectionState, }); } diff --git a/src/video-grid/NewVideoGrid.tsx b/src/video-grid/NewVideoGrid.tsx index 8922f6a..c255959 100644 --- a/src/video-grid/NewVideoGrid.tsx +++ b/src/video-grid/NewVideoGrid.tsx @@ -43,7 +43,7 @@ import { fillGaps, forEachCellInArea, cycleTileSize, - appendItems, + addItems, tryMoveTile, resize, } from "./model"; @@ -94,7 +94,7 @@ const useGridState = ( grid2.cells.filter((c) => c !== undefined).map((c) => c!.item.id) ); const newItems = items.filter((i) => !existingItemIds.has(i.id)); - const grid3 = appendItems(newItems, grid2); + const grid3 = addItems(newItems, grid2); return { ...grid3, generation: prevGrid.generation + 1 }; }, diff --git a/src/video-grid/TileDescriptor.tsx b/src/video-grid/TileDescriptor.tsx index cc37528..c215bc6 100644 --- a/src/video-grid/TileDescriptor.tsx +++ b/src/video-grid/TileDescriptor.tsx @@ -28,5 +28,7 @@ export interface TileDescriptor { presenter: boolean; callFeed?: CallFeed; isLocal?: boolean; + largeBaseSize: boolean; + placeNear?: string; connectionState: ConnectionState; } diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index b2d7984..5150ace 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -403,19 +403,94 @@ export function fillGaps(g: Grid): Grid { return result; } -export function appendItems(items: TileDescriptor[], g: Grid): Grid { - return { - ...g, - cells: [ - ...g.cells, - ...items.map((i) => ({ - item: i, - origin: true, - columns: 1, - rows: 1, - })), - ], +function createRows(g: Grid, count: number, atRow: number): Grid { + const result = { + columns: g.columns, + cells: new Array(g.cells.length + g.columns * count), }; + const offsetAfterNewRows = g.columns * count; + + // Copy tiles from the original grid to the new one, with the new rows + // inserted at the target location + g.cells.forEach((c, from) => { + if (c?.origin) { + const offset = row(from, g) >= atRow ? offsetAfterNewRows : 0; + forEachCellInArea( + from, + areaEnd(from, c.columns, c.rows, g), + g, + (c, i) => { + result.cells[i + offset] = c; + } + ); + } + }); + + return result; +} + +/** + * Adds a set of new items into the grid. + */ +export function addItems(items: TileDescriptor[], g: Grid): Grid { + let result = cloneGrid(g); + + for (const item of items) { + const cell = { + item, + origin: true, + columns: 1, + rows: 1, + }; + + let placeAt: number; + let hasGaps: boolean; + + if (item.placeNear === undefined) { + // This item has no special placement requests, so let's put it + // uneventfully at the end of the grid + placeAt = result.cells.length; + hasGaps = false; + } else { + // This item wants to be placed near another; let's put it on a row + // directly below the related tile + const placeNear = result.cells.findIndex( + (c) => c?.item.id === item.placeNear + ); + if (placeNear === -1) { + // Can't find the related tile, so let's give up and place it at the end + placeAt = result.cells.length; + hasGaps = false; + } else { + const placeNearCell = result.cells[placeNear]!; + const placeNearEnd = areaEnd( + placeNear, + placeNearCell.columns, + placeNearCell.rows, + result + ); + + result = createRows(result, 1, row(placeNearEnd, result) + 1); + placeAt = + placeNear + + Math.floor(placeNearCell.columns / 2) + + result.columns * placeNearCell.rows; + hasGaps = true; + } + } + + result.cells[placeAt] = cell; + + if (item.largeBaseSize) { + // Cycle the tile size once to set up the tile with its larger base size + // This also fills any gaps in the grid, hence no extra call to fillGaps + result = cycleTileSize(item.id, result); + } else if (hasGaps) { + result = fillGaps(result); + } + } + + return result; } const largeTileDimensions = (g: Grid): [number, number] => [ @@ -423,6 +498,9 @@ const largeTileDimensions = (g: Grid): [number, number] => [ 2, ]; +const extraLargeTileDimensions = (g: Grid): [number, number] => + g.columns > 3 ? [4, 3] : [g.columns, 2]; + /** * Changes the size of a tile, rearranging the grid to make space. * @param tileId The ID of the tile to modify. @@ -432,13 +510,19 @@ const largeTileDimensions = (g: Grid): [number, number] => [ export function cycleTileSize(tileId: string, g: Grid): Grid { const from = g.cells.findIndex((c) => c?.item.id === tileId); if (from === -1) return g; // Tile removed, no change - const fromWidth = g.cells[from]!.columns; - const fromHeight = g.cells[from]!.rows; + const fromCell = g.cells[from]!; + const fromWidth = fromCell.columns; + const fromHeight = fromCell.rows; const fromEnd = areaEnd(from, fromWidth, fromHeight, g); - // The target dimensions, which toggle between 1×1 and larger than 1×1 + const [baseDimensions, enlargedDimensions] = fromCell.item.largeBaseSize + ? [largeTileDimensions(g), extraLargeTileDimensions(g)] + : [[1, 1], largeTileDimensions(g)]; + // The target dimensions, which toggle between the base and enlarged sizes const [toWidth, toHeight] = - fromWidth === 1 && fromHeight === 1 ? largeTileDimensions(g) : [1, 1]; + fromWidth === baseDimensions[0] && fromHeight === baseDimensions[1] + ? enlargedDimensions + : baseDimensions; // If we're expanding the tile, we want to create enough new rows at the // tile's target position such that every new unit of grid area created during @@ -450,12 +534,6 @@ export function cycleTileSize(tileId: string, g: Grid): Grid { Math.ceil((toWidth * toHeight - fromWidth * fromHeight) / g.columns) ); - // This is the grid with the new rows added - const gappyGrid: Grid = { - ...g, - cells: new Array(g.cells.length + newRows * g.columns), - }; - // The next task is to scan for a spot to place the modified tile. Since we // might be creating new rows at the target position, this spot can be shorter // than the target height. @@ -510,22 +588,19 @@ export function cycleTileSize(tileId: string, g: Grid): Grid { const toRow = row(to, g); - // Copy tiles from the original grid to the new one, with the new rows - // inserted at the target location - g.cells.forEach((c, from) => { - if (c?.origin && c.item.id !== tileId) { - const offset = - row(from, g) > toRow + candidateHeight - 1 ? g.columns * newRows : 0; - forEachCellInArea( - from, - areaEnd(from, c.columns, c.rows, g), - g, - (c, i) => { - gappyGrid.cells[i + offset] = c; - } - ); - } - }); + // This is the grid with the new rows added + const gappyGrid = createRows(g, newRows, toRow + candidateHeight); + + // Remove the original tile + const fromInGappyGrid = + from + (row(from, g) >= toRow + candidateHeight ? g.columns * newRows : 0); + const fromEndInGappyGrid = fromInGappyGrid - from + fromEnd; + forEachCellInArea( + fromInGappyGrid, + fromEndInGappyGrid, + gappyGrid, + (_c, i) => (gappyGrid.cells[i] = undefined) + ); // Place the tile in its target position, making a note of the tiles being // overwritten diff --git a/test/video-grid/model-test.ts b/test/video-grid/model-test.ts index dd25e8a..f7d0330 100644 --- a/test/video-grid/model-test.ts +++ b/test/video-grid/model-test.ts @@ -15,7 +15,7 @@ limitations under the License. */ import { - appendItems, + addItems, column, cycleTileSize, fillGaps, @@ -313,20 +313,54 @@ dde ddf` ); -test("appendItems appends 1×1 tiles", () => { - const grid1 = ` +function testAddItems( + title: string, + items: TileDescriptor[], + input: string, + output: string +): void { + test(`addItems ${title}`, () => { + expect(showGrid(addItems(items, mkGrid(input)))).toBe(output); + }); +} + +testAddItems( + "appends 1×1 tiles", + ["e", "f"].map((i) => ({ id: i } as unknown as TileDescriptor)), + ` aab aac -d`; - const grid2 = ` +d`, + ` aab aac -def`; - const newItems = ["e", "f"].map( - (i) => ({ id: i } as unknown as TileDescriptor) - ); - expect(showGrid(appendItems(newItems, mkGrid(grid1)))).toBe(grid2); -}); +def` +); + +testAddItems( + "places one tile near another on request", + [{ id: "g", placeNear: "b" } as unknown as TileDescriptor], + ` +abc +def`, + ` +abc +gfe +d` +); + +testAddItems( + "places items with a large base size", + [{ id: "g", largeBaseSize: true } as unknown as TileDescriptor], + ` +abc +def`, + ` +abc +ggf +gge +d` +); function testTryMoveTile( title: string, From 4e5a75074acd15587c305dd8ca8ff655031d38a6 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sun, 18 Jun 2023 00:58:54 -0400 Subject: [PATCH 6/8] Fix tiles not collapsing toward their center --- src/video-grid/model.ts | 4 ++-- test/video-grid/model-test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 5150ace..15973c4 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -543,8 +543,8 @@ export function cycleTileSize(tileId: string, g: Grid): Grid { // To make the tile appear to expand outwards from its center, we're actually // scanning for locations to put the *center* of the tile. These numbers are // the offsets between the tile's origin and its center. - const scanColumnOffset = Math.floor((toWidth - 1) / 2); - const scanRowOffset = Math.floor((toHeight - 1) / 2); + const scanColumnOffset = Math.floor((toWidth - fromWidth) / 2); + const scanRowOffset = Math.floor((toHeight - fromHeight) / 2); const nextScanLocations = new Set([from]); const rows = row(g.cells.length - 1, g) + 1; diff --git a/test/video-grid/model-test.ts b/test/video-grid/model-test.ts index f7d0330..479f9cb 100644 --- a/test/video-grid/model-test.ts +++ b/test/video-grid/model-test.ts @@ -273,9 +273,9 @@ dbbe fghi jk`, ` -abhc -djge -fik` +akbc +djhe +fig` ); testCycleTileSize( From ddeb36db476fe27a9a3e469b00244037f9a36170 Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sun, 18 Jun 2023 11:32:21 -0400 Subject: [PATCH 7/8] Promote speakers to the first page of the grid --- src/room/InCallView.tsx | 2 ++ src/video-grid/NewVideoGrid.tsx | 4 ++++ src/video-grid/TileDescriptor.tsx | 1 + src/video-grid/model.ts | 28 ++++++++++++++++++++++++++++ test/video-grid/model-test.ts | 2 +- 5 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 6c2a709..2a62f9d 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -216,6 +216,7 @@ export function InCallView({ focused: screenshareFeeds.length === 0 && callFeed === activeSpeaker, isLocal: member.userId === localUserId && deviceId === localDeviceId, presenter, + isSpeaker: callFeed === activeSpeaker, largeBaseSize: false, connectionState, }); @@ -244,6 +245,7 @@ export function InCallView({ focused: true, isLocal: screenshareFeed.isLocal(), presenter: false, + isSpeaker: screenshareFeed === activeSpeaker, largeBaseSize: true, placeNear: `${member.userId} ${deviceId}`, connectionState, diff --git a/src/video-grid/NewVideoGrid.tsx b/src/video-grid/NewVideoGrid.tsx index c255959..711e70e 100644 --- a/src/video-grid/NewVideoGrid.tsx +++ b/src/video-grid/NewVideoGrid.tsx @@ -46,6 +46,7 @@ import { addItems, tryMoveTile, resize, + promoteSpeakers, } from "./model"; import { TileWrapper } from "./TileWrapper"; @@ -96,6 +97,9 @@ const useGridState = ( const newItems = items.filter((i) => !existingItemIds.has(i.id)); const grid3 = addItems(newItems, grid2); + // Step 4: Promote speakers to the top + promoteSpeakers(grid3); + return { ...grid3, generation: prevGrid.generation + 1 }; }, [columns, items] diff --git a/src/video-grid/TileDescriptor.tsx b/src/video-grid/TileDescriptor.tsx index c215bc6..771c45a 100644 --- a/src/video-grid/TileDescriptor.tsx +++ b/src/video-grid/TileDescriptor.tsx @@ -26,6 +26,7 @@ export interface TileDescriptor { member: RoomMember; focused: boolean; presenter: boolean; + isSpeaker: boolean; callFeed?: CallFeed; isLocal?: boolean; largeBaseSize: boolean; diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 15973c4..767ae19 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -668,3 +668,31 @@ export function resize(g: Grid, columns: number): Grid { return fillGaps(result); } + +/** + * Promotes speakers to the first page of the grid. + */ +export function promoteSpeakers(g: Grid) { + // This is all a bit of a hack right now, because we don't know if the designs + // will stick with this approach in the long run + // We assume that 4 rows are probably about 1 page + const firstPageEnd = g.columns * 4; + + for (let from = firstPageEnd; from < g.cells.length; from++) { + const fromCell = g.cells[from]; + // Don't bother trying to promote enlarged tiles + if ( + fromCell?.item.isSpeaker && + fromCell.columns === 1 && + fromCell.rows === 1 + ) { + // Promote this tile by making 10 attempts to place it on the first page + for (let j = 0; j < 10; j++) { + const to = Math.floor(Math.random() * firstPageEnd); + const toCell = g.cells[to]; + if (toCell === undefined || (toCell.columns === 1 && toCell.rows === 1)) + moveTile(g, from, to); + } + } + } +} diff --git a/test/video-grid/model-test.ts b/test/video-grid/model-test.ts index 479f9cb..5b5897f 100644 --- a/test/video-grid/model-test.ts +++ b/test/video-grid/model-test.ts @@ -33,7 +33,7 @@ import { TileDescriptor } from "../../src/video-grid/TileDescriptor"; function mkGrid(spec: string): Grid { const secondNewline = spec.indexOf("\n", 1); const columns = secondNewline === -1 ? spec.length : secondNewline - 1; - const cells = spec.match(/[a-z ]/g) ?? []; + const cells = spec.match(/[a-z ]/g) ?? ([] as string[]); const areas = new Set(cells); areas.delete(" "); // Space represents an empty cell, not an area const grid: Grid = { columns, cells: new Array(cells.length) }; From cd7ab00d8084c349316fb60d1ff676b11eb81aab Mon Sep 17 00:00:00 2001 From: Robin Townsend Date: Sun, 18 Jun 2023 11:45:01 -0400 Subject: [PATCH 8/8] Don't try to promote the same speaker multiple times --- src/video-grid/model.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/video-grid/model.ts b/src/video-grid/model.ts index 767ae19..646e534 100644 --- a/src/video-grid/model.ts +++ b/src/video-grid/model.ts @@ -690,8 +690,13 @@ export function promoteSpeakers(g: Grid) { for (let j = 0; j < 10; j++) { const to = Math.floor(Math.random() * firstPageEnd); const toCell = g.cells[to]; - if (toCell === undefined || (toCell.columns === 1 && toCell.rows === 1)) + if ( + toCell === undefined || + (toCell.columns === 1 && toCell.rows === 1) + ) { moveTile(g, from, to); + break; + } } } }