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

Expose an API for converting DrawableResource -> ByteArray #4360

Closed
colinrtwhite opened this issue Feb 24, 2024 · 7 comments · Fixed by #4576 or #4651
Closed

Expose an API for converting DrawableResource -> ByteArray #4360

colinrtwhite opened this issue Feb 24, 2024 · 7 comments · Fixed by #4576 or #4651
Assignees
Labels
enhancement New feature or request resources

Comments

@colinrtwhite
Copy link

Hi I'm looking to add support for the new multiplatform resources API in Coil so it's possible to do:

AsyncImage(
  model = Res.drawable.image,
  contentDescription = null,
)

Using AsyncImage would have a couple benefits vs. using Image and painterResource:

  • Automatic downscaling for large images into small layout bounds.
  • Support for Decoders to decode custom file types.
  • More memory cache control (e.g. control size and use LRU eviction, or disable caching).
  • AsyncImage is a unified API that supports the file system and network URLs too.

At the moment I don't think it's possible to add support as the APIs to fetch a resource (readResourceBytes, Resource.getPathByEnvironment, and ResourceEnvironment) are internal. Res.readBytes is public, but it's generated so it can't easily be used in libraries. Res.readBytes also requires a path, but there's no public method to convert a DrawableResource into a path.

I think exposing LocalComposeEnvironment.current.rememberEnvironment() and resource.getPathByEnvironment(environment) (basically whatever painterResource(resource: DrawableResource) uses) would be enough to support this, but let me know what you think!

@colinrtwhite colinrtwhite added enhancement New feature or request submitted labels Feb 24, 2024
@dima-avdeev-jb
Copy link
Contributor

Thanks! We will discuss it.

@terrakok
Copy link
Collaborator

Hi @colinrtwhite ! I have an idea to add a function getAsUri(Res id). It will return an uri to the file based on current environment. Does it cover your case?

@colinrtwhite
Copy link
Author

colinrtwhite commented Feb 26, 2024

@terrakok Yep, I think that would work! Thinking out loud this is how Coil would use it:

  • The user passes DrawableResource to AsyncImage.
  • AsyncImage uses getAsUri to translate the DrawableResource -> String (or URI type)
  • The URI is passed to the ImageLoader, which can already handle most URI formats (network, disk, Android R, etc.)

@terrakok
Copy link
Collaborator

terrakok commented Apr 4, 2024

Hi @colinrtwhite! I returned to the issue and think there is a problem: when we try to convert a DrawableResource to the Uri string, we do this depending on a current environment. It is a hidden logic which can leak to user's code. The uri string is static and points on concrete file instead of a dynamic accessor with the environment handling.

fun DrawableResource.toUri(): String {
    val resource = this
    val resourceReader = LocalResourceReader.current
    val environment = LocalComposeEnvironment.current.rememberEnvironment()
    val resourceItem = remember(resource, environment) { resource.getResourceItemByEnvironment(environment) }
    return resourceReader.getUri(resourceItem.path)
}

So, I will add the uri getter only for files by path: Res.getUri("files/map.png"). I hope it is enough. All coil's features will work for resources and the resource library API won't be broken.

terrakok added a commit that referenced this issue Apr 5, 2024
Adds a public `Res.getUri(path: String): String` function.
It lets external libraries a way to read resource files by a platform
dependent Uri.
E.g.: video players, image loaders or embedded web browsers.

```kotlin
val uri = Res.getUri("files/my_video.mp4")
```

fixes #4360
@colinrtwhite
Copy link
Author

colinrtwhite commented Apr 12, 2024

@terrakok Thanks for the update. Unfortunately, I don't think this solves the use case as Res is a generated class that Coil, as a library, can't directly reference so we can't call Res.getUri. The underlying method getResourceUri is marked as an internal API so we can't use that either.

Also it sounds like it won't be possible to load the generated DrawableResource entries? I was hoping to be able to support DrawableResource references as I think most users will prefer type safe drawable references vs. string paths. Is it possible to expose the path associated with a DrawableResource? Then that path could be used to get the URI, which can then be loaded by Coil.

@terrakok
Copy link
Collaborator

terrakok commented Apr 12, 2024

I don't say the coil calls Res.getUri. I'd expect a user may pass an uri string to the coil. Will it work?

AsyncImage(
  model = Res.getUri("files/my_photo.jpg"),
  contentDescription = null,
)

Regarding type safe drawable references, could you explain why the following approach doesn't work for you?

AsyncImage(
  model = painterResource(Res.drawable.some_icon),
  contentDescription = null,
)

@terrakok terrakok reopened this Apr 12, 2024
@colinrtwhite
Copy link
Author

colinrtwhite commented Apr 13, 2024

@terrakok Ah sorry - passing model = Res.getUri("files/my_photo.jpg") would work! That API does make it possible to load images given their path. Will it also work for images in the drawable folder: Res.getUri("drawable/my_photo.jpg")?

For model = painterResource(Res.drawable.some_icon) we could support it, but then there isn't any benefit to using AsyncImage over Image. The image is decoded by painterResource so AsyncImage wouldn't be able to downscale the image into the composable's bounds or add support for custom decoders.

terrakok added a commit that referenced this issue Apr 23, 2024
…ources. (#4651)

Implemented two new experimental functions:
```kotlin
/**
 * Retrieves the byte array of the drawable resource.
 *
 * @param environment The resource environment, which can be obtained from [rememberResourceEnvironment] or [getSystemResourceEnvironment].
 * @param resource The drawable resource.
 * @return The byte array representing the drawable resource.
 */
@ExperimentalResourceApi
suspend fun getDrawableResourceBytes(
    environment: ResourceEnvironment,
    resource: DrawableResource
): ByteArray {...}

/**
 * Retrieves the byte array of the font resource.
 *
 * @param environment The resource environment, which can be obtained from [rememberResourceEnvironment] or [getSystemResourceEnvironment].
 * @param resource The font resource.
 * @return The byte array representing the font resource.
 */
@ExperimentalResourceApi
suspend fun getFontResourceBytes(
    environment: ResourceEnvironment,
    resource: FontResource
): ByteArray {...}
```

fixes #4360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request resources
Projects
None yet
3 participants