Revert "Fast path for first media render."

This reverts commit 4b7fbeba72.
This commit is contained in:
Michelle Tang
2026-08-31 12:35:54 -04:00
parent f578c93367
commit 3749b47e3d
3 changed files with 11 additions and 97 deletions
@@ -163,7 +163,6 @@ class MediaPreviewFragment :
val startingAttachmentId = PartAuthority.requireAttachmentId(args.initialMediaUri)
val threadId = args.threadId
val appContext = requireContext().applicationContext
viewModel.fetchInitialAttachment(args)
viewModel.fetchAttachments(appContext, startingAttachmentId, threadId, sorting)
val dbObserver = DatabaseObserver.Observer { viewModel.refetchAttachments(appContext, startingAttachmentId, threadId, sorting) }
AppDependencies.databaseObserver.registerAttachmentUpdatedObserver(dbObserver)
@@ -230,43 +229,26 @@ class MediaPreviewFragment :
}
when (currentState.loadState) {
MediaPreviewState.LoadState.DATA_LOADED -> bindDataLoadedState(currentState)
MediaPreviewState.LoadState.MEDIA_READY -> {
// The full attachment window can arrive after the initially-opened media has already decoded.
if (syncPagerItems(currentState)) {
bindMediaReadyState(currentState)
}
}
MediaPreviewState.LoadState.MEDIA_READY -> bindMediaReadyState(currentState)
else -> Unit
}
}
/**
* Pushes the current record set into the pager. Both calls are self-guarded, so this is a no-op unless the
* backing items or the selected position actually changed.
*/
private fun syncPagerItems(currentState: MediaPreviewState): Boolean {
private fun bindDataLoadedState(currentState: MediaPreviewState) {
val currentPosition = currentState.position
val backingItems = currentState.mediaRecords.mapNotNull { it.attachment }
if (backingItems.isEmpty() || currentPosition < 0) {
onMediaNotAvailable()
return false
return
}
pagerAdapter.updateBackingItems(backingItems)
if (binding.mediaPager.currentItem != currentPosition) {
binding.mediaPager.setCurrentItem(currentPosition, false)
}
return true
}
private fun bindDataLoadedState(currentState: MediaPreviewState) {
if (!syncPagerItems(currentState)) {
return
}
val currentItem: MediaTable.MediaRecord = currentState.mediaRecords[currentState.position]
val currentItem: MediaTable.MediaRecord = currentState.mediaRecords[currentPosition]
bindTextViews(currentItem, currentState.showThread, currentState.messageBodies)
bindMenuItems(currentItem)
}
@@ -6,7 +6,6 @@ import android.text.SpannableString
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers
import io.reactivex.rxjava3.core.Completable
import io.reactivex.rxjava3.core.Flowable
import io.reactivex.rxjava3.core.Maybe
import io.reactivex.rxjava3.core.Single
import io.reactivex.rxjava3.schedulers.Schedulers
import org.signal.core.models.database.AttachmentId
@@ -46,8 +45,6 @@ class MediaPreviewRepository {
*/
fun getAttachments(context: Context, startingAttachmentId: AttachmentId, threadId: Long, sorting: Sorting, limit: Int = 500): Flowable<Result> {
return Single.fromCallable {
val stopwatch = Stopwatch("Attachment Window")
media.getGalleryMediaForThread(threadId, sorting).use { cursor ->
val mediaRecords = mutableListOf<MediaTable.MediaRecord>()
var startingRow = -1
@@ -57,7 +54,6 @@ class MediaPreviewRepository {
break
}
}
stopwatch.split("find starting row")
var itemPosition = -1
if (startingRow >= 0) {
@@ -68,7 +64,10 @@ class MediaPreviewRepository {
for (i in 0..limit) {
val element = MediaTable.MediaRecord.from(cursor)
if (element.attachment?.isDisplayable() == true) {
if (element.attachment?.transferState == AttachmentTable.TRANSFER_PROGRESS_DONE ||
element.attachment?.transferState == AttachmentTable.TRANSFER_PROGRESS_STARTED ||
element.attachment?.thumbnailUri != null
) {
mediaRecords.add(element)
if (startingAttachmentId.id == cursor.requireLong(AttachmentTable.ID)) {
@@ -85,32 +84,12 @@ class MediaPreviewRepository {
Log.w(TAG, "Unable to find target image for $startingAttachmentId")
}
}
stopwatch.split("build window of ${mediaRecords.size}")
stopwatch.stop(TAG)
Result(if (mediaRecords.isNotEmpty()) itemPosition.coerceIn(mediaRecords.indices) else itemPosition, mediaRecords)
}
}.subscribeOn(Schedulers.io()).toFlowable()
}
/**
* Primary-key read of a single attachment, so the tapped media can be rendered without waiting on
* [getAttachments] to sort and materialize the whole attachment window. Empty when the id does not
* name a displayable attachment, which is the normal case for previews of draft media.
*/
fun getInitialAttachment(attachmentId: AttachmentId): Maybe<DatabaseAttachment> {
return Maybe.fromCallable<DatabaseAttachment> {
SignalDatabase.attachments.getAttachment(attachmentId)?.takeIf { it.isDisplayable() }
}.subscribeOn(Schedulers.io())
}
/** Matches the filter [getAttachments] applies when building its window, so both agree on what can be paged to. */
private fun DatabaseAttachment.isDisplayable(): Boolean {
return transferState == AttachmentTable.TRANSFER_PROGRESS_DONE ||
transferState == AttachmentTable.TRANSFER_PROGRESS_STARTED ||
thumbnailUri != null
}
fun resolveMessageBodies(context: Context, messageIds: Set<Long>): Single<Map<Long, SpannableString>> {
return Single.fromCallable {
SignalDatabase.messages.getMessages(messageIds).toList().withAttachments()
@@ -29,7 +29,6 @@ import org.thoughtcrime.securesms.database.MediaTable
import org.thoughtcrime.securesms.database.SignalDatabase
import org.thoughtcrime.securesms.dependencies.AppDependencies
import org.thoughtcrime.securesms.logsubmit.SubmitDebugLogActivity
import org.thoughtcrime.securesms.mms.PartAuthority
import org.thoughtcrime.securesms.mms.PartUriParser
import org.thoughtcrime.securesms.notifications.NotificationChannels
import org.thoughtcrime.securesms.notifications.NotificationIds
@@ -69,45 +68,6 @@ class MediaPreviewViewModel : ViewModel() {
return currentPosition in store.state.mediaRecords.indices && store.state.mediaRecords[currentPosition].toMedia()?.uri == initialMediaUri
}
/**
* Publishes the tapped attachment on its own, ahead of [fetchAttachments], so the full-resolution decode is not
* gated on the whole-thread gallery query. Everything the record needs beyond the attachment row itself is
* already in [args].
*
* Only ever applies to a fresh load: the [MediaPreviewState.LoadState.INIT] check is re-run inside the state
* update so a single-item result can never replace an already-loaded window, whether because the full query won
* the race or because [refetchAttachments] is rebuilding it underneath a browsing user.
*/
fun fetchInitialAttachment(args: MediaIntentFactory.MediaPreviewArgs) {
if (store.state.loadState != MediaPreviewState.LoadState.INIT) {
return
}
disposables += repository.getInitialAttachment(PartAuthority.requireAttachmentId(args.initialMediaUri)).subscribe { attachment ->
store.update { oldState ->
if (oldState.loadState != MediaPreviewState.LoadState.INIT) {
return@update oldState
}
val record = MediaTable.MediaRecord(
attachment = attachment,
recipientId = args.fromRecipientId,
threadRecipientId = args.threadRecipientId,
threadId = args.threadId,
messageId = args.messageId,
date = args.date,
isOutgoing = args.outgoing
)
oldState.copy(
position = 0,
mediaRecords = listOf(record),
loadState = MediaPreviewState.LoadState.DATA_LOADED
)
}
}
}
fun fetchAttachments(context: Context, startingAttachmentId: AttachmentId, threadId: Long, sorting: MediaTable.Sorting, forceRefresh: Boolean = false) {
if (store.state.loadState == MediaPreviewState.LoadState.INIT || forceRefresh) {
disposables += repository.getAttachments(context, startingAttachmentId, threadId, sorting).subscribe { result ->
@@ -120,26 +80,19 @@ class MediaPreviewViewModel : ViewModel() {
}
acc
}
// Never downgrade a MEDIA_READY state: the initial attachment may already have finished decoding.
val loadState = if (oldState.loadState == MediaPreviewState.LoadState.MEDIA_READY) {
MediaPreviewState.LoadState.MEDIA_READY
} else {
MediaPreviewState.LoadState.DATA_LOADED
}
if (oldState.leftIsRecent) {
oldState.copy(
position = result.initialPosition,
mediaRecords = result.records,
albums = albums,
loadState = loadState
loadState = MediaPreviewState.LoadState.DATA_LOADED
)
} else {
oldState.copy(
position = result.records.size - result.initialPosition - 1,
mediaRecords = result.records.reversed(),
albums = albums.mapValues { it.value.reversed() },
loadState = loadState
loadState = MediaPreviewState.LoadState.DATA_LOADED
)
}
}