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

Respect fill=currentColor for svg images by referencing content color in theme #1209

Closed
digitalheir opened this issue Mar 21, 2022 · 5 comments · Fixed by #1210
Closed

Respect fill=currentColor for svg images by referencing content color in theme #1209

digitalheir opened this issue Mar 21, 2022 · 5 comments · Fixed by #1210
Labels
enhancement New feature or request

Comments

@digitalheir
Copy link

digitalheir commented Mar 21, 2022

Is your feature request related to a problem? Please describe.

I want to load a bunch of svg's which have their fill specified as currentColor. The color appears black in both light mode and dark mode. This looks bad for my app.

looking into the source code of caverock-androidsvg, I see that:

  def.color = Colour.BLACK; // currentColor defaults to black

Describe the solution you'd like

It would be grand if either of these happened:

  1. AsyncImage composable automatically picks up the LocalContentColor.current composition local and passes it to SvgDecoder to set the svg style.
  2. More styling settings settings, including currentColor, were exposed in SvgDecoder
  3. AndroidSVG was exposed as a transitive API in Coil (I wouldn't like this solution actually)

Additional context
Example svg:

	<svg xmlns="http://www.w3.org/2000/svg" width="100px" height="100px" viewBox="0 0 17 17">
		<path fill="currentColor" d="M14.755 14.006l-2.536-3.381H13.283a.531.531 0 0 0 .41-.869l-2.536-3.381h1.063a.532.532 0 0 0 .415-.863L8.385.199a.53.53 0 0 0-.83 0l-4.25 5.313a.531.531 0 0 0 .415.863h1.063l-2.55 3.4a.53.53 0 0 0 .425.85h1.063l-2.55 3.4a.53.53 0 0 0 .425.85h4.781v1.594c0 .293.238.531.531.531h2.125a.531.531 0 0 0 .531-.531v-1.594h4.782a.531.531 0 0 0 .41-.869z"/>
@digitalheir digitalheir added the enhancement New feature or request label Mar 21, 2022
@digitalheir
Copy link
Author

digitalheir commented Mar 21, 2022

(BTW, I'm aware of ColorFilter.tint and for my current use-case it actually works since my images are monochrome. But I think it fails if the path in an svg image are filled with a combination of currentColor and explicit color values.)

@colinrtwhite
Copy link
Member

colinrtwhite commented Mar 21, 2022

Does caverock-svg have a public API for this? I took a quick look and those values look like they can only be accessed internally.

@digitalheir
Copy link
Author

digitalheir commented Mar 22, 2022

I figured out how to set the path color in AndroidSVG, they expect you to pass it as CSS:

val bitmap = createBitmap(bitmapWidth, bitmapHeight, options.config.toSoftware())
val renderOptions = RenderOptions.create()
if (colorHex != null) renderOptions.css("svg { color: $colorHex }");
svg.renderToCanvas(Canvas(bitmap), renderOptions)

(See androidsvg#11)

Converting a Compose color to hex string:

fun androidx.compose.ui.graphics.Color.toHexColor(): String {
    return "#${Integer.toHexString(toArgb())}"
}

It's funny to me that this will perform so many different type conversions of a colour value, but alas.

The only thing I could not figure out is where to pass in LocalColor.current. I guess it should be a parameter to SvgDecoder:

class SvgDecoder @JvmOverloads constructor(
    private val source: ImageSource,
    private val options: Options,
    val useViewBoundsAsIntrinsicSize: Boolean = true,
    val colorHex: String? = null,
) : Decoder {

But I don't know how SvgDecoder.Factory should get the value. I guess it shouldn't be a param to SvgDecoder.Factory since it's long-lived (?), but added as an extra field in coil.request.Options?

@colinrtwhite
Copy link
Member

@digitalheir Thanks for looking into this! I added an ImageRequest.Builder.css extension function here to support setting any custom css when rendering the SVG. We can't automatically set the color using LocalContentColor.current as the coil-svg module doesn't depend on Compose (and coil-compose doesn't depend on coil-svg).

@digitalheir
Copy link
Author

Thank you! Suppose it's even better like this because it enables the developer to make the call on whether an AsyncImage needs to be recomposed when ContentColor changes. (Or even what to use as currentColor.)

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

Successfully merging a pull request may close this issue.

2 participants