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

Improve/Optimize Coil Usage #287

Merged
merged 4 commits into from
Apr 25, 2021
Merged

Improve/Optimize Coil Usage #287

merged 4 commits into from
Apr 25, 2021

Conversation

OxygenCobalt
Copy link
Contributor

@OxygenCobalt OxygenCobalt commented Mar 25, 2021

This PR fixes a variety of issues with how coil is used in the app:

  • All ImageLoader instances have been replaced with a single App-Wide loader, as is recommended by the Coil Documentation. This improves efficiency and memory usage since bitmaps can be shared across the app. Most of the shared configuration [Such as the error icon] have been moved to here.
  • Replaced most of the custom ImageRequest instances with the stock ImageView.load method.
  • Instead of using a blocking call to get a Bitmap and then passing that to coil, the cover URI is now directly passed to Coil, which then loads it to the ImageView asynchronously. This greatly increases efficiency.
  • Used Coil in the MediaMetadataCompat & Notification instances [Previously they used blocking calls and didn't share bitmap data with the rest of the app].
  • Removed all IO co-routines involved with music loading since Coil already uses them.
  • Enabled bitmap pooling, decreasing memory usage since bitmaps can be reused.
  • Disabled automatic disc-caching, which decreases app size over time.
  • Removed redundant state modifiers [E.G sIsCovers instead of just goPreference.isCovers]

Overall, this makes image loading both more maintainable and less error-prone compared to before. The most major API change is probably that the createNotification is now async, requiring an onDone lambda instead of returning the value directly. Other than that this preserves most of the normal architecture. Feel free to raise any concerns with my change.

Do multiple things to make the cover loading on the now-playing screen more elegant
- Use a dedicated image loader bound to the application
- Get rid of the coroutines, coil already does it
- Completely use the Built-In "Load" method
- Move image builder init to GoApp
Do multiple things to streamline coil usage on DetailsFragment/MusicContainersListFragment
- Don't use custom ImageLoader
- Use the load extension method with the app-wide imageloader, removing the need for error drawables
- Removed the unneccisary coroutines
Use coil on the notification and the mediasession, instead of the old method that would halt execution.
Fix some problems with the new coil usage:
- Enable bitmap pooling for efficency, but disable disk caching since it isnt needed
- Fix a problem where the metadata wouldnt properly update
- Fix a problem where the album detail view would respect the cover settings
@enricocid enricocid merged commit a30f411 into enricocid:main Apr 25, 2021
@enricocid
Copy link
Owner

Thanks :)

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

Successfully merging this pull request may close these issues.

2 participants