Skip to content

Commit

Permalink
Make PicassoPainter's request aware of state read changes
Browse files Browse the repository at this point in the history
  • Loading branch information
gamepro65 committed Sep 11, 2023
1 parent 8dd1f53 commit 200d733
Show file tree
Hide file tree
Showing 6 changed files with 315 additions and 7 deletions.
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ composeRuntime = { module = 'androidx.compose.runtime:runtime', version.ref = 'c
composeUi-foundation = { module = 'androidx.compose.foundation:foundation', version.ref = 'composeUi' }
composeUi-material = { module = 'androidx.compose.material:material', version.ref = 'composeUi' }
composeUi-uiTooling = { module = 'androidx.compose.ui:ui-tooling', version.ref = 'composeUi' }
composeUi-test = { module = 'androidx.compose.ui:ui-test-junit4', version.ref = 'composeUi' }
composeUi-testManifest = { module = 'androidx.compose.ui:ui-test-manifest', version.ref = 'composeUi' }

drawablePainter = { module = 'com.google.accompanist:accompanist-drawablepainter', version = '0.30.1' }

Expand Down
8 changes: 8 additions & 0 deletions picasso-compose/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ android {

defaultConfig {
minSdkVersion libs.versions.minSdk.get() as int

testInstrumentationRunner 'androidx.test.runner.AndroidJUnitRunner'
}

buildFeatures {
Expand Down Expand Up @@ -41,6 +43,12 @@ dependencies {
implementation libs.drawablePainter
implementation libs.composeUi
implementation libs.composeUi.foundation
implementation libs.composeRuntime

debugImplementation libs.composeUi.testManifest

androidTestImplementation libs.composeUi.test
androidTestImplementation libs.truth

compileOnly libs.androidx.annotations
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
/*
* Copyright (C) 2013 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.picasso3.compose

import android.graphics.Bitmap
import android.graphics.Bitmap.Config.ARGB_8888
import androidx.compose.foundation.Canvas
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.requiredSize
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.layout.onSizeChanged
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.dp
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import com.google.common.truth.Truth.assertThat
import com.squareup.picasso3.Picasso
import com.squareup.picasso3.Picasso.LoadedFrom
import com.squareup.picasso3.Request
import com.squareup.picasso3.RequestHandler
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import kotlinx.coroutines.Dispatchers

@RunWith(AndroidJUnit4::class)
class PicassoPainterTest {

@get:Rule
val rule = createComposeRule()

@Test
fun firstFrameConsumesStateFromLayout() {
lateinit var lastRequest: Request
val context = InstrumentationRegistry.getInstrumentation().targetContext

val picasso = Picasso.Builder(context)
.callFactory { throw RuntimeException() }
.dispatchers(Dispatchers.Unconfined, Dispatchers.Unconfined)
.addRequestHandler(object : RequestHandler() {
override fun canHandleRequest(data: Request): Boolean = true
override fun load(picasso: Picasso, request: Request, callback: Callback) {
lastRequest = request
callback.onSuccess(Result.Bitmap(Bitmap.createBitmap(1, 1, ARGB_8888), LoadedFrom.MEMORY))
}
})
.build()
var size: IntSize by mutableStateOf(IntSize.Zero)
var drawn = false

rule.setContent {
CompositionLocalProvider(LocalDensity provides Density(1f)) {
val painter = picasso.rememberPainter {
it.load("http://example.com/")
// Headers are not part of a cache key, using a stable key to break cache
.stableKey("http://example.com/$size")
.addHeader("width", size.width.toString())
.addHeader("height", size.height.toString())
}
Canvas(
Modifier
.requiredSize(9.dp)
.onSizeChanged { size = it }
) {
val canvasSize = this.size

with(painter) {
draw(canvasSize)
}
drawn = true
}
}
}

rule.waitUntil { drawn }

// Draw triggers request was made with size.
assertThat(lastRequest.headers?.toMultimap()).containsAtLeastEntriesIn(
mapOf(
"width" to listOf("9"),
"height" to listOf("9")
)
)
}

@Test
fun redrawDoesNotReexecuteUnchangedRequest() {
var requestCount = 0
val context = InstrumentationRegistry.getInstrumentation().targetContext
val picasso = Picasso.Builder(context)
.callFactory { throw RuntimeException() }
.dispatchers(Dispatchers.Unconfined, Dispatchers.Unconfined)
.addRequestHandler(object : RequestHandler() {
override fun canHandleRequest(data: Request): Boolean = true
override fun load(picasso: Picasso, request: Request, callback: Callback) {
requestCount++
callback.onSuccess(Result.Bitmap(Bitmap.createBitmap(1, 1, ARGB_8888), LoadedFrom.MEMORY))
}
})
.build()

var drawInvalidator by mutableStateOf(0)
var drawCount = 0
rule.setContent {
CompositionLocalProvider(LocalDensity provides Density(1f)) {
val painter = picasso.rememberPainter {
it.load("http://example.com/")
}
Canvas(Modifier.fillMaxSize()) {
drawCount++
drawInvalidator = 1

val canvasSize = this.size
with(painter) {
draw(canvasSize)
}
}
}
}

rule.waitUntil { drawCount == 2 }
assertThat(requestCount).isEqualTo(1)
}

@Test
fun newRequestLoaded_whenRequestDependenciesChangedAfterFirstFrame() {
lateinit var lastRequest: Request
val context = InstrumentationRegistry.getInstrumentation().targetContext
val picasso = Picasso.Builder(context)
.callFactory { throw RuntimeException() }
.dispatchers(Dispatchers.Unconfined, Dispatchers.Unconfined)
.addRequestHandler(object : RequestHandler() {
override fun canHandleRequest(data: Request): Boolean = true
override fun load(picasso: Picasso, request: Request, callback: Callback) {
lastRequest = request
callback.onSuccess(Result.Bitmap(Bitmap.createBitmap(1, 1, ARGB_8888), LoadedFrom.MEMORY))
}
})
.build()
var testHeader by mutableStateOf("one")

rule.setContent {
CompositionLocalProvider(LocalDensity provides Density(1f)) {
val painter = picasso.rememberPainter {
it.load("http://example.com/")
// Headers are not part of a cache key, using a stable key to break cache
.stableKey("http://example.com/$testHeader")
.addHeader("testHeader", testHeader)
}
Canvas(Modifier.fillMaxSize()) {
val canvasSize = this.size

with(painter) {
draw(canvasSize)
}
}
}
}

rule.runOnIdle {
assertThat(lastRequest.headers?.get("testHeader")).isEqualTo("one")
}

var currentRequest = lastRequest
testHeader = "two"

// On API 21 runOnIdle runs before the composition recomposes :-(
// Waiting until the request updates, then asserting
rule.waitUntil { currentRequest != lastRequest }
assertThat(lastRequest.headers?.get("testHeader")).isEqualTo("two")

currentRequest = lastRequest
testHeader = "three"

rule.waitUntil { currentRequest != lastRequest }
assertThat(lastRequest.headers?.get("testHeader")).isEqualTo("three")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ package com.squareup.picasso3.compose
import android.graphics.drawable.Drawable
import androidx.compose.runtime.Composable
import androidx.compose.runtime.RememberObserver
import androidx.compose.runtime.State
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshots.Snapshot
import androidx.compose.ui.geometry.Size
import androidx.compose.ui.graphics.ColorFilter
import androidx.compose.ui.graphics.DefaultAlpha
Expand All @@ -48,12 +51,19 @@ internal class PicassoPainter(
private val onError: ((Exception) -> Unit)? = null
) : Painter(), RememberObserver, DrawableTarget {

private var lastRequestCreator: RequestCreator? by mutableStateOf(null)
private val requestCreator: State<RequestCreator> = derivedStateOf { request(picasso) }
private var painter: Painter by mutableStateOf(EmptyPainter)
private var alpha: Float by mutableStateOf(DefaultAlpha)
private var colorFilter: ColorFilter? by mutableStateOf(null)

override val intrinsicSize: Size
get() = painter.intrinsicSize
get() {
// Make sure we're using the latest request. If the request function reads any state, it will
// invalidate whatever scope this property is being read from.
load()
return painter.intrinsicSize
}

override fun applyAlpha(alpha: Float): Boolean {
this.alpha = alpha
Expand All @@ -66,13 +76,20 @@ internal class PicassoPainter(
}

override fun DrawScope.onDraw() {
// Make sure we're using the latest request. If the request function reads any state, it will
// invalidate this draw scope when it changes.
load()
with(painter) {
draw(size, alpha, colorFilter)
}
}

override fun onRemembered() {
request.invoke(picasso).into(this)
// This is called from composition, but if the request provider function reads any state we
// don't want that to invalidate composition. It will invalidate draw, later.
Snapshot.withoutReadObservation {
load()
}
}

override fun onAbandoned() {
Expand Down Expand Up @@ -100,6 +117,20 @@ internal class PicassoPainter(
errorDrawable?.let(::setPainter)
}

private fun load() {
// This derived state read will return the same instance of RequestCreator if one has been
// cached and none of the state dependencies have since changed.
val requestCreator = requestCreator.value
// lastRequestCreator is just used for diffing, we don't want it to invalidate anything.
val lastRequestCreator = Snapshot.withoutReadObservation { lastRequestCreator }

// Only launch a new request if anything has actually changed.
if (requestCreator != lastRequestCreator) {
this.lastRequestCreator = requestCreator
requestCreator.into(this)
}
}

private fun setPainter(drawable: Drawable) {
(painter as? RememberObserver)?.onForgotten()
painter = DrawablePainter(drawable).apply(DrawablePainter::onRemembered)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ import android.content.Context
import android.net.NetworkInfo
import android.os.Handler
import com.squareup.picasso3.Dispatcher.Companion.RETRY_DELAY
import com.squareup.picasso3.Picasso.Priority.HIGH
import com.squareup.picasso3.Utils.OWNER_DISPATCHER
import com.squareup.picasso3.Utils.VERB_CANCELED
import com.squareup.picasso3.Utils.log
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext
import kotlinx.coroutines.CoroutineName
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -127,8 +132,22 @@ internal class InternalCoroutineDispatcher internal constructor(
}

override fun dispatchSubmit(hunter: BitmapHunter) {
hunter.job = scope.launch(CoroutineName(hunter.getName())) {
hunter.run()
val highPriority = hunter.action?.request?.priority == HIGH
val context = if (highPriority) EmptyCoroutineContext else mainContext

scope.launch(context) {
channel.trySend {
if (hunter.action != null) {
hunter.job = scope.launch(CoroutineName(hunter.getName())) {
hunter.run()
}
} else {
hunterMap.remove(hunter.key)
if (hunter.picasso.isLoggingEnabled) {
log(OWNER_DISPATCHER, VERB_CANCELED, hunter.key)
}
}
}
}
}

Expand Down
Loading

0 comments on commit 200d733

Please sign in to comment.