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

Increase quality of drawables on older devices #5468

Merged
merged 1 commit into from Feb 1, 2024

Conversation

Jean-BaptisteC
Copy link
Contributor

@Jean-BaptisteC Jean-BaptisteC commented Jan 31, 2024

This PR replaces android:src by app:srcCompat
That improves the quality of drawables on older devices because AppCompat library does a better scaling that Android library -> more information here https://stackoverflow.com/questions/34936590/why-isnt-my-vector-drawable-scaling-as-expected
No impact in Kotlin code -> It's the same code to set icons with Android library and AppCompat library.

@westnordost
Copy link
Member

westnordost commented Jan 31, 2024

more information here https://stackoverflow.com/questions/34936590/why-isnt-my-vector-drawable-scaling-as-expected

Mh, at least the first comment(s) don't mention srcCompat and those that do, mention it only as part of some more involved workaround. Plus the whole discussion is 8 years old. Impossible to say if nothing has changed.

Do you have a device that runs on Android Lollipop and have actually been experiencing issues? The more "kit" is added to solve compatibility issues (that maybe don't exist or are no issue??), the more complicated it gets.

@Jean-BaptisteC
Copy link
Contributor Author

Jean-BaptisteC commented Feb 1, 2024

My explanation is not fully correct, we already use AppCompatImageView, because ImageView is auto inflated in AppCompatImageView.
And we use already app:tint property, a property from AppCompat -> https://developer.android.com/reference/kotlin/androidx/appcompat/widget/AppCompatImageView
Why do not use new properties from AppCompat for better drawables support? And specially in this case look of drawables is better on older devices with low dpi -> I have doing some tests with another app on old tablet, we see a real difference, drawables is more clean. And keep in mind, at the start of Android ImageView has been developped to support Image (webp, png) not to support SVG, that's why ImageView library is better with images that with SVG.

@westnordost
Copy link
Member

I just want to make sure we are not trying to solve a problem that does not exist. I don't know what other changes using srcCompat may bring. Will it only replace behavior on Android versions that are known to have issues, or will it also replace behavior on versions that don't (, adding overhead)? Are there actually any old Android versions still supported by the app that do have issues with vector drawable scaling?

I know of one appcompat thing the Android linter also keeps nagging about that is actually not even relevant for the app anymore as its min SDK version is above the version that used to cause issues. I had to turn off that linter warning in the build.gradle.kts to have my peace.

@Jean-BaptisteC
Copy link
Contributor Author

I understood your position, srcCompat replace behavior on all Android versions.
Scaling problem appears on devices with Android 5 and low dpi. Here you can find example in other app -> organicmaps/organicmaps#3556

I have planned to doing somes tests on older device and add table before/after

@westnordost
Copy link
Member

I have planned to doing somes tests on older device and add table before/after

If I knew this PR solves an actual problem, that would make me feel somewhat more comfortable. Alternatively, knowing what exactly srcCompat does (on all Android versions) would also help.

@Jean-BaptisteC
Copy link
Contributor Author

I have done some tests on SC (On Lenovo Yoga Tab 2 - Android 5) and I agree, the difference is very little
Here is an example with screenshots tutorials

Before After

I have found more information about vectors support with AppCompat -> https://medium.com/androiddevelopers/appcompat-v23-2-age-of-the-vectors-91cbafa87c88#.pf6wmj3oz
I'm not an expert on the topic, but from my experience, I use this property on exodus app project from app has been reworked in Kotlin with any problems.

@Jean-BaptisteC Jean-BaptisteC changed the title Use AppCompat ImageView instead ImageView Increase quality of drawables on older devices Feb 1, 2024
@westnordost
Copy link
Member

Alright, let's merge it then. (I've also read the article, it mentions that the compat drawing is only used on versions below API 21, which can't be entirely true because your tablet is API 21)

Thank you for bearing with me!

@westnordost westnordost merged commit 3ddd761 into streetcomplete:master Feb 1, 2024
@Jean-BaptisteC Jean-BaptisteC deleted the CompatImageView branch April 14, 2024 15:16
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.

None yet

2 participants