Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Self video disappears when scrolling through video grid (WPB-4651) #2246

Merged
merged 5 commits into from
Sep 20, 2023
Merged

fix: Self video disappears when scrolling through video grid (WPB-4651) #2246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
14cc3b7
fix: memory leak renderer related to video renderer
ohassine Sep 15, 2023
61de03b
Merge remote-tracking branch 'origin/develop' into memory_leak_renderer
ohassine Sep 15, 2023
bd3e584
Merge branch 'develop' into memory_leak_renderer
ohassine Sep 15, 2023
04c8278
fix: Self video disappears when scrolling through video grid
ohassine Sep 19, 2023
5fc6fe0
Merge remote-tracking branch 'origin/develop' into unnecessary_recomp…
ohassine Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 7 additions & 9 deletions app/src/main/kotlin/com/wire/android/ui/calling/ongoing/OngoingCallScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.hilt.navigation.compose.hiltViewModel
import com.ramcosta.composedestinations.annotation.Destination
Expand Down Expand Up @@ -81,6 +80,7 @@ import com.wire.android.ui.common.topappbar.WireCenterAlignedTopAppBar
import com.wire.android.ui.theme.wireColorScheme
import com.wire.android.ui.theme.wireDimensions
import com.wire.android.ui.theme.wireTypography
import com.wire.android.util.ui.PreviewMultipleThemes
import com.wire.kalium.logic.data.id.ConversationId
import java.util.Locale

Expand Down Expand Up @@ -250,12 +250,8 @@ private fun OngoingCallContent(
onSelfVideoPreviewCreated = setVideoPreview,
onSelfClearVideoPreview = clearVideoPreview,
requestVideoStreams = requestVideoStreams,
onDoubleTap = { selectedUserId, selectedClientId, isSelf ->
selectedParticipantForFullScreen = SelectedParticipant(
userId = selectedUserId,
clientId = selectedClientId,
isSelfUser = isSelf
)
onDoubleTap = { selectedParticipant ->
selectedParticipantForFullScreen = selectedParticipant
shouldOpenFullScreen = !shouldOpenFullScreen
}
)
Expand Down Expand Up @@ -295,7 +291,9 @@ private fun OngoingCallTopBar(
.fillMaxWidth()
.offset(y = -(5).dp),
textAlign = TextAlign.Center,
text = stringResource(id = R.string.calling_constant_bit_rate_indication).uppercase(Locale.getDefault()),
text = stringResource(id = R.string.calling_constant_bit_rate_indication).uppercase(
Locale.getDefault()
),
color = colorsScheme().secondaryText,
style = MaterialTheme.wireTypography.title03,
)
Expand Down Expand Up @@ -353,8 +351,8 @@ private fun CallingControls(
}
}

@PreviewMultipleThemes
@Composable
@Preview
fun PreviewOngoingCallTopBar() {
OngoingCallTopBar("Default", true) { }
}
6 changes: 4 additions & 2 deletions app/src/main/kotlin/com/wire/android/ui/calling/ongoing/fullscreen/FullScreenTile.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import androidx.compose.ui.draw.clipToBounds
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import androidx.hilt.navigation.compose.hiltViewModel
Expand All @@ -43,6 +42,7 @@ import com.wire.android.ui.calling.SharedCallingViewModel
import com.wire.android.ui.calling.ongoing.OngoingCallViewModel.Companion.DOUBLE_TAP_TOAST_DISPLAY_TIME
import com.wire.android.ui.calling.ongoing.participantsview.ParticipantTile
import com.wire.android.ui.common.dimensions
import com.wire.android.util.ui.PreviewMultipleThemes
import kotlinx.coroutines.delay

@Composable
Expand Down Expand Up @@ -74,6 +74,8 @@ fun FullScreenTile(
),
participantTitleState = it,
isSelfUser = selectedParticipant.isSelfUser,
isSelfUserCameraOn = selectedParticipant.isSelfUserCameraOn,
isSelfUserMuted = selectedParticipant.isSelfUserMuted,
shouldFill = false,
isZoomingEnabled = true,
onSelfUserVideoPreviewCreated = sharedCallingViewModel::setVideoPreview,
Expand All @@ -98,7 +100,7 @@ fun FullScreenTile(
}
}

@Preview
@PreviewMultipleThemes
@Composable
fun PreviewFullScreenVideoCall() {
FullScreenTile(
Expand Down
4 changes: 3 additions & 1 deletion app/src/main/kotlin/com/wire/android/ui/calling/ongoing/fullscreen/SelectedParticipant.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,7 @@
data class SelectedParticipant(
val userId: UserId = UserId("", ""),
val clientId: String = "",
val isSelfUser: Boolean = false
val isSelfUser: Boolean = false,
val isSelfUserCameraOn: Boolean = false,
val isSelfUserMuted: Boolean = false

Check warning on line 27 in app/src/main/kotlin/com/wire/android/ui/calling/ongoing/fullscreen/SelectedParticipant.kt

View check run for this annotation

Codecov / codecov/patch

app/src/main/kotlin/com/wire/android/ui/calling/ongoing/fullscreen/SelectedParticipant.kt#L25-L27 

Added lines #L25 - L27 were not covered by tests
)
35 changes: 22 additions & 13 deletions app/src/main/kotlin/com/wire/android/ui/calling/ongoing/participantsview/ParticipantTile.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import com.wire.android.ui.common.dimensions
import com.wire.android.ui.home.conversationslist.model.Membership
import com.wire.android.ui.theme.wireColorScheme
import com.wire.android.ui.theme.wireTypography
import com.wire.android.util.ui.PreviewMultipleThemes
import com.wire.kalium.logic.data.id.QualifiedID

@Composable
Expand All @@ -90,6 +91,8 @@ fun ParticipantTile(
isSelfUser: Boolean,
shouldFill: Boolean = true,
isZoomingEnabled: Boolean = false,
isSelfUserMuted: Boolean,
isSelfUserCameraOn: Boolean,
onSelfUserVideoPreviewCreated: (view: View) -> Unit,
onClearSelfUserVideoPreview: () -> Unit
) {
Expand All @@ -114,7 +117,7 @@ fun ParticipantTile(

if (isSelfUser) {
CameraPreview(
isCameraOn = participantTitleState.isCameraOn,
isCameraOn = isSelfUserCameraOn,
onSelfUserVideoPreviewCreated = onSelfUserVideoPreviewCreated,
onClearSelfUserVideoPreview = onClearSelfUserVideoPreview
)
Expand All @@ -139,7 +142,7 @@ fun ParticipantTile(
bottom.linkTo(parent.bottom)
start.linkTo(parent.start)
},
isMuted = participantTitleState.isMuted,
isMuted = if (isSelfUser) isSelfUserMuted else participantTitleState.isMuted,
hasEstablishedAudio = participantTitleState.hasEstablishedAudio
)

Expand Down Expand Up @@ -216,12 +219,12 @@ private fun CameraPreview(
setShouldFill(false)
}
}
AndroidView(factory = {
val frameLayout = FrameLayout(it)
onSelfUserVideoPreviewCreated(videoPreview)
frameLayout.addView(videoPreview)
frameLayout
})
AndroidView(
factory = { videoPreview },
update = {
onSelfUserVideoPreviewCreated(videoPreview)
}
)
} else {
onClearSelfUserVideoPreview()
}
Expand Down Expand Up @@ -419,11 +422,13 @@ fun PreviewParticipantTile() {
),
onClearSelfUserVideoPreview = {},
onSelfUserVideoPreviewCreated = {},
isSelfUser = false
isSelfUser = false,
isSelfUserMuted = false,
isSelfUserCameraOn = false
)
}

@Preview
@PreviewMultipleThemes
@Composable
fun PreviewParticipantTalking() {
ParticipantTile(
Expand All @@ -442,11 +447,13 @@ fun PreviewParticipantTalking() {
),
onClearSelfUserVideoPreview = {},
onSelfUserVideoPreviewCreated = {},
isSelfUser = false
isSelfUser = false,
isSelfUserMuted = false,
isSelfUserCameraOn = false
)
}

