bumptech / bumptech/glide

[Enhancement] Request to add 'clipToBounds: Boolean = true' parameter to Compose's GlideImage

Open
#5,336 0 comments 0 reactions 0 assignees View on GitHub
Compose
Dominant language
Java
Stars
35k
Forks
6.2k
Avg merge
1d 11h
Merged PRs (30d)
8

Description

An image zoom component [ZoomImage](https://github.com/panpf/zoomimage) that I am developing that supports Compose encountered problems when adapting to GlideImage.

Because ContentScale.None needs to be supported, and the user can also drag to view content that cannot be seen at first, GlideImage needs to add a `clipToBounds: Boolean = true` parameter to control not clip the content.

The final modified location is at [GlideModifier.kt](https://github.com/bumptech/glide/blob/master/integration/compose/src/main/java/com/bumptech/glide/integration/compose/GlideModifier.kt ) at line 324:

```kotlin
private fun ContentDrawScope.drawOne(
painter: Painter?,
cache: CachedPositionAndSize?,
drawOne: DrawScope.(Size) -> Unit
): CachedPositionAndSize? {
...

clipRect {
translate(currentPositionAndSize.position.x, currentPositionAndSize.position.y) {
drawOne.invoke(this, currentPositionAndSize.size)
}
}
return currentPositionAndSize
}
```

The modification is as follows:

```kotlin
private fun ContentDrawScope.drawOne(
painter: Painter?,
cache: CachedPositionAndSize?,
drawOne: DrawScope.(Size) -> Unit
): CachedPositionAndSize? {
...

if (clipToBounds) {
clipRect {
translate(currentPositionAndSize.position.x, currentPositionAndSize.position.y) {
drawOne.invoke(this, currentPositionAndSize.size)
}
}
} else {
translate(currentPositionAndSize.position.x, currentPositionAndSize.position.y) {
drawOne.invoke(this, currentPositionAndSize.size)
}
}
return currentPositionAndSize
}
```

GlideImage needs to add the `clipToBounds: Boolean = true` parameter:

```kotlin
@Composable
public fun GlideImage(
model: Any?,
contentDescription: String?,
modifier: Modifier = Modifier,
alignment: Alignment = Alignment.Center,
contentScale: ContentScale = ContentScale.Fit,
alpha: Float = DefaultAlpha,
colorFilter: ColorFilter? = null,
clipToBounds: Boolean = true,
// TODO(judds): Consider using separate GlideImage* methods instead of sealed classes.
// See http://shortn/_x79pjkMZIH for an internal discussion.
loading: Placeholder? = null,
failure: Placeholder? = null,
transition: Transition.Factory? = null,
// TODO(judds): Consider defaulting to load the model here instead of always doing so below.
requestBuilderTransform: RequestBuilderTransform = { it },
)
```

Please also consider my request!

If can't add parameters to support my request, I will have to clone all the source code of GlideImage and modify the implementation myself, but this will not be able to synchronize the updates of GlideImage in time, resulting in a split situation, which I don't want to see.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.