-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Static component for rendering permanent output #66
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
package com.jakewharton.mosaic | ||
|
||
import androidx.compose.runtime.Composable | ||
import androidx.compose.runtime.ComposeNode | ||
import androidx.compose.runtime.DisallowComposableCalls | ||
import androidx.compose.runtime.LaunchedEffect | ||
import androidx.compose.runtime.mutableStateListOf | ||
import androidx.compose.runtime.remember | ||
import kotlinx.coroutines.flow.Flow | ||
import kotlinx.coroutines.flow.collect | ||
|
||
/** | ||
* Will render each value emitted by [items] as permanent output above the | ||
* regular display. | ||
*/ | ||
@Composable | ||
fun <T> Static( | ||
items: Flow<T>, | ||
content: @Composable (T) -> Unit, | ||
) { | ||
class Item(val value: T, var rendered: Boolean) | ||
|
||
// Keep list of items which have not yet been rendered. | ||
val pending = remember { mutableStateListOf<Item>() } | ||
|
||
LaunchedEffect(items) { | ||
items.collect { | ||
pending.add(Item(it, rendered = false)) | ||
} | ||
} | ||
|
||
Static( | ||
postRender = { | ||
// Remove any items which have been rendered. | ||
pending.removeIf { it.rendered } | ||
} | ||
) { | ||
for (item in pending) { | ||
Row { | ||
// Render item and mark it as having been included in render. | ||
content(item.value) | ||
item.rendered = true | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Renders [content] permanently above the normal canvas. When content has | ||
* actually been written to a [TextCanvas], the [postRender] callback will be | ||
* invoked to allow clearing of content. | ||
* | ||
* @param postRender Callback after rendering to a [TextCanvas] is complete. | ||
* @param content Content which should be rendered permanently above normal | ||
* canvas. | ||
*/ | ||
@Composable | ||
internal fun Static( | ||
postRender: () -> Unit = {}, | ||
content: @Composable () -> Unit, | ||
) { | ||
ComposeNode<StaticNode, MosaicNodeApplier>( | ||
factory = ::StaticNode, | ||
update = { | ||
set(postRender) { | ||
this.postRender = postRender | ||
} | ||
}, | ||
content = content, | ||
) | ||
} | ||
|
||
internal class StaticNode : ContainerNode() { | ||
// Delegate container column for static content. | ||
private val box = BoxNode().also { | ||
it.isRow = false | ||
} | ||
|
||
override val children: MutableList<MosaicNode> | ||
get() = box.children | ||
|
||
var postRender: () -> Unit = {} | ||
|
||
override fun measure() { | ||
// Not visible. | ||
} | ||
|
||
override fun layout() { | ||
// Not visible. | ||
} | ||
|
||
override fun renderTo(canvas: TextCanvas) { | ||
// Render contents of static node to a separate display. | ||
val other = box.render() | ||
|
||
// Add display canvas to static canvases if it is not empty. | ||
if (other.width > 0 && other.height > 0) { | ||
canvas.static.add(other) | ||
} | ||
|
||
// Propagate any static content of the display. | ||
canvas.static.addAll(other.static) | ||
Comment on lines
+97
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is my least favorite part of the change. The fact that a render causes a side-effect to collect these other renders and then something at the call site magically emits that first. It's hard to argue with the output, but I think I would like to try and find something better for this. I'm not going to block merging here because the output is truly fantastic and it's all implementation detail. Render (well, renderTo) is a tree walk, and I think perhaps we separate out the static tree walk to its own function which can be less reliant on side-effects + mutation. It also hopefully means the canvas isn't aware of these static renders. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this was definitely a result of not wanting to make a bunch of render things
Are you thinking something like 2
The tricky part here was that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking a whole separate function, like |
||
|
||
postRender() | ||
} | ||
|
||
override fun toString() = box.children.joinToString(prefix = "Static(", postfix = ")") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't super efficient but it's probably fine. One thing to note is that the
true
s are contiguous starting at index 0 so we could find the firstfalse
index and then dosubList(0, firstFalse).clear()
for an operation that is likely the most optimal.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea. I always forget that modifications on subList are reflected in the full list. Also worth noting that just
pending.clear()
worked in all of the test samples I created but I wasn't convinced that would work in all cases so I added the wrapperItem
class to be safe.