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

Update SvgDecoder.android.kt to support scaling #2090

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

seanpont
Copy link

Scales SVGs correctly

Issue: #2048

Please run ./test.sh before submitting to ensure your pull request passes the automated checks.

Scales SVGs correctly

Issue: coil-kt#2048
@colinrtwhite
Copy link
Member

Thanks! I think we can ship this with a few changes:

  • Let's put these changes behind a flag in SvgDecoder.Factory's constructor. We need to preserve the existing behaviour for existing users.
  • Do we need to make the same changes to SvgDecoder.nonAndroid? Ideally we want the Android and non-Android implementations to behave as similarly as possible.
  • Please add a test in AbstractSvgDecoderTest. This'll make sure that we don't break the behaviour in the future.

@seanpont
Copy link
Author

seanpont commented Jan 30, 2024 via email

@colinrtwhite
Copy link
Member

@seanpont Ah gotcha! If you'd like to make the change to the 2.x releases you'll need to target your PR to the 2.x branch. main currently has all the 3.0 alpha code which supports multiplatform.

I think we should still add a flag to SvgDecoder's constructor to avoid changing the behaviour for existing users. Maybe densityScalingEnabled: Boolean = false?

I can work on releasing 2.6.0 with those changes once they're merged.

@davidcorradowell
Copy link

Looking at the commits does svg.scale even exist? I attempted to copy this code and it did not work. Is there something I am missing to get svg.scale be available?

Is there any obvious downsides of doing an implementation like this by default eventually?

@androidacy-user
Copy link

Gonna +1 getting this merged in some form. For us, SVGs render at a miniscule size on Android when using coil

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

4 participants