Previously we had a ViewModel class which was responsible for little more than creating an ObservableScope. However, since this ObservableScope would be created implicitly upon view model construction, it became a tad bit harder for callers to remember to eventually end the scope (as you wouldn't just have to remember to end ObservableScopes, but also to destroy ViewModels). Requiring the scope to be specified explicitly by the caller also makes it possible for the caller to reuse the scope for other purposes, reducing the number of scopes mentally in flight that need tending to, and for all state holders (not just view models) to be handled uniformly by helper functions such as generateKeyed$.
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.
* 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 <hughns@element.io>
* Enable @typescript-eslint/consistent-type-imports lint rule
This is to help ensure that we get proper vite/rollup lazy loading by not `import`ing more than we need to.
Revert "Enable @typescript-eslint/consistent-type-imports lint rule"
This reverts commit ba385fa00b7e410cc508fd5fb9fe972233ae114f.
Enable @typescript-eslint/consistent-type-imports lint rule
This is to help ensure that we get proper vite/rollup lazy loading by not `import`ing more than we need to.
.
* Format
* Keep tiles in a stable order
This introduces a new layer of abstraction on top of MediaViewModel: TileViewModel, which gives us a place to store data relating to tiles rather than their media, and also generally makes it easier to reason about tiles as they move about the call layout. I have created a class called TileStore to keep track of these tiles.
This allows us to swap out the media shown on a tile as the spotlight speaker changes, and avoid moving tiles around unless they really need to jump between the visible/invisible regions of the layout.
* Don't throttle spotlight updates
Since we now assume that the spotlight and grid will be in sync (i.e. an active speaker in one will behave as an active speaker in the other), we don't want the spotlight to ever lag behind due to throttling. If this causes usability issues we should maybe look into making LiveKit's 'speaking' indicators less erratic first.
* Make layout shifts due to a change in speaker less surprising
Although we try now to avoid layout shifts due to the spotlight speaker changing wherever possible, a spotlight speaker coming from off screen can still trigger one. Let's shift the layout a bit more gracefully in this case.
* Improve the tile ordering tests
* Maximize the spotlight tile in portrait layout
* Tell tiles whether they're actually visible in a more timely manner
* Fix test
* Fix speaking indicators logic
* Improve readability of marbles
* Fix test case
---------
Co-authored-by: Hugh Nimmo-Smith <hughns@element.io>
