Merge pull request #3882 from element-hq/local-screen-share

Fix local screen share not appearing in one-on-one calls

src/initializer.test.ts

@@ -18,7 +18,7 @@ import {
 
 import { mockConfig } from "./utils/test";
 
-const sentryInitSpy = vi.fn();
+const sentryInitSpy = vi.hoisted(() => vi.fn());
 
 // Place the mock after the spy is defined
 vi.mock("@sentry/react", () => ({

src/state/CallViewModel/CallViewModel.test.ts

@@ -36,7 +36,6 @@ import { deepCompare } from "matrix-js-sdk/lib/utils";
 
 import { type Layout } from "../layout-types.ts";
 import {
-  mockLocalParticipant,
   mockMatrixRoomMember,
   mockRemoteParticipant,
   withTestScheduler,
@@ -61,7 +60,10 @@ import {
 import { MediaDevices } from "../MediaDevices.ts";
 import { getValue } from "../../utils/observable.ts";
 import { type Behavior, constant } from "../Behavior.ts";
-import { withCallViewModel as withCallViewModelInMode } from "./CallViewModelTestUtils.ts";
+import {
+  localParticipant,
+  withCallViewModel as withCallViewModelInMode,
+} from "./CallViewModelTestUtils.ts";
 import { MatrixRTCMode } from "../../settings/settings.ts";
 import { initializeWidget } from "../../widget.ts";
 
@@ -104,16 +106,7 @@ const dave = mockMatrixRoomMember(daveRtcMember, { rawDisplayName: "Dave" });
 
 const daveId = `${dave.userId}:${daveRtcMember.deviceId}`;
 
-const localParticipant = mockLocalParticipant({ identity: "" });
-const aliceSharingScreen = mockRemoteParticipant({
-  identity: aliceId,
-  isScreenShareEnabled: true,
-});
 const bobParticipant = mockRemoteParticipant({ identity: bobId });
-const bobSharingScreen = mockRemoteParticipant({
-  identity: bobId,
-  isScreenShareEnabled: true,
-});
 const daveParticipant = mockRemoteParticipant({ identity: daveId });
 
 export interface GridLayoutSummary {
@@ -277,11 +270,12 @@ describe.each([
     });
   });
 
-  test("screen sharing activates spotlight layout", () => {
+  test("remote screen sharing activates spotlight layout", () => {
     withTestScheduler(({ behavior, schedule, expectObservable }) => {
       // Start with no screen shares, then have Alice and Bob share their screens,
       // then return to no screen shares, then have just Alice share for a bit
-      const participantInputMarbles = "    abcda-ba";
+      const aliceSharingInputMarbles = "   ny-n--yn";
+      const bobSharingInputMarbles = "     n-y-n---";
       // While there are no screen shares, switch to spotlight manually, and then
       // switch back to grid at the end
       const modeInputMarbles = "           -----s--g";
@@ -292,13 +286,12 @@ describe.each([
       const expectedShowSpeakingMarbles = "y----nyny";
       withCallViewModel(
         {
-          remoteParticipants$: behavior(participantInputMarbles, {
+          remoteParticipants$: constant([aliceParticipant, bobParticipant]),
-            a: [aliceParticipant, bobParticipant],
-            b: [aliceSharingScreen, bobParticipant],
-            c: [aliceSharingScreen, bobSharingScreen],
-            d: [aliceParticipant, bobSharingScreen],
-          }),
           rtcMembers$: constant([localRtcMember, aliceRtcMember, bobRtcMember]),
+          sharingScreen: new Map([
+            [aliceParticipant, behavior(aliceSharingInputMarbles, yesNo)],
+            [bobParticipant, behavior(bobSharingInputMarbles, yesNo)],
+          ]),
         },
         (vm) => {
           schedule(modeInputMarbles, {
@@ -358,6 +351,76 @@ describe.each([
     });
   });
 
+  test("local screen sharing stays in grid layout", () => {
+    withTestScheduler(({ behavior, expectObservable }) => {
+      // Local participant shares their screen, then stops sharing
+      const sharingInputMarbles = "  nyn";
+      // Layout should show the screen share but stay in type: "grid"
+      const expectedLayoutMarbles = "aba";
+      withCallViewModel(
+        {
+          remoteParticipants$: constant([aliceParticipant, bobParticipant]),
+          rtcMembers$: constant([localRtcMember, aliceRtcMember, bobRtcMember]),
+          sharingScreen: new Map([
+            [localParticipant, behavior(sharingInputMarbles, yesNo)],
+          ]),
+        },
+        (vm) => {
+          expectObservable(summarizeLayout$(vm.layout$)).toBe(
+            expectedLayoutMarbles,
+            {
+              a: {
+                type: "grid",
+                spotlight: undefined,
+                grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`],
+              },
+              b: {
+                type: "grid",
+                spotlight: [`${localId}:0:screen-share`],
+                grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`],
+              },
+            },
+          );
+        },
+      );
+    });
+  });
+
+  test("local screen sharing in one-on-one call activates grid layout", () => {
+    withTestScheduler(({ behavior, expectObservable }) => {
+      // Local participant shares their screen, then stops sharing
+      const sharingInputMarbles = "  nyn";
+      // Layout should switch to grid layout then back to one-on-one layout
+      const expectedLayoutMarbles = "aba";
+      withCallViewModel(
+        {
+          remoteParticipants$: constant([aliceParticipant]),
+          rtcMembers$: constant([localRtcMember, aliceRtcMember]),
+          sharingScreen: new Map([
+            [localParticipant, behavior(sharingInputMarbles, yesNo)],
+          ]),
+        },
+        (vm) => {
+          expectObservable(summarizeLayout$(vm.layout$)).toBe(
+            expectedLayoutMarbles,
+            {
+              a: {
+                type: "one-on-one",
+                pip: `${localId}:0`,
+                spotlight: `${aliceId}:0`,
+              },
+              b: {
+                type: "grid",
+                spotlight: [`${localId}:0:screen-share`],
+                grid: [`${localId}:0`, `${aliceId}:0`],
+              },
+            },
+          );
+        },
+      );
+    });
+  });
+
   test("participants stay in the same order unless to appear/disappear", () => {
     withTestScheduler(({ behavior, schedule, expectObservable }) => {
       const visibilityInputMarbles = "a";
@@ -688,7 +751,7 @@ describe.each([
       withCallViewModel(
         {
           remoteParticipants$: constant([
-            aliceSharingScreen,
+            aliceParticipant,
             bobParticipant,
             daveParticipant,
           ]),
@@ -702,6 +765,7 @@ describe.each([
             [bobParticipant, behavior(bSpeakingInputMarbles, yesNo)],
             [daveParticipant, behavior(dSpeakingInputMarbles, yesNo)],
           ]),
+          sharingScreen: new Map([[aliceParticipant, constant(true)]]),
         },
         (vm) => {
           schedule(modeInputMarbles, {
@@ -856,26 +920,30 @@ describe.each([
     withTestScheduler(({ behavior, expectObservable }) => {
       // iterate through a number of combinations of participants and MatrixRTC memberships
       // Bob never has an MatrixRTC membership
-      const scenarioInputMarbles = " abcdec";
+      const participantInputMarbles = "abcd-c";
+      // Bob even tries to share his screen at the end
+      const bobSharingInputMarbles = " n---yn";
       // Bob should never be visible
-      const expectedLayoutMarbles = "a-bc-b";
+      const expectedLayoutMarbles = "  a-bc-b";
 
       withCallViewModel(
         {
-          remoteParticipants$: behavior(scenarioInputMarbles, {
+          remoteParticipants$: behavior(participantInputMarbles, {
             a: [],
             b: [bobParticipant],
             c: [aliceParticipant, bobParticipant],
             d: [aliceParticipant, daveParticipant, bobParticipant],
-            e: [aliceParticipant, daveParticipant, bobSharingScreen],
           }),
-          rtcMembers$: behavior(scenarioInputMarbles, {
+          rtcMembers$: behavior(participantInputMarbles, {
             a: [localRtcMember],
             b: [localRtcMember],
             c: [localRtcMember, aliceRtcMember],
             d: [localRtcMember, aliceRtcMember, daveRtcMember],
             e: [localRtcMember, aliceRtcMember, daveRtcMember],
           }),
+          sharingScreen: new Map([
+            [bobParticipant, behavior(bobSharingInputMarbles, yesNo)],
+          ]),
         },
         (vm) => {
           vm.setGridMode("grid");

src/state/CallViewModel/CallViewModel.ts

@@ -105,12 +105,16 @@ import {
 import {
   createLocalTransport$,
   JwtEndpointVersion,
+  type LocalTransport,
 } from "./localMember/LocalTransport.ts";
 import {
   createMemberships$,
   membershipsAndTransports$,
 } from "../SessionBehaviors.ts";
-import { ECConnectionFactory } from "./remoteMembers/ConnectionFactory.ts";
+import {
+  type ConnectionFactory,
+  ECConnectionFactory,
+} from "./remoteMembers/ConnectionFactory.ts";
 import {
   type ConnectionManagerData,
   createConnectionManager$,
@@ -170,6 +174,10 @@ export interface CallViewModelOptions {
   connectionState$?: Behavior<ConnectionState>;
   /** Optional behavior overriding the computed window size, mainly for testing purposes. */
   windowSize$?: Behavior<{ width: number; height: number }>;
+  /** Optional value overriding the local transport, for testing purposes. */
+  localTransport?: LocalTransport;
+  /** Optional value overriding the connection factory, for testing purposes. */
+  connectionFactory?: ConnectionFactory;
   /** The version & compatibility mode of MatrixRTC that we should use. */
   matrixRTCMode$?: Behavior<MatrixRTCMode>;
 }
@@ -441,6 +449,7 @@ export function createCallViewModel$(
         // Re-create LocalTransport whenever the mode changes
         (mode) => ({ keys: [mode], data: undefined }),
         (scope, _data$, mode) =>
+          options.localTransport ??
           createLocalTransport$({
             scope: scope,
             memberships$: memberships$,
@@ -467,17 +476,19 @@ export function createCallViewModel$(
     ),
   );
 
-  const connectionFactory = new ECConnectionFactory(
+  const connectionFactory =
-    client,
+    options.connectionFactory ??
-    matrixRoom.roomId,
+    new ECConnectionFactory(
-    mediaDevices,
+      client,
-    trackProcessorState$,
+      matrixRoom.roomId,
-    livekitKeyProvider,
+      mediaDevices,
-    getUrlParams().controlledAudioDevices,
+      trackProcessorState$,
-    options.livekitRoomFactory,
+      livekitKeyProvider,
-    getUrlParams().echoCancellation,
+      getUrlParams().controlledAudioDevices,
-    getUrlParams().noiseSuppression,
+      options.livekitRoomFactory,
-  );
+      getUrlParams().echoCancellation,
+      getUrlParams().noiseSuppression,
+    );
 
   const connectionManager = createConnectionManager$({
     scope: scope,
@@ -1078,9 +1089,10 @@ export function createCallViewModel$(
     );
 
   const oneOnOneLayoutMedia$: Observable<OneOnOneLayoutMedia | null> =
-    userMedia$.pipe(
+    combineLatest([userMedia$, screenShares$]).pipe(
-      switchMap((userMedia) => {
+      switchMap(([userMedia, screenShares]) => {
-        if (userMedia.length <= 2) {
+        // One-on-one layout only supports 2 user media, no screen shares
+        if (userMedia.length <= 2 && screenShares.length === 0) {
           const local = userMedia.find(
             (vm): vm is WrappedUserMediaViewModel & LocalUserMediaViewModel =>
               vm.type === "user" && vm.local,

src/state/CallViewModel/CallViewModelTestUtils.ts

@@ -15,7 +15,7 @@ import {
   type Room as LivekitRoom,
 } from "livekit-client";
 import { SyncState } from "matrix-js-sdk/lib/sync";
-import { BehaviorSubject, type Observable, map, of } from "rxjs";
+import { BehaviorSubject, combineLatest, map, of } from "rxjs";
 import { onTestFinished, vi } from "vitest";
 import { ClientEvent, type RoomMember, type MatrixClient } from "matrix-js-sdk";
 import EventEmitter from "events";
@@ -30,7 +30,10 @@ import {
   type CallViewModelOptions,
 } from "./CallViewModel";
 import {
+  exampleSfuConfig,
+  exampleTransport,
   mockConfig,
+  MockConnection,
   mockLivekitRoom,
   mockLocalParticipant,
   mockMatrixRoom,
@@ -68,13 +71,14 @@ export interface CallViewModelInputs {
   rtcMembers$: Behavior<Partial<CallMembership>[]>;
   roomMembers: RoomMember[];
   livekitConnectionState$: Behavior<ConnectionState>;
-  speaking: Map<Participant, Observable<boolean>>;
+  speaking: Map<Participant, Behavior<boolean>>;
+  sharingScreen: Map<Participant, Behavior<boolean>>;
   mediaDevices: MediaDevices;
   initialSyncState: SyncState;
   windowSize$: Behavior<{ width: number; height: number }>;
 }
 
-const localParticipant = mockLocalParticipant({ identity: "" });
+export const localParticipant = mockLocalParticipant({ identity: "" });
 
 export function withCallViewModel(mode: MatrixRTCMode) {
   return (
@@ -94,6 +98,7 @@ export function withCallViewModel(mode: MatrixRTCMode) {
         ConnectionState.Connected,
       ),
       speaking = new Map(),
+      sharingScreen = new Map(),
       mediaDevices = mockMediaDevices({}),
       initialSyncState = SyncState.Syncing,
       windowSize$ = constant({ width: 1000, height: 800 }),
@@ -154,13 +159,19 @@ export function withCallViewModel(mode: MatrixRTCMode) {
     const eventsSpy = vi
       .spyOn(ComponentsCore, "observeParticipantEvents")
       .mockImplementation((p, ...eventTypes) => {
-        if (eventTypes.includes(ParticipantEvent.IsSpeakingChanged)) {
+        return combineLatest([
-          return (speaking.get(p) ?? of(false)).pipe(
+          (eventTypes.includes(ParticipantEvent.IsSpeakingChanged) &&
-            map((s): Participant => ({ ...p, isSpeaking: s }) as Participant),
+            speaking.get(p)) ||
-          );
+            constant(false),
-        } else {
+          (eventTypes.includes(ParticipantEvent.TrackPublished) &&
-          return of(p);
+            sharingScreen.get(p)) ||
-        }
+            constant(false),
+        ]).pipe(
+          map(
+            ([isSpeaking, isScreenShareEnabled]) =>
+              ({ ...p, isSpeaking, isScreenShareEnabled }) as Participant,
+          ),
+        );
       });
 
     const roomEventSelectorSpy = vi
@@ -172,6 +183,13 @@ export function withCallViewModel(mode: MatrixRTCMode) {
     );
     const reactions$ = new BehaviorSubject<Record<string, ReactionInfo>>({});
 
+    const livekitRoomFactory = (): LivekitRoom =>
+      mockLivekitRoom({
+        localParticipant,
+        disconnect: async () => Promise.resolve(),
+        setE2EEEnabled: async () => Promise.resolve(),
+      });
+
     const vm = createCallViewModel$(
       testScope(),
       rtcSession.asMockedSession(),
@@ -181,14 +199,38 @@ export function withCallViewModel(mode: MatrixRTCMode) {
       {
         encryptionSystem: { kind: E2eeType.PER_PARTICIPANT },
         autoLeaveWhenOthersLeft: false,
-        livekitRoomFactory: (): LivekitRoom =>
+        livekitRoomFactory,
-          mockLivekitRoom({
-            localParticipant,
-            disconnect: async () => Promise.resolve(),
-            setE2EEEnabled: async () => Promise.resolve(),
-          }),
         connectionState$,
         windowSize$,
+        localTransport: {
+          active$: constant({
+            transport: exampleTransport,
+            sfuConfig: exampleSfuConfig,
+          }),
+          advertised$: constant(exampleTransport),
+        },
+        connectionFactory: {
+          createConnection(
+            scope,
+            transport,
+            ownMembershipIdentity,
+            logger,
+            sfuConfig,
+          ) {
+            return new MockConnection(
+              {
+                scope,
+                transport,
+                ownMembershipIdentity,
+                existingSFUConfig: sfuConfig,
+                client: room.client,
+                roomId: room.roomId,
+                livekitRoomFactory,
+              },
+              logger,
+            );
+          },
+        },
         matrixRTCMode$: constant(mode),
         ...options,
       },

src/utils/test.ts

@@ -74,6 +74,8 @@ import {
   createRemoteScreenShare,
   type RemoteScreenShareViewModel,
 } from "../state/media/RemoteScreenShareViewModel";
+import { Connection } from "../state/CallViewModel/remoteMembers/Connection";
+import { type SFUConfig } from "../livekit/openIDSFU";
 
 export function withFakeTimers(continuation: () => void): void {
   vi.useFakeTimers();
@@ -210,6 +212,13 @@ export const exampleTransport: LivekitTransport = {
   livekit_alias: "!alias:example.org",
 };
 
+export const exampleSfuConfig: SFUConfig = {
+  jwt: "foo",
+  livekitAlias: "bar",
+  livekitIdentity: "baz",
+  url: "bro",
+};
+
 export function mockRtcMembership(
   user: string | RoomMember,
   deviceId: string,
@@ -564,3 +573,8 @@ export function mockMuteStates(
     videoEnabled: false,
   });
 }
+
+export class MockConnection extends Connection {
+  public async start(): Promise<void> {}
+  public async stop(): Promise<void> {}
+}