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 16, 2024
1 parent 521d17d commit 2a3f3c0
Show file tree
Hide file tree
Showing 15 changed files with 70 additions and 35 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, resulting in multiple invocations
* of this callback.
*/
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, resulting in multiple invocations
* of this callback.
*/
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
3 changes: 1 addition & 2 deletions redwood-protocol-guest/api/redwood-protocol-guest.api
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ public final class app/cash/redwood/protocol/guest/ProtocolMismatchHandler$Compa
}

public final class app/cash/redwood/protocol/guest/ProtocolRedwoodCompositionKt {
public static final fun ProtocolRedwoodComposition-C-DY9sE (Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/protocol/guest/GuestProtocolAdapter;ILapp/cash/redwood/ui/OnBackPressedDispatcher;Landroidx/compose/runtime/saveable/SaveableStateRegistry;Lkotlinx/coroutines/flow/StateFlow;Lkotlin/jvm/functions/Function0;)Lapp/cash/redwood/compose/RedwoodComposition;
public static synthetic fun ProtocolRedwoodComposition-C-DY9sE$default (Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/protocol/guest/GuestProtocolAdapter;ILapp/cash/redwood/ui/OnBackPressedDispatcher;Landroidx/compose/runtime/saveable/SaveableStateRegistry;Lkotlinx/coroutines/flow/StateFlow;Lkotlin/jvm/functions/Function0;ILjava/lang/Object;)Lapp/cash/redwood/compose/RedwoodComposition;
public static final fun ProtocolRedwoodComposition-9-Eitbk (Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/protocol/guest/GuestProtocolAdapter;ILapp/cash/redwood/ui/OnBackPressedDispatcher;Landroidx/compose/runtime/saveable/SaveableStateRegistry;Lkotlinx/coroutines/flow/StateFlow;)Lapp/cash/redwood/compose/RedwoodComposition;
}

