Skip to content

Commit

Permalink
[Skia] Fix hazeSource content changes not updating hazeEffects (#476)
Browse files Browse the repository at this point in the history
There's two related issues which this PR fixes:

- We no longer draw graphics layers in a
`Snapshot.withoutReadObservation`. This was done to avoid unwanted
invalidations in Haze, but it has the knock-on effect that it breaks
internal invalidation in the Skiko GraphicsLayer impl. Fixed by using
`withoutReadObservation` more locally around state reads.
- We were not observing `HazeArea.contentLayer`. This probably isn't the
root cause of this issue, but this is more defensive for the future.

Fixes #475
  • Loading branch information
chrisbanes authored Jan 16, 2025
1 parent 0bcdb36 commit 4196500
Show file tree
Hide file tree
Showing 18 changed files with 81 additions and 38 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion haze/src/commonMain/kotlin/dev/chrisbanes/haze/Haze.kt
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class HazeArea {
/**
* The content [GraphicsLayer].
*/
var contentLayer: GraphicsLayer? = null
var contentLayer: GraphicsLayer? by mutableStateOf(null)
internal set

internal val bounds: Rect? get() = when {
Expand Down
66 changes: 32 additions & 34 deletions haze/src/commonMain/kotlin/dev/chrisbanes/haze/HazeEffectNode.kt
Original file line number Diff line number Diff line change
Expand Up @@ -341,40 +341,38 @@ class HazeEffectNode(
val bg = resolveBackgroundColor()
require(bg.isSpecified) { "backgroundColor not specified. Please provide a color." }

// We don't want to observe any state here. Anything we state we read should be observed
// be triggered from `dirtyTracker` only
Snapshot.withoutReadObservation {
val bounds = Rect(positionOnScreen, size)

layer.record(scaledSize.roundToIntSize()) {
drawRect(bg)

clipRect {
scale(scale = scaleFactor, pivot = Offset.Zero) {
translate(offset = -positionOnScreen) {
for (area in areas) {
require(!area.contentDrawing) {
"Modifier.haze nodes can not draw Modifier.hazeChild nodes. " +
"This should not happen if you are providing correct values for zIndex on Modifier.haze. " +
"Alternatively you can use can `canDrawArea` to to filter out parent areas."
}

val areaBounds = area.bounds
if (areaBounds == null || !bounds.overlaps(areaBounds)) {
log(TAG) { "Area does not overlap us. Skipping... $area" }
continue
}

translate(area.positionOnScreen.orZero) {
// Draw the content into our effect layer
area.contentLayer
?.takeUnless { it.isReleased }
?.takeUnless { it.size.width <= 0 || it.size.height <= 0 }
?.let {
log(TAG) { "Drawing HazeArea GraphicsLayer: $it" }
drawLayer(it)
}
}
val bounds = Rect(positionOnScreen, size)

layer.record(scaledSize.roundToIntSize()) {
drawRect(bg)

clipRect {
scale(scale = scaleFactor, pivot = Offset.Zero) {
translate(offset = -positionOnScreen) {
for (area in areas) {
require(!area.contentDrawing) {
"Modifier.haze nodes can not draw Modifier.hazeChild nodes. " +
"This should not happen if you are providing correct values for zIndex on Modifier.haze. " +
"Alternatively you can use can `canDrawArea` to to filter out parent areas."
}

val areaBounds = Snapshot.withoutReadObservation { area.bounds }
if (areaBounds == null || !bounds.overlaps(areaBounds)) {
log(TAG) { "Area does not overlap us. Skipping... $area" }
continue
}

val position = Snapshot.withoutReadObservation { area.positionOnScreen.orZero }
translate(position) {
// Draw the content into our effect layer. We do want to observe this via snapshot
// state
area.contentLayer
?.takeUnless { it.isReleased }
?.takeUnless { it.size.width <= 0 || it.size.height <= 0 }
?.let {
log(TAG) { "Drawing HazeArea GraphicsLayer: $it" }
drawLayer(it)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,39 @@ class HazeScreenshotTest : ScreenshotTest() {
captureRoot()
}

/**
* This test does not currently produce the correct output on Skia platforms.
* It works correctly when run on device, etc. It seems to be a timing setup thing in tests.
*
* My working theory is that state updates are ran immediately in the CMP UI tests, which
* breaks how dependent graphics layers are invalidated. In non-tests, state updates are deferred
* until the next 'pass'.
*
* This is being re-worked in CMP 1.8, so there's little point in investigating this too much:
* https://youtrack.jetbrains.com/issue/CMP-6703
*/
@Test
fun creditCard_sourceContentChange() = runScreenshotTest {
var backgroundColors by mutableStateOf(listOf(Color.Blue, Color.Cyan))

setContent {
ScreenshotTheme {
CreditCardSample(backgroundColors = backgroundColors, tint = DefaultTint)
}
}

waitForIdle()
captureRoot("blue")

backgroundColors = listOf(Color.Yellow, Color.hsl(0.4f, 0.94f, 0.58f))
waitForIdle()
captureRoot("yellow")

backgroundColors = listOf(Color.Red, Color.hsl(0.06f, 0.69f, 0.35f))
waitForIdle()
captureRoot("red")
}

companion object {
val DefaultTint = HazeTint(Color.White.copy(alpha = 0.1f))
val OverrideStyle = HazeStyle(tints = listOf(HazeTint(Color.Red.copy(alpha = 0.5f))))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import androidx.compose.ui.unit.dp

@Composable
internal fun CreditCardSample(
backgroundColors: List<Color> = listOf(Color.Blue, Color.Cyan),
style: HazeStyle = HazeStyle.Unspecified,
tint: HazeTint = HazeTint.Unspecified,
blurRadius: Dp = 8.dp,
Expand All @@ -52,7 +53,7 @@ internal fun CreditCardSample(
Spacer(
Modifier
.fillMaxSize()
.background(brush = Brush.linearGradient(colors = listOf(Color.Blue, Color.Cyan))),
.background(brush = Brush.linearGradient(colors = backgroundColors)),
)

Text(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,6 @@ fun randomSampleImageUrl(
): String = "https://picsum.photos/seed/$seed/$width/$height"

@Composable
fun rememberRandomSampleImageUrl(index: Int = -1): String = rememberSaveable {
fun rememberRandomSampleImageUrl(index: Int = -1): String = rememberSaveable(index) {
precannedImageUrls.getOrNull(index) ?: randomSampleImageUrl()
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.filled.ArrowBack
import androidx.compose.material.icons.filled.Refresh
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton
Expand All @@ -21,7 +22,10 @@ import androidx.compose.material3.Scaffold
import androidx.compose.material3.Text
import androidx.compose.material3.TopAppBar
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
Expand All @@ -38,6 +42,8 @@ import dev.chrisbanes.haze.materials.HazeMaterials
@OptIn(ExperimentalMaterial3Api::class, ExperimentalHazeMaterialsApi::class)
@Composable
fun ListOverImage(navigator: Navigator) {
var imageIndex by remember { mutableIntStateOf(0) }

MaterialTheme {
Scaffold(
topBar = {
Expand All @@ -48,6 +54,11 @@ fun ListOverImage(navigator: Navigator) {
Icon(Icons.AutoMirrored.Filled.ArrowBack, null)
}
},
actions = {
IconButton(onClick = { imageIndex++ }) {
Icon(Icons.Default.Refresh, "Refresh background button")
}
},
modifier = Modifier.fillMaxWidth(),
)
},
Expand All @@ -57,7 +68,7 @@ fun ListOverImage(navigator: Navigator) {

Box {
AsyncImage(
model = rememberRandomSampleImageUrl(0),
model = rememberRandomSampleImageUrl(imageIndex),
contentScale = ContentScale.Crop,
contentDescription = null,
modifier = Modifier
Expand Down

0 comments on commit 4196500

Please sign in to comment.