PR-C1: adaptive video grid sizing (foundation) #2
Loading…
Reference in a new issue
No description provided.
Delete branch "flotilla-sph-adaptive-grid"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Foundation for the adaptive tiling rework. Behavior-preserving grid sizing.
computeGridSizehelper insrc/app/call.ts: given tile count + container dimensions, returns rows/cols maximizing tile area with ~16:9 cells (smooth 1–12+ tiles).grid-template-columns/rowson the multi-grid (no-spotlight) path inVideoCallContent.svelte, replacing the fixed>2 tiles / grid-cols-2heuristic.bind:clientWidth/clientHeight.Scope: multi-grid path only. Spotlight strip, screen-share selection, and device breakpoints are C2/C3/C4.
Bead: flotilla-sph. Tester verified via logic simulation (1/2/3/4/6/12 tiles, desktop + mobile aspects); lint/format pass, check shows only pre-existing welshman errors.
@hudson I want to take a different approach here, what's here doesn't seem to make the best use of space and it ends up with very horizontally stretched video at some points.
I want try try what jitsi meet does. Their app is open source, maybe you can read their code. But it seems like what they are doing is allowing video tiles to size between 16:9 and 9:16 depending on the layout. They do a good job filling whitespace by even using different sized tiles in different rows. For example if there are 3 participants the first row has two square tiles and the bottom row has a single centered 16:9 tile. Take a crack at that.
Replaces the uniform grid-sizing helper (computeGridSize -> rows/cols) with a Jitsi-inspired mixed-size layout algorithm (computeTileLayout). Key changes: - Tiles flex between 9:16 and 16:9 aspect ratio per row, matched to container dimensions - Rows can have different tile counts (e.g. [2,1] for 3 participants, [3,2] for 5, [4,3] for 7) - Last row with fewer tiles gets a wider (more landscape) aspect ratio while earlier rows stay more square-ish - CSS grid replaced with flex rows; each tile's aspect-ratio set via inline style (no more hardcoded aspect-video) - Container dimensions bound via bind:clientWidth/clientHeight - Dead {:else} branch (always-true useMultiGrid) removed - Overflow penalty, whitespace penalty, and balance penalty ensure reasonable layouts at all tile counts Per @matt's PR review feedback: read jitsi-meet source (react/features/video-layout/functions.web.ts, react/features/filmstrip/functions.web.ts) for the tile sizing approach.Bug 1 — chicken-and-egg gating: '{:else if useMultiGrid && tileLayout}' prevented the measuring container from ever mounting, because tileLayout is undefined while gridWidth/gridHeight are 0, and the bind:clientWidth/ clientHeight are on the same div. Fix: always mount the measuring container when useMultiGrid, gate only the rows with {#if tileLayout}. Bug 2 — collapsed tile height: 'default' tile class lost aspect-video but gained no replacement for intrinsic height. All children are absolutely positioned (VideoCallTile 'absolute inset-0', profile fallback, badges, label, pin button), so the div collapses to 0 height. Fix: add 'h-full' to the default tile class so it fills the aspect-ratio wrapper.1. Array-comparison bug (CRITICAL): 'key < bestKey' coerces arrays to comma-joined strings, so [1668,...] < [23,...] is TRUE ('1'<'2'). Replaced with explicit numeric lexicographic compare (compareKeys). Root cause of @matt's report: n=2 @1920x500 picked 1x2 stacked with 1668px overflow instead of 2x1 side-by-side fitting nearly exactly. 2. <MIN branch bug (CRITICAL): fillAspect < 3/4 computed tileW = rowH * TILE_ASPECT_MIN which ALWAYS exceeds widthLimit (the column width), producing tiles wider than the container. Fix: tileW = widthLimit; tileH = widthLimit / TILE_ASPECT_MIN (letterbox vertically, fits). 3. Missing horizontal overflow scoring: added hOverflow to the key (after vOverflow) as belt-and-braces guard.There is still a major bug with this. When enabling the second video the tile for the second video is rendered half off screen. We should only require scrolling when the tiles reach some unacceptable minimum size. Somewhere around 6 or 8 tiles on a standard portrait sized phone? Try again.
@ -34,0 +37,4 @@const TILE_GAP = 8/*** Uniform-size tile grid: all tiles are identical px dimensions, arrangedIt's not really a uniform grid is it? It's a grid where the last row is centered. Uniform grid makes me think of a screen door.
@ -34,0 +69,4 @@*/export const computeUniformGrid = (tileCount: number,cw: number,use real words here, not abbreviations
@ -34,0 +102,4 @@tileH = rowH}const bboxW = cols * tileW + (cols - 1) * TILE_GAPprefer full words in these vars too: boundingBoxWidth, verticalOverflow, etc.
@ -34,0 +115,4 @@-Math.round(tileW * tileH),]if (!bestKey || compareKeys(key, bestKey) < 0) {I don't like that we are calling these "keys". Can we come up with a more semantically meaningful name?
Edited and opened manually at https://gitea.coracle.social/coracle/flotilla/pulls/355
Pull request closed