From b834d8f679654e8cefa27fa2cd7b2dad02a6f404 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 11 Dec 2024 05:23:42 -0500 Subject: [PATCH] Add some quick-and-dirty debug info for TileStore (#2887) * Add some quick-and-dirty debug info for TileStore I'm still in need of more detailed data in order to understand why big layout shifts happen in large calls. This adds a developer option to enable logging and a visual indicator for the state of the TileStore. The indicator should be useful for matching up the behavior I'm seeing in my recordings with the right timestamps. * Reduce performance impact of checking for whether debug mode is enabled --------- Co-authored-by: Hugh Nimmo-Smith --- locales/en/app.json | 1 + src/room/InCallView.tsx | 10 ++++++ src/settings/DeveloperSettingsTab.tsx | 15 +++++++++ src/settings/settings.ts | 2 ++ src/state/CallViewModel.ts | 23 +++++++++++--- src/state/TileStore.ts | 46 +++++++++++++++++++++++++-- 6 files changed, 91 insertions(+), 6 deletions(-) diff --git a/locales/en/app.json b/locales/en/app.json index bcfc0945..0d2b5f15 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -70,6 +70,7 @@ }, "developer_mode": { "crypto_version": "Crypto version: {{version}}", + "debug_tile_layout_label": "Debug tile layout", "device_id": "Device ID: {{id}}", "duplicate_tiles_label": "Number of additional tile copies per participant", "hostname": "Hostname: {{hostname}}", diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 6ee9f184..976e4e94 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -88,6 +88,10 @@ import { ReactionsAudioRenderer } from "./ReactionAudioRenderer"; import { useSwitchCamera } from "./useSwitchCamera"; import { ReactionsOverlay } from "./ReactionsOverlay"; import { CallEventAudioRenderer } from "./CallEventAudioRenderer"; +import { + debugTileLayout as debugTileLayoutSetting, + useSetting, +} from "../settings/settings"; const canScreenshare = "getDisplayMedia" in (navigator.mediaDevices ?? {}); @@ -223,6 +227,8 @@ export const InCallView: FC = ({ const windowMode = useObservableEagerState(vm.windowMode); const layout = useObservableEagerState(vm.layout); + const tileStoreGeneration = useObservableEagerState(vm.tileStoreGeneration); + const [debugTileLayout] = useSetting(debugTileLayoutSetting); const gridMode = useObservableEagerState(vm.gridMode); const showHeader = useObservableEagerState(vm.showHeader); const showFooter = useObservableEagerState(vm.showFooter); @@ -585,6 +591,10 @@ export const InCallView: FC = ({ height={11} aria-label={import.meta.env.VITE_PRODUCT_NAME || "Element Call"} /> + {/* Don't mind this odd placement, it's just a little debug label */} + {debugTileLayout + ? `Tiles generation: ${tileStoreGeneration}` + : undefined} )} {showControls &&
{buttons}
} diff --git a/src/settings/DeveloperSettingsTab.tsx b/src/settings/DeveloperSettingsTab.tsx index 0a33d52f..209bc41e 100644 --- a/src/settings/DeveloperSettingsTab.tsx +++ b/src/settings/DeveloperSettingsTab.tsx @@ -12,6 +12,7 @@ import { FieldRow, InputField } from "../input/Input"; import { useSetting, duplicateTiles as duplicateTilesSetting, + debugTileLayout as debugTileLayoutSetting, } from "./settings"; import type { MatrixClient } from "matrix-js-sdk/src/client"; @@ -22,6 +23,9 @@ interface Props { export const DeveloperSettingsTab: FC = ({ client }) => { const { t } = useTranslation(); const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); + const [debugTileLayout, setDebugTileLayout] = useSetting( + debugTileLayoutSetting, + ); return ( <> @@ -70,6 +74,17 @@ export const DeveloperSettingsTab: FC = ({ client }) => { )} /> + + ): void => + setDebugTileLayout(event.target.checked) + } + /> + ); }; diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 8d32adbe..63c38a57 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -75,6 +75,8 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); +export const debugTileLayout = new Setting("debug-tile-layout", false); + export const audioInput = new Setting( "audio-input", undefined, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index a2a10884..72712aa9 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -891,10 +891,9 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - /** - * The layout of tiles in the call interface. - */ - public readonly layout: Observable = this.layoutMedia.pipe( + public readonly layoutInternals: Observable< + LayoutScanState & { layout: Layout } + > = this.layoutMedia.pipe( // Each layout will produce a set of tiles, and these tiles have an // observable indicating whether they're visible. We loop this information // back into the layout process by using switchScan. @@ -949,10 +948,26 @@ export class CallViewModel extends ViewModel { visibleTiles: new Set(), }, ), + this.scope.state(), + ); + + /** + * The layout of tiles in the call interface. + */ + public readonly layout: Observable = this.layoutInternals.pipe( map(({ layout }) => layout), this.scope.state(), ); + /** + * The current generation of the tile store, exposed for debugging purposes. + */ + public readonly tileStoreGeneration: Observable = + this.layoutInternals.pipe( + map(({ tiles }) => tiles.generation), + this.scope.state(), + ); + public showSpotlightIndicators: Observable = this.layout.pipe( map((l) => l.type !== "grid"), this.scope.state(), diff --git a/src/state/TileStore.ts b/src/state/TileStore.ts index ab918cb0..2464d9eb 100644 --- a/src/state/TileStore.ts +++ b/src/state/TileStore.ts @@ -6,10 +6,19 @@ Please see LICENSE in the repository root for full details. */ import { BehaviorSubject } from "rxjs"; +import { logger } from "matrix-js-sdk/src/logger"; import { type MediaViewModel, type UserMediaViewModel } from "./MediaViewModel"; import { GridTileViewModel, SpotlightTileViewModel } from "./TileViewModel"; import { fillGaps } from "../utils/iter"; +import { debugTileLayout } from "../settings/settings"; + +function debugEntries(entries: GridTileData[]): string[] { + return entries.map((e) => e.media.member?.rawDisplayName ?? "[👻]"); +} + +let DEBUG_ENABLED = false; +debugTileLayout.value.subscribe((value) => (DEBUG_ENABLED = value)); class SpotlightTileData { private readonly media_: BehaviorSubject; @@ -69,6 +78,10 @@ export class TileStore { private constructor( private readonly spotlight: SpotlightTileData | null, private readonly grid: GridTileData[], + /** + * A number incremented on each update, just for debugging purposes. + */ + public readonly generation: number, ) {} public readonly spotlightTile = this.spotlight?.vm; @@ -81,7 +94,7 @@ export class TileStore { * Creates an an empty collection of tiles. */ public static empty(): TileStore { - return new TileStore(null, []); + return new TileStore(null, [], 0); } /** @@ -92,8 +105,9 @@ export class TileStore { return new TileStoreBuilder( this.spotlight, this.grid, - (spotlight, grid) => new TileStore(spotlight, grid), + (spotlight, grid) => new TileStore(spotlight, grid, this.generation + 1), visibleTiles, + this.generation, ); } } @@ -133,6 +147,10 @@ export class TileStoreBuilder { grid: GridTileData[], ) => TileStore, private readonly visibleTiles: Set, + /** + * A number incremented on each update, just for debugging purposes. + */ + private readonly generation: number, ) {} /** @@ -140,6 +158,11 @@ export class TileStoreBuilder { * will be no spotlight tile. */ public registerSpotlight(media: MediaViewModel[], maximised: boolean): void { + if (DEBUG_ENABLED) + logger.debug( + `[TileStore, ${this.generation}] register spotlight: ${media.map((m) => m.member?.rawDisplayName ?? "[👻]")}`, + ); + if (this.spotlight !== null) throw new Error("Spotlight already set"); if (this.numGridEntries > 0) throw new Error("Spotlight must be registered before grid tiles"); @@ -159,6 +182,11 @@ export class TileStoreBuilder { * media, then that media will have no grid tile. */ public registerGridTile(media: UserMediaViewModel): void { + if (DEBUG_ENABLED) + logger.debug( + `[TileStore, ${this.generation}] register grid tile: ${media.member?.rawDisplayName ?? "[👻]"}`, + ); + if (this.spotlight !== null) { // We actually *don't* want spotlight speakers to appear in both the // spotlight and the grid, so they're filtered out here @@ -246,6 +274,20 @@ export class TileStoreBuilder { ...this.invisibleGridEntries, ]), ]; + if (DEBUG_ENABLED) { + logger.debug( + `[TileStore, ${this.generation}] stationary: ${debugEntries(this.stationaryGridEntries)}`, + ); + logger.debug( + `[TileStore, ${this.generation}] visible: ${debugEntries(this.visibleGridEntries)}`, + ); + logger.debug( + `[TileStore, ${this.generation}] invisible: ${debugEntries(this.invisibleGridEntries)}`, + ); + logger.debug( + `[TileStore, ${this.generation}] result: ${debugEntries(grid)}`, + ); + } // Destroy unused tiles if (this.spotlight === null && this.prevSpotlight !== null)