@Preview
@PreviewMultipleThemes
@Composable
fun PreviewParticipantConnecting() {
ParticipantTile(
Expand All @@ -467,6 +474,8 @@ fun PreviewParticipantConnecting() {
),
onClearSelfUserVideoPreview = {},
onSelfUserVideoPreviewCreated = {},
isSelfUser = false
isSelfUser = false,
isSelfUserMuted = false,
isSelfUserCameraOn = false
)
}
35 changes: 23 additions & 12 deletions ...src/main/kotlin/com/wire/android/ui/calling/ongoing/participantsview/ParticipantsTiles.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,23 @@ import androidx.compose.material3.Surface
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import com.wire.android.ui.calling.model.UICallParticipant
import com.wire.android.ui.calling.ongoing.fullscreen.SelectedParticipant
import com.wire.android.ui.calling.ongoing.participantsview.gridview.GroupCallGrid
import com.wire.android.ui.calling.ongoing.participantsview.horizentalview.CallingHorizontalView
import com.wire.android.ui.common.colorsScheme
import com.wire.android.ui.common.dimensions
import com.wire.android.ui.theme.wireDimensions
import com.wire.kalium.logic.data.user.UserId
import com.wire.android.util.ui.PreviewMultipleThemes

private const val MAX_TILES_PER_PAGE = 8
private const val MAX_ITEMS_FOR_HORIZONTAL_VIEW = 3

@OptIn(ExperimentalFoundationApi::class)
@Composable
Expand All @@ -61,7 +65,7 @@ fun VerticalCallingPager(
onSelfVideoPreviewCreated: (view: View) -> Unit,
onSelfClearVideoPreview: () -> Unit,
requestVideoStreams: (participants: List<UICallParticipant>) -> Unit,
onDoubleTap: (userId: UserId, clientId: String, isSelfUser: Boolean) -> Unit
onDoubleTap: (selectedParticipant: SelectedParticipant) -> Unit
) {
Column(
modifier = Modifier
Expand All @@ -78,7 +82,9 @@ fun VerticalCallingPager(
) { pageIndex ->
if (participants.isNotEmpty()) {

val participantsChunkedList = participants.chunked(MAX_TILES_PER_PAGE)
val participantsChunkedList = remember(participants) {
participants.chunked(MAX_TILES_PER_PAGE)
}
val participantsWithCameraOn by rememberUpdatedState(participants.count { it.isCameraOn })
val participantsWithScreenShareOn by rememberUpdatedState(participants.count { it.isSharingScreen })

Expand All @@ -90,14 +96,18 @@ fun VerticalCallingPager(
requestVideoStreams(participantsChunkedList[pagerState.currentPage])
}

if (participantsChunkedList[pageIndex].size <= MAX_ITEMS_FOR_ONE_ON_ONE_VIEW) {
if (participantsChunkedList[pageIndex].size <= MAX_ITEMS_FOR_HORIZONTAL_VIEW) {
CallingHorizontalView(
participants = participantsChunkedList[pageIndex],
pageIndex = pageIndex,
isSelfUserMuted = isSelfUserMuted,
isSelfUserCameraOn = isSelfUserCameraOn,
contentHeight = contentHeight,
onSelfVideoPreviewCreated = onSelfVideoPreviewCreated,
onSelfVideoPreviewCreated = {
if (pagerState.currentPage == 0) {
onSelfVideoPreviewCreated(it)
}
},
onSelfClearVideoPreview = onSelfClearVideoPreview,
onDoubleTap = onDoubleTap
)
Expand All @@ -108,7 +118,11 @@ fun VerticalCallingPager(
isSelfUserMuted = isSelfUserMuted,
isSelfUserCameraOn = isSelfUserCameraOn,
contentHeight = contentHeight,
onSelfVideoPreviewCreated = onSelfVideoPreviewCreated,
onSelfVideoPreviewCreated = {
if (pagerState.currentPage == 0) {
onSelfVideoPreviewCreated(it)
}
},
onSelfClearVideoPreview = onSelfClearVideoPreview,
onDoubleTap = onDoubleTap
)
Expand Down Expand Up @@ -152,11 +166,8 @@ private fun pagesCount(size: Int): Int {
} else pages
}

private const val MAX_TILES_PER_PAGE = 8
private const val MAX_ITEMS_FOR_ONE_ON_ONE_VIEW = 3

@PreviewMultipleThemes
@Composable
@Preview
fun PreviewVerticalCallingPager() {
VerticalCallingPager(
participants = listOf(),
Expand All @@ -166,6 +177,6 @@ fun PreviewVerticalCallingPager() {
onSelfVideoPreviewCreated = {},
onSelfClearVideoPreview = {},
requestVideoStreams = {},
onDoubleTap = { _, _, _ -> }
onDoubleTap = { }
)
}