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.

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

When attaching multiple images at once, they appear in random order #4754

Closed
mcclure opened this issue Nov 8, 2024 · 6 comments · Fixed by #4841
Closed

When attaching multiple images at once, they appear in random order #4754

mcclure opened this issue Nov 8, 2024 · 6 comments · Fixed by #4841
Labels

Comments

@mcclure
Copy link
Collaborator

mcclure commented Nov 8, 2024

Steps to reproduce the problem

  1. Start composing a post
  2. Click paperclip, "add media"
  3. In the image selector, longpress four images, tap "select"

Expected behavior: The four images appear in the post in the order I longpressed them

Observed behavior: The four images appear in what appears to be literally random order. It's not chronological.

This significantly slows down making posts with many images, because on my phone I have to do multiple clicks to get into the Photos app and then that is multiplied by four due to this bug.

Odd point for comparison: In my testing it appears the bluesky social app does have this problem and Discord does not. The fact Discord does not have the problem makes me suspect the problem is in the app and not in Android.

Debug information

Tusky 26.2
"Version: 4.4.0-nightly.2024-11-06"
Android 13
Sony Xperia 5 III
mastodon.social

However, also @connyduck reports he saw this.

@mcclure mcclure added the bug label Nov 8, 2024
@sharponlooker
Copy link

Same issue, same Tusky version on Android 14, Samsung Galaxy

@Lakoja
Copy link
Collaborator

Lakoja commented Dec 30, 2024

Technical reason: The asyncness in ComposeActivity#pickMedia() randomizes the list elements somehow.

One can index the elements for example in ComposeActivity#pickMediaFileLauncher to see this.

@connyduck
Copy link
Collaborator

Technical reason: The asyncness in ComposeActivity#pickMedia() randomizes the list elements somehow.

Oh, nice find. I assumed it was because the file picker already gives us the files in random order.
Not switching to Dispatchers.IO before adding media to the queue seems to fix this. Let me check if that has some kind of performance drawback...

@connyduck
Copy link
Collaborator

connyduck commented Dec 31, 2024

Ok, so MediaUploader.prepareMedia does some I/O and definitely must run on Dispatchers.IO or we risk blocking the main thread.

Here is what I would try:
Make ComposeViewModel.pickMedia take a list of picked media. In there, call prepareMedia for each media and keep track of the order. Probably with async/await.

There is also quite some potential for refactoring / code quality improvements.
stashMediaItem in ComposeViewModel.addMediaToQueue seems unnecessary, and addMediaToQueue does not need to be suspending. And ComposeActivity should not launch the coroutines, instead ComposeViewModel should (because it does not get cancelled on screen rotation there).

@connyduck
Copy link
Collaborator

Do you want to send a pull request for this @Lakoja or should I?

@Lakoja
Copy link
Collaborator

Lakoja commented Dec 31, 2024

I'll happily review it. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants