Take screen density into account when requesting thumbnails

[jmartinesp]

Type of change

• Feature
• Bugfix
• Technical
• Other :

Content

Have CoilMediaFetcher calculate the right size to ask for thumbnails using the screen density.

Motivation and context

Fixes issues seen with blurry avatars on some homeservers.

Screenshots / GIFs

Before	After

Tests

• Log in to element.io.
• Check avatar images.

They should look a lot shaper.

Tested devices

• Physical
• Emulator
• OS version(s): 11

Checklist

• Changes have been tested on an Android device or Android emulator with API 23
• UI change has been tested on both light and dark themes
• Accessibility has been taken into account. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
• Pull request is based on the develop branch
• Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-x-android/blob/develop/CONTRIBUTING.md#changelog
• Pull request includes screenshots or videos if containing UI changes
• Pull request includes a sign off
• You've made a self review of your PR

[github-actions]

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/FS8iLq

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.02% ⚠️

Comparison is base (5a0e0a8) 57.65% compared to head (e7e41b0) 57.64%.
Report is 23 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1262      +/-   ##
===========================================
- Coverage    57.65%   57.64%   -0.02%     
===========================================
  Files         1081     1081              
  Lines        28041    28046       +5     
  Branches      5777     5777              
===========================================
  Hits         16167    16167              
- Misses        9350     9355       +5     
  Partials      2524     2524              

Files Changed	Coverage Δ
...roid/libraries/matrix/ui/media/CoilMediaFetcher.kt	0.00% <0.00%> (ø)
.../libraries/matrix/ui/media/ImageLoaderFactories.kt	0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[bmarty]

Thanks for looking at this.

Don't you think the fix should be located at the call site?

For instance here: https://github.com/vector-im/element-x-android/blob/develop/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/media/AvatatarDataExt.kt#L26

We should pass a pixel size instead of a DP size. Maybe more complicated though...

[jmartinesp]

Yes, that was my first idea, but to add it there we'd either have to make that function @Composable or pass either a context or a density value. And if instead of storing the dp size in AvatarSize we wanted the pixel one we'd have to do it when those are created, and given it's an enum class that would be quite convoluted to make it work.

Placing this calculation in the fetcher seemed like the best compromise.

[bmarty]

OK, thanks for explanation!

[bmarty]

Not a blocker, but instead of passing the whole context, can we just pass the screen density here?
Not sure that this can change over time, but maybe (with battery saver mode which simulate low density screen).

Or maybe just context.resources.displayMetrics.

I am definitely thinking out loud ;)

[jmartinesp]

Actually, I'm wondering about what would happen if we passed it here and then changed the settings for display element scaling in the OS. Maybe we should pass a lambda or a wrapper DensityProvider class that hides the context?

[jmartinesp]

Done in e7e41b0.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[bmarty]

OK, thanks!