saket / telephoto

Building blocks for designing media experiences in Compose UI
https://saket.github.io/telephoto/
Apache License 2.0
996 stars 32 forks source link

[Improvement] Supports loading file URI #19

Closed ganfra closed 1 year ago

ganfra commented 1 year ago

So, we are trying to use telephoto to render some images from internal storage.

Basically we were just using this with Coil, and it was working correctly :

 AsyncImage(
        modifier = Modifier.fillMaxSize(),
        model = uri,
        contentDescription = "MediaImage"
    )

And now we use the same way :

 ZoomableAsyncImage(
        modifier = Modifier.fillMaxSize(),
        model = uri,
        contentDescription = "MediaImage"
    )

The uri has this form : /data/user/0/appId/cache/.tmpy7ImGl.jpeg

After some investigation I found that this code is failing :

try {
        decoder.value = factory.create(context, imageSource, imageOptions)
      } catch (e: IOException) {
        errorReporter.onImageLoadingFailed(e, imageSource)
      }

Where the imageSource is coming from this method :

private fun ImageResult.toSubSamplingImageSource(imageLoader: ImageLoader): SubSamplingImageSource? {
    val result = this
    val requestData = result.request.data
    val preview = (result.drawable as? BitmapDrawable)?.bitmap?.asImageBitmap()

    if (result is SuccessResult && result.drawable is BitmapDrawable) {
      // Prefer reading of images directly from files whenever possible because
      // that is significantly faster than reading from their input streams.
      val imageSource = when {
        result.diskCacheKey != null -> {
          val diskCache = imageLoader.diskCache!!
          val cached = diskCache[result.diskCacheKey!!] ?: error("Coil returned a null image from disk cache")
          SubSamplingImageSource.file(cached.data, preview)
        }
        result.dataSource.let { it == DataSource.DISK || it == DataSource.MEMORY_CACHE } -> when {
          requestData is Uri -> SubSamplingImageSource.contentUri(requestData, preview)
          requestData is String -> SubSamplingImageSource.contentUri(Uri.parse(requestData), preview)
          result.request.context.isResourceId(requestData) -> SubSamplingImageSource.resource(requestData, preview)
          else -> null
        }
        else -> null
      }

      if (imageSource != null) {
        return imageSource
      }
    }

    return null
  }

So I guess we are missing some check on the RequestData.Uri so it can be used with SubSamplingImageSource.file() instead.

Coil is doing some mapping internally like that :

internal class FileUriMapper : Mapper<Uri, File> {

    override fun map(data: Uri, options: Options): File? {
        if (!isApplicable(data)) return null
        if (data.scheme == SCHEME_FILE) {
            return data.path?.let(::File)
        } else {
            // If the scheme is not "file", it's null, representing a literal path on disk.
            // Assume the entire input, regardless of any reserved characters, is valid.
            return File(data.toString())
        }
    }

    private fun isApplicable(data: Uri): Boolean {
        return !isAssetUri(data) &&
            data.scheme.let { it == null || it == SCHEME_FILE } &&
            data.path.orEmpty().startsWith('/') && data.firstPathSegment != null
    }
}

Let me know if this is something you want to fix or if I've to do the conversion on my side. Thanks!

saket commented 1 year ago

Hi @ganfra. Can you share the full error stacktrace?

Can you also check what happens if you use SubSamplingImage() directly with your URI?

val zoomableState = rememberZoomableState()
val imageState = rememberSubSamplingImageState(
  zoomableState = zoomableState,
  imageSource = ImageSource.contentUri(…)
)

SubSamplingImage(
  modifier = Modifier
    .fillMaxSize()
    .zoomable(zoomableState),
  state = imageState,
  contentDescription = …,
)
ganfra commented 1 year ago

Hi, sorry for the late reply. Here is the stacktrace :

0 = {StackTraceElement@30479} "android.content.ContentResolver.openTypedAssetFileDescriptor(ContentResolver.java:2013)"
1 = {StackTraceElement@30480} "android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:1842)"
2 = {StackTraceElement@30481} "android.content.ContentResolver.openInputStream(ContentResolver.java:1518)"
3 = {StackTraceElement@30482} "me.saket.telephoto.subsamplingimage.UriImageSource.decoder(SubSamplingImageSource.kt:136)"
4 = {StackTraceElement@30483} "me.saket.telephoto.subsamplingimage.internal.AndroidImageRegionDecoder$Companion$Factory$1.create(AndroidImageRegionDecoder.kt:47)"
5 = {StackTraceElement@30484} "me.saket.telephoto.subsamplingimage.internal.PooledImageRegionDecoder$Companion$Factory$1$create$decoders$1.invokeSuspend(PooledImageRegionDecoder.kt:52)"
6 = {StackTraceElement@30485} "kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)"
7 = {StackTraceElement@30486} "kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)"
8 = {StackTraceElement@30487} "kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)"
9 = {StackTraceElement@30488} "kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)"
10 = {StackTraceElement@30489} "kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)"
11 = {StackTraceElement@30490} "kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)"

And yes, it fails the same way using directly SubSamplingImage

And this is expected, because ContentResolver.openInputStream only works with those defined schemes:

 * <h5>Accepts the following URI schemes:</h5>
     * <ul>
     * <li>content ({@link #SCHEME_CONTENT})</li>
     * <li>android.resource ({@link #SCHEME_ANDROID_RESOURCE})</li>
     * <li>file ({@link #SCHEME_FILE})</li>

The URI I provide doesn't contain any scheme. Like I said, Coil fixes this with his FileUriMapper.

saket commented 1 year ago

Gotcha. I don't mind supporting this, but is there a reason you're not specifying the file scheme? What's the benefit of writing "/path/to/file.jpg" instead of "file:///path/to/file.jpg"?

ganfra commented 1 year ago

I can also try to convert my path to a uri at some point instead, of course. It was more a question about having "parity" treatment with regular AsyncImage, but it's up to you :P I let you close the issue.

saket commented 1 year ago

Yea that's a fair point. Regardless of their validity, I will probably have to support them if ZoomableAsyncImage aims to be a drop-in. Let me sleep on this.