Skip to content

Commit

Permalink
Fix decoding high colour depth images. (coil-kt#358)
Browse files Browse the repository at this point in the history 
* Fix decoding high colour depth images.

* Update API.

* Fix test.

* Refactor.

* Fix test.

* Fix tests.

* Rename tests.
  • Loading branch information
@colinrtwhite
colinrtwhite authored Apr 11, 2020
1 parent b58385d commit 6224a83
Show file tree
Hide file tree
Showing 19 changed files with 263 additions and 235 deletions.
1 change: 1 addition & 0 deletions coil-base/api/coil-base.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ public final class coil/decode/DecodeResult {
public final class coil/decode/DecodeUtils {
public static final field INSTANCE Lcoil/decode/DecodeUtils;
public static final fun calculateInSampleSize (IIIILcoil/size/Scale;)I
public static final fun computeOutputSize (IILcoil/size/Size;Lcoil/size/Scale;)Lcoil/size/PixelSize;
public static final fun computeSizeMultiplier (DDDDLcoil/size/Scale;)D
public static final fun computeSizeMultiplier (FFFFLcoil/size/Scale;)F
public static final fun computeSizeMultiplier (IIIILcoil/size/Scale;)D
Expand Down
171 changes: 104 additions & 67 deletions coil-base/src/androidTest/java/coil/decode/BitmapFactoryDecoderTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import android.content.Context
import android.graphics.Bitmap
import android.graphics.drawable.BitmapDrawable
import android.os.Build.VERSION.SDK_INT
import androidx.core.graphics.createBitmap
import androidx.test.core.app.ApplicationProvider
import coil.bitmappool.BitmapPool
import coil.size.OriginalSize
Expand All @@ -16,6 +17,7 @@ import coil.util.size
import kotlinx.coroutines.runBlocking
import okio.buffer
import okio.source
import org.junit.Assume.assumeTrue
import org.junit.Before
import org.junit.Test
import kotlin.test.assertEquals
Expand All @@ -37,151 +39,186 @@ class BitmapFactoryDecoderTest {

@Test
fun basic() {
val source = context.assets.open("normal.jpg").source().buffer()
val (drawable, isSampled) = runBlocking {
service.decode(
pool = pool,
source = source,
size = PixelSize(100, 100),
options = createOptions()
)
}
val (drawable, isSampled) = decode(
assetName = "normal.jpg",
size = PixelSize(100, 100)
)

val exception = assertFailsWith<IllegalStateException> { source.exhausted() }
assertEquals("closed", exception.message)
assertTrue(isSampled)
assertTrue(drawable is BitmapDrawable)
assertEquals(PixelSize(100, 125), drawable.bitmap.size)
assertEquals(drawable.bitmap.config, Bitmap.Config.ARGB_8888)
assertEquals(Bitmap.Config.ARGB_8888, drawable.bitmap.config)
}

@Test
fun malformedImageThrows() {
assertFailsWith<IllegalStateException> {
runBlocking {
service.decode(
pool = pool,
source = context.assets.open("malformed.jpg").source().buffer(),
size = PixelSize(100, 100),
options = createOptions()
)
}
decode(
assetName = "malformed.jpg",
size = PixelSize(100, 100)
)
}
}

@Test
fun resultIsSampledIfGreaterThanHalfSize() {
val (drawable, isSampled) = runBlocking {
service.decode(
pool = pool,
source = context.assets.open("normal.jpg").source().buffer(),
size = PixelSize(600, 600),
options = createOptions()
)
}
val (drawable, isSampled) = decode(
assetName = "normal.jpg",
size = PixelSize(600, 600)
)

assertTrue(isSampled)
assertTrue(drawable is BitmapDrawable)
assertEquals(PixelSize(600, 750), drawable.bitmap.size)
}

@Test
fun originalSizeDimensionsAreResolvedCorrectly() {
val size = OriginalSize
val normal = decode("normal.jpg", size)
val normal = decodeBitmap("normal.jpg", size)
assertEquals(PixelSize(1080, 1350), normal.size)
}

@Test
fun exifTransformationsAreAppliedCorrectly() {
val size = PixelSize(500, 500)
val normal = decode("normal.jpg", size)
val normal = decodeBitmap("normal.jpg", size)

for (index in 1..8) {
val other = decode("exif/$index.jpg", size)
val other = decodeBitmap("exif/$index.jpg", size)
assertTrue(normal.isSimilarTo(other), "Image with index $index is incorrect.")
}
}

@Test
fun largeExifMetadata() {
val size = PixelSize(500, 500)
val normal = decode("exif/large_metadata_normalized.jpg", size)
val actual = decode("exif/large_metadata_normalized.jpg", size)
val normal = decodeBitmap("exif/large_metadata_normalized.jpg", size)
val actual = decodeBitmap("exif/large_metadata_normalized.jpg", size)
assertTrue(normal.isSimilarTo(actual))
}

@Test
fun allowInexactSize_True() {
val result = decode(
fileName = "normal.jpg",
fun allowInexactSize_true() {
val result = decodeBitmap(
assetName = "normal.jpg",
size = PixelSize(1500, 1500),
options = { createOptions(scale = Scale.FIT, allowInexactSize = true) }
options = createOptions(scale = Scale.FIT, allowInexactSize = true)
)
assertEquals(PixelSize(1080, 1350), result.size)
}

@Test
fun allowInexactSize_False() {
val result = decode(
fileName = "normal.jpg",
fun allowInexactSize_false() {
val result = decodeBitmap(
assetName = "normal.jpg",
size = PixelSize(1500, 1500),
options = { createOptions(scale = Scale.FIT, allowInexactSize = false) }
options = createOptions(scale = Scale.FIT, allowInexactSize = false)
)
assertEquals(PixelSize(1200, 1500), result.size)
}

@Test
fun allowRgb565_true() {
val result = decodeBitmap(
assetName = "normal.jpg",
size = PixelSize(500, 500),
options = createOptions(allowRgb565 = true)
)
assertEquals(PixelSize(500, 625), result.size)
assertEquals(Bitmap.Config.RGB_565, result.config)
}

@Test
fun allowRgb565_false() {
val result = decodeBitmap(
assetName = "normal.jpg",
size = PixelSize(500, 500),
options = createOptions(allowRgb565 = false)
)
assertEquals(PixelSize(500, 625), result.size)
assertEquals(Bitmap.Config.ARGB_8888, result.config)
}

@Test
fun pooledBitmap_exactSize() {
val pooledBitmap = Bitmap.createBitmap(1080, 1350, Bitmap.Config.ARGB_8888)
val pooledBitmap = createBitmap(1080, 1350, Bitmap.Config.ARGB_8888)
pool.put(pooledBitmap)

val result = decode(
fileName = "normal.jpg",
val result = decodeBitmap(
assetName = "normal.jpg",
size = PixelSize(1080, 1350),
options = {
createOptions(
config = Bitmap.Config.ARGB_8888,
scale = Scale.FIT,
allowInexactSize = false
)
}
options = createOptions(
config = Bitmap.Config.ARGB_8888,
scale = Scale.FIT,
allowInexactSize = false
)
)
assertEquals(PixelSize(1080, 1350), result.size)
assertEquals(pooledBitmap, result)
}

@Test
fun pooledBitmap_inexactSize() {
val pooledBitmap = Bitmap.createBitmap(1080, 1350, Bitmap.Config.ARGB_8888)
val pooledBitmap = createBitmap(1080, 1350, Bitmap.Config.ARGB_8888)
pool.put(pooledBitmap)

val result = decode(
fileName = "normal.jpg",
val result = decodeBitmap(
assetName = "normal.jpg",
size = PixelSize(500, 500),
options = {
createOptions(
config = Bitmap.Config.ARGB_8888,
scale = Scale.FILL,
allowInexactSize = false
)
}
options = createOptions(
config = Bitmap.Config.ARGB_8888,
scale = Scale.FILL,
allowInexactSize = false
)
)
assertEquals(PixelSize(500, 625), result.size)
// The bitmap should not be re-used on pre-API 19.
assertEquals(pooledBitmap === result, SDK_INT >= 19)
}

@Test
fun png_16bit() {
// The emulator runs out of memory while decoding 16_bit.png on pre-23.
assumeTrue(SDK_INT >= 23)

val (drawable, isSampled) = decode(
assetName = "16_bit.png",
size = PixelSize(250, 250),
options = createOptions()
)

assertTrue(isSampled)
assertTrue(drawable is BitmapDrawable)
assertEquals(PixelSize(250, 250), drawable.bitmap.size)

val expectedConfig = if (SDK_INT >= 26) Bitmap.Config.RGBA_F16 else Bitmap.Config.ARGB_8888
assertEquals(expectedConfig, drawable.bitmap.config)
}

private fun decode(
fileName: String,
assetName: String,
size: Size,
options: () -> Options = { createOptions() }
): Bitmap = runBlocking {
options: Options = createOptions()
): DecodeResult = runBlocking {
val source = context.assets.open(assetName).source().buffer()
val result = service.decode(
pool = pool,
source = context.assets.open(fileName).source().buffer(),
source = source,
size = size,
options = options()
options = options
)
return@runBlocking (result.drawable as BitmapDrawable).bitmap

// Assert that the source has been closed.
val exception = assertFailsWith<IllegalStateException> { source.exhausted() }
assertEquals("closed", exception.message)

return@runBlocking result
}

private fun decodeBitmap(
assetName: String,
size: Size,
options: Options = createOptions()
): Bitmap = (decode(assetName, size, options).drawable as BitmapDrawable).bitmap
}
19 changes: 13 additions & 6 deletions coil-base/src/main/java/coil/RealImageLoader.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal class RealImageLoader(

private val delegateService = DelegateService(this, referenceCounter, logger)
private val requestService = RequestService(defaults, logger)
private val memoryCacheService = MemoryCacheService(requestService, defaults, logger)
private val memoryCacheService = MemoryCacheService(requestService, logger)
private val drawableDecoder = DrawableDecoderService(bitmapPool)
private val networkObserver = NetworkObserver(context, logger)

Expand Down Expand Up @@ -389,18 +389,25 @@ internal class RealImageLoader(
return@run result
}

// Convert the drawable into a bitmap.
// Convert the drawable into a bitmap with a valid config.
eventListener.transformStart(request)
val baseBitmap = if (result.drawable is BitmapDrawable) {
result.drawable.bitmap
val resultBitmap = result.drawable.bitmap
if (resultBitmap.safeConfig in RequestService.VALID_TRANSFORMATION_CONFIGS) {
resultBitmap
} else {
logger?.log(TAG, Log.INFO) {
"Converting bitmap with config ${resultBitmap.safeConfig} to apply transformations: $transformations"
}
drawableDecoder.convert(result.drawable, options.config, size, options.scale, options.allowInexactSize)
}
} else {
logger?.log(TAG, Log.INFO) {
"Converting drawable of type ${result.drawable::class.java.canonicalName} " +
"to apply transformations: $transformations"
}
drawableDecoder.convert(result.drawable, size, options.config)
drawableDecoder.convert(result.drawable, options.config, size, options.scale, options.allowInexactSize)
}

val transformedBitmap = transformations.foldIndices(baseBitmap) { bitmap, transformation ->
transformation.transform(bitmapPool, bitmap, size).also { ensureActive() }
}
Expand Down Expand Up @@ -451,7 +458,7 @@ internal class RealImageLoader(
private var size: Size? = null

/**
* Call [SizeResolver.size] and cache the result.
* Calls [TargetDelegate.start], [SizeResolver.size], and caches the resolved size.
*
* This method is inlined as long as it is called from inside [RealImageLoader].
* [beforeResolveSize] and [afterResolveSize] are outlined to reduce the amount of inlined code.
Expand Down
40 changes: 28 additions & 12 deletions coil-base/src/main/java/coil/decode/BitmapFactoryDecoder.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ internal class BitmapFactoryDecoder(private val context: Context) : Decoder {
val srcWidth = if (isSwapped) outHeight else outWidth
val srcHeight = if (isSwapped) outWidth else outHeight

// Disable hardware bitmaps if we need to perform EXIF transformations.
val safeConfig = if (isFlipped || isRotated) options.config.toSoftware() else options.config
inPreferredConfig = if (allowRgb565(options.allowRgb565, safeConfig, outMimeType)) Bitmap.Config.RGB_565 else safeConfig
inPreferredConfig = computeConfig(options, isFlipped, isRotated)

if (SDK_INT >= 26 && options.colorSpace != null) {
inPreferredColorSpace = options.colorSpace
Expand All @@ -84,7 +82,7 @@ internal class BitmapFactoryDecoder(private val context: Context) : Decoder {
inScaled = false

if (inMutable) {
inBitmap = pool.getDirtyOrNull(outWidth, outHeight, inPreferredConfig)
inBitmap = pool.getDirty(outWidth, outHeight, inPreferredConfig)
}
}
else -> {
Expand Down Expand Up @@ -120,7 +118,7 @@ internal class BitmapFactoryDecoder(private val context: Context) : Decoder {
inBitmap = when {
// If we're not scaling the image, use the image's source dimensions.
inSampleSize == 1 && !inScaled -> {
pool.getDirtyOrNull(outWidth, outHeight, inPreferredConfig)
pool.getDirty(outWidth, outHeight, inPreferredConfig)
}
// We can only re-use bitmaps that don't match the image's source dimensions on API 19 and above.
SDK_INT >= 19 -> {
Expand Down Expand Up @@ -165,13 +163,31 @@ internal class BitmapFactoryDecoder(private val context: Context) : Decoder {
)
}

/** TODO: Peek the source to figure out its format (and if it has alpha) instead of relying on the MIME type. */
private fun allowRgb565(
allowRgb565: Boolean,
config: Bitmap.Config,
mimeType: String?
): Boolean {
return allowRgb565 && (SDK_INT < 26 || config == Bitmap.Config.ARGB_8888) && mimeType == MIME_TYPE_JPEG
/** Compute and return [BitmapFactory.Options.inPreferredConfig]. */
private fun BitmapFactory.Options.computeConfig(
options: Options,
isFlipped: Boolean,
isRotated: Boolean
): Bitmap.Config {
var config = options.config

// Disable hardware bitmaps if we need to perform EXIF transformations.
if (isFlipped || isRotated) {
config = config.toSoftware()
}

// Decode the image as RGB_565 as an optimization if allowed.
// TODO: Peek the source to figure out its format (and if it has alpha) instead of relying on the MIME type.
if (options.allowRgb565 && config == Bitmap.Config.ARGB_8888 && outMimeType == MIME_TYPE_JPEG) {
config = Bitmap.Config.RGB_565
}

// High color depth images must be decoded as either RGBA_F16 or HARDWARE.
if (SDK_INT >= 26 && outConfig == Bitmap.Config.RGBA_F16 && config != Bitmap.Config.HARDWARE) {
config = Bitmap.Config.RGBA_F16
}

return config
}

/** NOTE: This method assumes [config] is not [Bitmap.Config.HARDWARE] if the image has to be transformed. */
Expand Down
Loading

0 comments on commit 6224a83

Please sign in to comment.