public abstract interface class app/cash/redwood/protocol/guest/ProtocolWidgetSystemFactory {
Expand Down
2 changes: 1 addition & 1 deletion redwood-protocol-guest/api/redwood-protocol-guest.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ final val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_Protoc
final val app.cash.redwood.protocol.guest/guestRedwoodVersion // app.cash.redwood.protocol.guest/guestRedwoodVersion|{}guestRedwoodVersion[0]
final fun <get-guestRedwoodVersion>(): app.cash.redwood.protocol/RedwoodVersion // app.cash.redwood.protocol.guest/guestRedwoodVersion.<get-guestRedwoodVersion>|<get-guestRedwoodVersion>(){}[0]

final fun app.cash.redwood.protocol.guest/ProtocolRedwoodComposition(kotlinx.coroutines/CoroutineScope, app.cash.redwood.protocol.guest/GuestProtocolAdapter, kotlin/UInt, app.cash.redwood.ui/OnBackPressedDispatcher, androidx.compose.runtime.saveable/SaveableStateRegistry?, kotlinx.coroutines.flow/StateFlow<app.cash.redwood.ui/UiConfiguration>, kotlin/Function0<kotlin/Unit> = ...): app.cash.redwood.compose/RedwoodComposition // app.cash.redwood.protocol.guest/ProtocolRedwoodComposition|ProtocolRedwoodComposition(kotlinx.coroutines.CoroutineScope;app.cash.redwood.protocol.guest.GuestProtocolAdapter;kotlin.UInt;app.cash.redwood.ui.OnBackPressedDispatcher;androidx.compose.runtime.saveable.SaveableStateRegistry?;kotlinx.coroutines.flow.StateFlow<app.cash.redwood.ui.UiConfiguration>;kotlin.Function0<kotlin.Unit>){}[0]
final fun app.cash.redwood.protocol.guest/ProtocolRedwoodComposition(kotlinx.coroutines/CoroutineScope, app.cash.redwood.protocol.guest/GuestProtocolAdapter, kotlin/UInt, app.cash.redwood.ui/OnBackPressedDispatcher, androidx.compose.runtime.saveable/SaveableStateRegistry?, kotlinx.coroutines.flow/StateFlow<app.cash.redwood.ui/UiConfiguration>): app.cash.redwood.compose/RedwoodComposition // app.cash.redwood.protocol.guest/ProtocolRedwoodComposition|ProtocolRedwoodComposition(kotlinx.coroutines.CoroutineScope;app.cash.redwood.protocol.guest.GuestProtocolAdapter;kotlin.UInt;app.cash.redwood.ui.OnBackPressedDispatcher;androidx.compose.runtime.saveable.SaveableStateRegistry?;kotlinx.coroutines.flow.StateFlow<app.cash.redwood.ui.UiConfiguration>){}[0]
final fun app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_DefaultGuestProtocolAdapter$stableprop_getter(): kotlin/Int // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_DefaultGuestProtocolAdapter$stableprop_getter|app_cash_redwood_protocol_guest_DefaultGuestProtocolAdapter$stableprop_getter(){}[0]
final fun app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_GuestProtocolAdapter$stableprop_getter(): kotlin/Int // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_GuestProtocolAdapter$stableprop_getter|app_cash_redwood_protocol_guest_GuestProtocolAdapter$stableprop_getter(){}[0]
final fun app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_ProtocolWidgetChildren$stableprop_getter(): kotlin/Int // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_ProtocolWidgetChildren$stableprop_getter|app_cash_redwood_protocol_guest_ProtocolWidgetChildren$stableprop_getter(){}[0]
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
1 change: 0 additions & 1 deletion redwood-treehouse-guest/api/redwood-treehouse-guest.api
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
public final class app/cash/redwood/treehouse/StandardAppLifecycle : app/cash/redwood/treehouse/AppLifecycle {
public synthetic fun <init> (Lapp/cash/redwood/protocol/guest/ProtocolWidgetSystemFactory;Lkotlinx/serialization/json/Json;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun close ()V
public final fun getFrameClock ()Landroidx/compose/runtime/MonotonicFrameClock;
public fun getGuestProtocolVersion-7jYel6c ()Ljava/lang/String;
public fun sendFrame (J)V
public fun start (Lapp/cash/redwood/treehouse/AppLifecycle$Host;)V
Expand Down
2 changes: 0 additions & 2 deletions redwood-treehouse-guest/api/redwood-treehouse-guest.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
final class app.cash.redwood.treehouse/StandardAppLifecycle : app.cash.redwood.treehouse/AppLifecycle { // app.cash.redwood.treehouse/StandardAppLifecycle|null[0]
constructor <init>(app.cash.redwood.protocol.guest/ProtocolWidgetSystemFactory, kotlinx.serialization.json/Json, kotlin/UInt) // app.cash.redwood.treehouse/StandardAppLifecycle.<init>|<init>(app.cash.redwood.protocol.guest.ProtocolWidgetSystemFactory;kotlinx.serialization.json.Json;kotlin.UInt){}[0]

final val frameClock // app.cash.redwood.treehouse/StandardAppLifecycle.frameClock|{}frameClock[0]
final fun <get-frameClock>(): androidx.compose.runtime/MonotonicFrameClock // app.cash.redwood.treehouse/StandardAppLifecycle.frameClock.<get-frameClock>|<get-frameClock>(){}[0]
final val guestProtocolVersion // app.cash.redwood.treehouse/StandardAppLifecycle.guestProtocolVersion|{}guestProtocolVersion[0]
final fun <get-guestProtocolVersion>(): app.cash.redwood.protocol/RedwoodVersion // app.cash.redwood.treehouse/StandardAppLifecycle.guestProtocolVersion.<get-guestProtocolVersion>|<get-guestProtocolVersion>(){}[0]

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,17 @@ private class RedwoodZiplineTreehouseUi(
private val guestAdapter: GuestProtocolAdapter,
) : ZiplineTreehouseUi,
ZiplineScoped,
EventSink by guestAdapter {
EventSink by guestAdapter,
StandardAppLifecycle.FrameListener {

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

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 +136,23 @@ 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.
guestAdapter.emitChanges()
}

override fun snapshotState(): StateSnapshot {
Expand All @@ -145,6 +161,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 2a3f3c0

Please sign in to comment.