Skip to content

Commit

Permalink
Emit all Treehouse guest changes at the end of a frame
Browse files Browse the repository at this point in the history
Prior to this changes were emitted when the applier's onEndChanges callback was invoked. Compose can do multiple rounds of changes within a single setContent call or sendFrame call resulting in multiple begin/end change callbacks on the applier.
  • Loading branch information
JakeWharton committed Nov 15, 2024
1 parent 521d17d commit 19017e4
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,15 @@ public interface RedwoodComposition {
/**
* @param scope A [CoroutineScope] whose [coroutineContext][kotlin.coroutines.CoroutineContext]
* must have a [MonotonicFrameClock] key which is being ticked.
* @param onChanges Invoked when changes are being applied to the widget tree. Multiple rounds of
* changes can be applied in a single frame or setContent call. This should be relied on only for
* a signal that changes occurred, not for when they are complete.
*/
public fun <W : Any> RedwoodComposition(
scope: CoroutineScope,
view: RedwoodView<W>,
widgetSystem: WidgetSystem<W>,
onEndChanges: () -> Unit = {},
onChanges: () -> Unit = {},
): RedwoodComposition {
view.children.remove(0, view.children.widgets.size)

Expand Down Expand Up @@ -100,13 +103,16 @@ public fun <W : Any> RedwoodComposition(
saveableStateRegistry,
view.uiConfiguration,
widgetSystem,
onEndChanges,
onChanges,
)
}

/**
* @param scope A [CoroutineScope] whose [coroutineContext][kotlin.coroutines.CoroutineContext]
* must have a [MonotonicFrameClock] key which is being ticked.
* @param onChanges Invoked when changes are being applied to the widget tree. Multiple rounds of
* changes can be applied in a single frame or setContent call. This should be relied on only for
* a signal that changes occurred, not for when they are complete.
*/
public fun <W : Any> RedwoodComposition(
scope: CoroutineScope,
Expand All @@ -115,14 +121,14 @@ public fun <W : Any> RedwoodComposition(
saveableStateRegistry: SaveableStateRegistry?,
uiConfigurations: StateFlow<UiConfiguration>,
widgetSystem: WidgetSystem<W>,
onEndChanges: () -> Unit = {},
onChanges: () -> Unit = {},
): RedwoodComposition {
return WidgetRedwoodComposition(
scope,
onBackPressedDispatcher,
saveableStateRegistry,
uiConfigurations,
NodeApplier(widgetSystem, container, onEndChanges),
NodeApplier(widgetSystem, container, onChanges),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import kotlin.math.min
internal class NodeApplier<W : Any>(
override val widgetSystem: WidgetSystem<W>,
root: Widget.Children<W>,
private val onEndChanges: () -> Unit,
private val onChanges: () -> Unit,
) : AbstractApplier<Node<W>>(ChildrenNode(root)),
RedwoodApplier<W> {
private var closed = false
Expand All @@ -66,6 +66,13 @@ internal class NodeApplier<W : Any>(
}
}

override fun onBeginChanges() {
super.onBeginChanges()
// We invoke this here rather than in the end change callback to try and ensure
// no one relies on it to signal the end of changes.
onChanges.invoke()
}

override fun onEndChanges() {
check(!closed)

Expand All @@ -75,8 +82,6 @@ internal class NodeApplier<W : Any>(
}
changedWidgets.clear()
}

onEndChanges.invoke()
}

override fun insertTopDown(index: Int, instance: Node<W>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ public class DefaultGuestProtocolAdapter(
}

override fun emitChanges() {
changesSink.sendChanges(takeChanges())
if (changes.isNotEmpty()) {
changesSink.sendChanges(takeChanges())
}
}

public override fun removeWidget(id: Id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ public abstract class GuestProtocolAdapter(

public abstract fun initChangesSink(changesSink: ChangesSink)

/**
* Write changes to the underlying [ChangesSink].
* This function may no-op if there are no changes to send.
*/
public abstract fun emitChanges()

@RedwoodCodegenApi
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ public fun ProtocolRedwoodComposition(
onBackPressedDispatcher: OnBackPressedDispatcher,
saveableStateRegistry: SaveableStateRegistry?,
uiConfigurations: StateFlow<UiConfiguration>,
onEndChanges: () -> Unit = {},
): RedwoodComposition {
val composition = RedwoodComposition(
scope = scope,
Expand All @@ -47,7 +46,6 @@ public fun ProtocolRedwoodComposition(
saveableStateRegistry = saveableStateRegistry,
uiConfigurations = uiConfigurations,
widgetSystem = guestAdapter.widgetSystem,
onEndChanges = onEndChanges,
)
return ProtocolRedwoodComposition(composition, widgetVersion)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import app.cash.redwood.ui.UiConfiguration
import assertk.assertFailure
import assertk.assertThat
import assertk.assertions.containsExactly
import assertk.assertions.isEmpty
import assertk.assertions.isEqualTo
import assertk.assertions.isInstanceOf
import assertk.assertions.message
Expand Down Expand Up @@ -86,9 +85,6 @@ class ProtocolTest {
},
saveableStateRegistry = null,
uiConfigurations = MutableStateFlow(UiConfiguration()),
onEndChanges = {
assertThat(guestAdapter.takeChanges()).isEmpty()
},
)

var actualDisplayVersion = 0U
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private class RealTestRedwoodComposition<W : Any, S>(
saveableStateRegistry = savedStateRegistry,
uiConfigurations = uiConfigurations,
widgetSystem = widgetSystem,
onEndChanges = { hasChanges = true },
onChanges = { hasChanges = true },
)

override fun setContent(content: @Composable () -> Unit) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import app.cash.redwood.compose.current
import app.cash.redwood.layout.testing.RedwoodLayoutTestingWidgetFactory
import app.cash.redwood.lazylayout.testing.RedwoodLazyLayoutTestingWidgetFactory
import app.cash.redwood.leaks.LeakDetector
import app.cash.redwood.protocol.Change
import app.cash.redwood.protocol.ChildrenChange.Add
import app.cash.redwood.protocol.ChildrenTag
import app.cash.redwood.protocol.Create
Expand Down Expand Up @@ -115,15 +114,13 @@ class ViewTreesTest {
.isEqualTo(expected)

// Validate that the normal Compose protocol backend produces the same list of changes.
val protocolChanges = mutableListOf<Change>()
val guestAdapter = DefaultGuestProtocolAdapter(
hostVersion = hostRedwoodVersion,
widgetSystemFactory = TestSchemaProtocolWidgetSystemFactory,
)
val composition = ProtocolRedwoodComposition(
scope = this + BroadcastFrameClock(),
guestAdapter = guestAdapter,
onEndChanges = { protocolChanges += guestAdapter.takeChanges() },
widgetVersion = UInt.MAX_VALUE,
onBackPressedDispatcher = object : OnBackPressedDispatcher {
override fun addCallback(onBackPressedCallback: OnBackPressedCallback): Cancellable {
Expand All @@ -138,7 +135,7 @@ class ViewTreesTest {
composition.setContent(content)
composition.cancel()

assertThat(protocolChanges).isEqualTo(expected)
assertThat(guestAdapter.takeChanges()).isEqualTo(expected)

// Ensure when the changes are applied with the widget protocol we get equivalent values.
val widgetSystem = TestSchemaWidgetSystem(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package app.cash.redwood.treehouse

import androidx.compose.runtime.BroadcastFrameClock
import androidx.compose.runtime.MonotonicFrameClock
import app.cash.redwood.protocol.EventTag
import app.cash.redwood.protocol.Id
import app.cash.redwood.protocol.RedwoodVersion
Expand All @@ -38,6 +36,7 @@ public class StandardAppLifecycle(
) : AppLifecycle {
private var started = false
private lateinit var host: Host
private val frameListeners = mutableListOf<FrameListener>()

override val guestProtocolVersion: RedwoodVersion
get() = guestRedwoodVersion
Expand All @@ -50,12 +49,11 @@ public class StandardAppLifecycle(
}
}

private val broadcastFrameClock: BroadcastFrameClock = BroadcastFrameClock {
internal fun requestHostFrame() {
if (started) {
host.requestFrame()
}
}
public val frameClock: MonotonicFrameClock get() = broadcastFrameClock

internal val mismatchHandler: ProtocolMismatchHandler = object : ProtocolMismatchHandler {
override fun onUnknownEvent(widgetTag: WidgetTag, tag: EventTag) {
Expand Down Expand Up @@ -87,6 +85,20 @@ public class StandardAppLifecycle(
}

override fun sendFrame(timeNanos: Long) {
broadcastFrameClock.sendFrame(timeNanos)
for (frameListener in frameListeners) {
frameListener.onFrame(timeNanos)
}
}

internal fun addFrameListener(listener: FrameListener) {
frameListeners += listener
}

internal fun removeFrameListener(listener: FrameListener) {
frameListeners.remove(listener)
}

internal fun interface FrameListener {
fun onFrame(timeNanos: Long)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package app.cash.redwood.treehouse

import androidx.compose.runtime.BroadcastFrameClock
import androidx.compose.runtime.saveable.SaveableStateRegistry
import app.cash.redwood.compose.RedwoodComposition
import app.cash.redwood.protocol.Change
Expand Down Expand Up @@ -56,7 +57,18 @@ private class RedwoodZiplineTreehouseUi(
private val guestAdapter: GuestProtocolAdapter,
) : ZiplineTreehouseUi,
ZiplineScoped,
EventSink by guestAdapter {
EventSink by guestAdapter,
StandardAppLifecycle.FrameListener {

private val clock = BroadcastFrameClock {
appLifecycle.requestHostFrame()
}
private var hasChanges = false

override fun onFrame(timeNanos: Long) {
clock.sendFrame(timeNanos)
guestAdapter.emitChanges()
}

/**
* By overriding [ZiplineScoped.scope], all services passed into [start] are added to this scope,
Expand Down Expand Up @@ -125,18 +137,24 @@ private class RedwoodZiplineTreehouseUi(

guestAdapter.initChangesSink(changesSink)

appLifecycle.addFrameListener(this)

val composition = ProtocolRedwoodComposition(
scope = coroutineScope + appLifecycle.frameClock,
scope = coroutineScope + clock,
guestAdapter = guestAdapter,
widgetVersion = appLifecycle.widgetVersion,
onEndChanges = { guestAdapter.emitChanges() },
onBackPressedDispatcher = host.asOnBackPressedDispatcher(),
saveableStateRegistry = saveableStateRegistry,
uiConfigurations = host.uiConfigurations,
)
this.composition = composition

composition.bind(treehouseUi)

// Explicitly emit the initial changes produced by calling 'setContent' (within 'bind').
// All other changes are initiated and emitted by the [onFrame] callback.
hasChanges = false
guestAdapter.emitChanges()
}

override fun snapshotState(): StateSnapshot {
Expand All @@ -145,6 +163,7 @@ private class RedwoodZiplineTreehouseUi(
}

override fun close() {
appLifecycle.removeFrameListener(this)
coroutineScope.coroutineContext.job.invokeOnCompletion {
scope.close()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,10 @@ internal class FastGuestProtocolAdapter(
}

override fun emitChanges() {
sendChanges(changesSinkService, arrayOf(changes))
changes.clear()
if (changes.length > 0) {
sendChanges(changesSinkService, arrayOf(changes))
changes.clear()
}
}

override fun removeWidget(id: Id) {
Expand Down

0 comments on commit 19017e4

Please sign in to comment.