Apply optional transformations for ContentScales

Using optional trasnformations more closely matches the behavior of
RequestBuilder when loading into Views.

Using non-optional transformations will cause exceptions to be thrown if
the loaded resource type cannot be transformed. In particular any
non-Drawable / non-Bitmap resource will throw as will any Drawable type
that cannot be converted into a simple Bitmap.

Fixes #4943

integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageCustomDrawableTransformationTest.kt

@@ -1,4 +1,100 @@
+@file:OptIn(ExperimentalGlideComposeApi::class, ExperimentalCoroutinesApi::class)
+
 package com.bumptech.glide.integration.compose
 
-class GlideImageCustomDrawableTransformationTest {
-}
+import android.graphics.Canvas
+import android.graphics.ColorFilter
+import android.graphics.drawable.Animatable
+import android.graphics.drawable.Drawable
+import androidx.compose.foundation.layout.size
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.geometry.Size
+import androidx.compose.ui.layout.ContentScale
+import androidx.compose.ui.layout.ScaleFactor
+import androidx.compose.ui.unit.dp
+import com.bumptech.glide.integration.compose.test.Constants
+import com.bumptech.glide.integration.compose.test.GlideComposeRule
+import com.bumptech.glide.integration.compose.test.assertDisplaysInstance
+import com.bumptech.glide.integration.compose.test.onNodeWithDefaultContentDescription
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runTest
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+/**
+ * Tests Issue #4943.
+ *
+ * Transformable types are tested in [GlideImageDefaultTransformationTest].
+ */
+@RunWith(Parameterized::class)
+class GlideImageCustomDrawableTransformationTest(private val contentScale: ContentScale) {
+  @get:Rule val glideComposeRule = GlideComposeRule()
+
+  @Test
+  fun glideImage_nonBitmapDrawable_doesNotThrow() = runTest {
+    val customDrawable = FakeDrawable()
+
+    glideComposeRule.setContent {
+      GlideImage(
+        model = customDrawable,
+        contentScale = contentScale,
+        contentDescription = Constants.DEFAULT_DESCRIPTION,
+        modifier = Modifier.size(100.dp, 200.dp)
+      )
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplaysInstance(customDrawable)
+  }
+
+  @Test
+  fun glideImage_animatableDrawable_doesNotThrow() = runTest {
+    val customDrawable = FakeAnimatableDrawable()
+
+    glideComposeRule.setContent {
+      GlideImage(
+        model = customDrawable,
+        contentScale = contentScale,
+        contentDescription = Constants.DEFAULT_DESCRIPTION,
+        modifier = Modifier.size(200.dp, 100.dp)
+      )
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplaysInstance(customDrawable)
+  }
+
+  companion object {
+    @JvmStatic
+    @Parameterized.Parameters(name = "{0}: contentScale")
+    fun data() = arrayOf(
+      ContentScale.Crop,
+      ContentScale.FillBounds,
+      ContentScale.FillHeight,
+      ContentScale.FillWidth,
+      ContentScale.Fit,
+      ContentScale.Inside,
+      ContentScale.None,
+      object : ContentScale {
+        override fun computeScaleFactor(srcSize: Size, dstSize: Size): ScaleFactor =
+          ContentScale.Fit.computeScaleFactor(srcSize, dstSize)
+      },
+    )
+  }
+}
+
+@Suppress("DeprecatedCallableAddReplaceWith")
+private open class FakeDrawable : Drawable() {
+  override fun draw(p0: Canvas) {
+  }
+  override fun setAlpha(p0: Int) = throw UnsupportedOperationException()
+  override fun setColorFilter(p0: ColorFilter?) = throw UnsupportedOperationException()
+  @Deprecated("Deprecated in Java")
+  override fun getOpacity(): Int = throw UnsupportedOperationException()
+}
+
+private class FakeAnimatableDrawable : FakeDrawable(), Animatable {
+  override fun start() {}
+  override fun stop() {}
+  override fun isRunning(): Boolean = throw UnsupportedOperationException()
+}

integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/GlideImageDefaultTransformationTest.kt

@@ -0,0 +1,183 @@
+@file:OptIn(ExperimentalCoroutinesApi::class, ExperimentGlideFlows::class,
+            ExperimentalGlideComposeApi::class)
+
+package com.bumptech.glide.integration.compose
+
+import android.content.Context
+import android.content.res.Resources
+import android.graphics.drawable.Drawable
+import android.util.TypedValue
+import androidx.annotation.DrawableRes
+import androidx.compose.foundation.layout.size
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.layout.ContentScale
+import androidx.compose.ui.unit.dp
+import androidx.test.core.app.ApplicationProvider
+import com.bumptech.glide.Glide
+import com.bumptech.glide.RequestBuilder
+import com.bumptech.glide.integration.compose.test.Constants
+import com.bumptech.glide.integration.compose.test.GlideComposeRule
+import com.bumptech.glide.integration.compose.test.assertDisplays
+import com.bumptech.glide.integration.compose.test.onNodeWithDefaultContentDescription
+import com.bumptech.glide.integration.ktx.ExperimentGlideFlows
+import com.bumptech.glide.integration.ktx.Resource
+import com.bumptech.glide.integration.ktx.Status
+import com.bumptech.glide.integration.ktx.flow
+import kotlin.math.roundToInt
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.test.runTest
+import org.junit.Rule
+import org.junit.Test
+
+/**
+ * Non-transformable types are tested in [GlideImageCustomDrawableTransformationTest]
+ */
+class GlideImageDefaultTransformationTest {
+  private val context: Context = ApplicationProvider.getApplicationContext()
+  @get:Rule val glideComposeRule = GlideComposeRule()
+
+  @Test
+  fun glideImage_withContentScaleNone_noTransformation_doesNotApplyTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId)
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.None,
+      )
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+  @Test
+  fun glideImage_withContentScaleFit_noTransformation_appliesCenterInsideTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId) { it.centerInside() }
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.Fit,
+      )
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+  @Test
+  fun glideImage_withContentScaleFit_explicitTransformation_usesExplicitTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId) { it.centerCrop() }
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.Fit,
+      ) { it.centerCrop() }
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+  @Test
+  fun glideImage_withContentScaleInside_noTransformation_appliesCenterInsideTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId) { it.centerInside() }
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.Inside,
+      )
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+  @Test
+  fun glideImage_withContentScaleInside_explicitTransformation_usesExplicitTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId) { it.centerCrop() }
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.Inside,
+      ) { it.centerCrop() }
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+
+  @Test
+  fun glideImage_withContentScaleCrop_noTransformation_appliesCenterCropTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId) { it.centerCrop() }
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.Crop,
+      )
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+  @Test
+  fun glideImage_withContentScaleCrop_explicitTransformation_usesExplicitTransformation() = runTest {
+    val resourceId = android.R.drawable.star_big_on
+    val expectedDrawable = loadExpectedDrawable(resourceId) { it.centerInside() }
+
+    glideComposeRule.setContent {
+      ContentScaleGlideImage(
+        model = resourceId,
+        contentScale = ContentScale.Crop,
+      ) { it.centerInside() }
+    }
+
+    glideComposeRule.onNodeWithDefaultContentDescription().assertDisplays(expectedDrawable)
+  }
+
+
+  private suspend fun RequestBuilder<Drawable>.loadRequiringSuccess() =
+    (this.flow().first { it.status == Status.SUCCEEDED} as Resource<Drawable>).resource
+
+  private suspend fun loadExpectedDrawable(
+    @DrawableRes resourceId: Int,
+    transformation: (RequestBuilder<Drawable>) -> RequestBuilder<Drawable> = {it -> it},
+  ): Drawable =
+    transformation(Glide.with(context).load(resourceId).override(WIDTH.px(), HEIGHT.px()))
+      .loadRequiringSuccess()
+
+  @Composable
+  private fun ContentScaleGlideImage(
+    model: Any?,
+    contentScale: ContentScale,
+    requestBuilderTransform: RequestBuilderTransform<Drawable> = {it -> it},
+  ) =
+    GlideImage(
+      model = model,
+      contentDescription = Constants.DEFAULT_DESCRIPTION,
+      modifier = SIZE_MODIFIER,
+      contentScale = contentScale,
+      requestBuilderTransform = requestBuilderTransform,
+    )
+
+  companion object {
+    const val WIDTH = 25
+    // non-square
+    const val HEIGHT = 30
+
+    val SIZE_MODIFIER = Modifier.size(WIDTH.dp, HEIGHT.dp)
+  }
+}
+
+fun Int.px() =
+  TypedValue.applyDimension(
+    TypedValue.COMPLEX_UNIT_DIP, this.toFloat(), Resources.getSystem().displayMetrics).roundToInt()

integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/expectations.kt

@@ -63,3 +63,13 @@ private fun <ValueT, TransformedValueT> expectStateValue(
     }
     true
   }
+
+fun expectSameInstance(expectedDrawable: Drawable) =
+  SemanticsMatcher("${DisplayedDrawableKey.name} = '$expectedDrawable'") {
+    val actualValue: Drawable? = it.config.getOrElseNullable(DisplayedDrawableKey) { null }?.value
+    if (actualValue !== expectedDrawable) {
+      throw AssertionError("Expected: $expectedDrawable, but was: $actualValue")
+    }
+    true
+  }
+

integration/compose/src/androidTest/java/com/bumptech/glide/integration/compose/test/nodes.kt

@@ -1,2 +1,20 @@
 package com.bumptech.glide.integration.compose.test
 
+import android.graphics.drawable.Drawable
+import androidx.compose.ui.test.SemanticsNodeInteraction
+import androidx.compose.ui.test.assert
+import androidx.compose.ui.test.junit4.ComposeContentTestRule
+import androidx.compose.ui.test.onNodeWithContentDescription
+
+object Constants {
+  const val DEFAULT_DESCRIPTION = "test"
+}
+
+fun ComposeContentTestRule.onNodeWithDefaultContentDescription() =
+  onNodeWithContentDescription(Constants.DEFAULT_DESCRIPTION)
+
+fun SemanticsNodeInteraction.assertDisplays(drawable: Drawable) =
+  assert(expectDisplayedDrawable(drawable))
+
+fun SemanticsNodeInteraction.assertDisplaysInstance(drawable: Drawable) =
+  assert(expectSameInstance(drawable))

integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideImage.kt

@@ -232,15 +232,15 @@ private fun RequestBuilder<Drawable>.contentScaleTransform(
 ): RequestBuilder<Drawable> {
   return when (contentScale) {
     ContentScale.Crop -> {
-      centerCrop()
+      optionalCenterCrop()
     }
     ContentScale.Inside,
     ContentScale.Fit -> {
       // Outside compose, glide would use fitCenter() for FIT. But that's probably not a good
       // decision given how unimportant Bitmap re-use is relative to minimizing texture sizes now.
       // So instead we'll do something different and prefer not to upscale, which means using
       // centerInside(). The UI can still scale the view even if the Bitmap is smaller.
-      centerInside()
+      optionalCenterInside()
     }
     else -> {
       this