android / android-ktx

A set of Kotlin extensions for Android app development.
https://android.github.io/android-ktx/core-ktx/
7.47k stars 563 forks source link

Adding extensions for ImageDecoder #540

Closed RamV13 closed 6 years ago

RamV13 commented 6 years ago

These extensions add decodeBitmap and decodeDrawable functions to the ImageDecoder API for decoding ImageDecoder.Source instances. Further, the decoder instance is now passed as the this to the lambda to enable calling ImageDecoder functions more seamlessly.

Before

val src = ImageDecoder.createSource(/* ... */)
val b = ImageDecoder.decodeBitmap(src) { decoder, info, source ->
    decoder.setTargetSize(width, height)
    decoder.setTargetColorSpace(ColorSpace.get(ColorSpace.Named.SRGB))
    decoder.setPostProcessor { canvas ->
       canvas.drawTest(/* ... */)
       PixelFormat.UNKNOWN
    }
}

After

val src = ImageDecoder.createSource(/* ... */)
val b = src.decodeBitmap { info, source ->
    setTargetSize(width, height)
    setTargetColorSpace(ColorSpace.get(ColorSpace.Named.SRGB))
    setPostProcessor { canvas ->
       canvas.drawTest(/* ... */)
       PixelFormat.UNKNOWN
    }
}

Code snippet adapted from I/O 2018 (https://youtu.be/eMHsnvhcf78?t=1695)

romainguy commented 6 years ago

This is quite different from the conversion functions defined on Bitmap and Color. This is not a conversion, but an expensive decode. All you are effectively saving is the ImageDecoder. prefix in the original call (and admittedly having to use the decoder receiver inside the lambda). If the extension was called decode instead of toBitmap I'd be more inclined to accept it.

RamV13 commented 6 years ago

That makes sense. What would be a better name then, because decodeDrawable and decodeBitmap wouldn't clearly indicate that it's decoding to that type whereas decodeToDrawable and decodeToBitmap might be too verbose?

RamV13 commented 6 years ago

Given the wording in the docs for ImageDecoder.Source, I think it's reasonable to name the functions decodeBitmap and decodeDrawable. Updated the PR.

romainguy commented 6 years ago

Ran tests locally, everything's green, thanks for the contribution!