Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    Is it possible to take a
    CompositionReference
    from a composable function, pass it through to a
    FrameLayout
    hosted inside the composition, and have that legacy view pass that reference through to a composable hosted inside itself? I.e. so that any ambients in effect where the legacy view is added to the composition get propagated to the leaf composable inside the legacy view. I’m doing the naïve thing right now of just having the legacy view call
    setViewContent
    instead of
    setContent
    , which takes a
    CompositionReference
    . However, the leaf composable doesn’t seem to be getting model updates, and throws an IllegalStateException: “Unsupported node type LayoutNode” if I try composing something like a
    Text
    . I’m sure all this infrastructure is full of holes right now, but is this something that’s ideally supposed to work?
    setViewContent
    has a TODO on it to remove, and the only other API that seems relevant here is
    subcomposeInto
    but that requires a
    ComponentNode
    which I haven’t tried to figure out how to get yet.
    Andrey Kulikov

    Andrey Kulikov

    2 years ago
    but looks like we are also loosing the ambient values
    Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    // TODO: This should probably create a child composition of the original
    Does this mean the code here needs to change, or that
    Recomposer
    should be responsible for propagating ambients across compositions, like I'm trying to use
    CompositionReference
    for?
    The documentation on
    compositionReference()
    is pretty straightforward and seems to be exactly what I want, so i guess another way to ask that is, is
    Recomposer
    replacing
    CompositionReference
    at some point?
    Just looked through the source for
    Recomposer
    and it looks like it's singularly responsible for scheduling and performing composition passes. There's nothing in there about ambients.
    l

    Leland Richardson [G]

    2 years ago
    these are pretty different things… sorry i’m not sure what you’re trying to do exactly. Are you trying to mix views/compose ui in the content of a
    setViewContent
    call? I am not sure how well we handle going back and forth between tree types right now… especially with a view at the root (which is what setViewContent is). long term
    setViewContent
    is likely to go away, and
    setContent
    will handle both types of trees regardless of what you do.
    Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    I’ve got a view/composable structure like this:
    @Composable Foo() {
        Providers(MyAmbient provides foo) {
            AndroidView(R.layout.bar) { view ->
                view.setContent {
                    Baz(MyAmbient.current)
                }
            }
        }
    }
    And I want that innermost composable to be able to access ambients from the outer one.
    l

    Leland Richardson [G]

    2 years ago
    i see. yeah, you want to use
    compositionRefernce()
    but i’m not sure that our newer setContent APIs are making that easy
    you probably want to use
    subcomposeInto
    specifying a
    compositionRefernce
    and
    Recomposer.current()
    oh actually subcomposeInto is only defined for componentnodes
    we might want to have an equivalent API for views
    or alternatively we could add compositionReference as a parameter to
    setContent
    some of these APIs are jkind of a mess right now as we refactor things to be multi-threaded
    can you file a bug with this use case and assign it to me?
    Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    Sure!
    FWIW I got it working using a bunch of reflection to get around the current API limitations, and verified that ambients are indeed propagating, at least initially (changes don’t show up, but it’s progress).
    l

    Leland Richardson [G]

    2 years ago
    yeah the composition reference creates a link between the two compositions so that changes are propagated correctly and sequenced correctly
    Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    So changes to ambients are supposed to get passed through? If I animate the ambient value in the outer composable, the inner composable gets the initial ambient value, but doesn’t get recomposed when the ambient changes.
    l

    Leland Richardson [G]

    2 years ago
    with a composition reference they should get passed through
    without one, they shouldn’t get passed at all (it should go back to the default values)
    the strange part is propagating changes. if you want invalidations to flow through correctly you really need to think of them as one composition. compositionReference allows you to do that. we are hoping to change some of these APIs to have a shape where compositionReference is no longer needed and this always just happens if you use one API, and doesn’t if you use another
    Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    if you want invalidations to flow through correctly you really need to think of them as one composition.
    That makes sense, and it matches the behavior I’m seeing initially. But when the value provided by the ambient changes, the child compositions aren’t seeing the value. I suspect this is a bug, will file as well.
    Filed the API issue: https://issuetracker.google.com/issues/156527485 And the updates issue: https://issuetracker.google.com/issues/156527486 Thanks again for your help! This was fun.
    I have been hacking a lot on this, trying to understand how subcomposition and especially ambient propagation work, and I'm am stuck at a point where I'm not sure if the behavior I'm seeing is a bug or my own mistake. I'm not sure if it's worth filing a bug yet, since this isn't even using public API. I'm using the reflection I posted on the issue, basically just a direct line-for-line copy/paste of the
    ViewGroup.setContent
    implementation with the additional
    CompositionReference
    parameter. It seems there are two keys to using subcomposition that aren't immediately obvious. The first one is that, for the subcomposition to recompose after changing some observed
    State
    , you have to call
    setContent
    explicitly. I tried only calling it once, when setting up the Android View, and it did the initial compose pass successfully, but mutating a
    State
    object observed in the child composition didn't trigger recomposition. I even tried having the child pass its
    invalidate
    function out and calling it directly, which didn't work. I didn't think this would be necessary because the normal
    setContent
    doesn't require constantly being re-invoked. However, I noticed that both
    WithConstraints
    and
    AdapterList
    call
    subcomposeInto
    multiple times so I guess that's expected. They also always call
    nextFrame
    after doing so, but I couldn't see any difference in behavior with or without calling that. Secondly, and this is where it seems like there's a bug, the above strategy of calling
    setContent
    multiple times works for keeping the content inside the composition fresh. However, on the next composition pass, the subcomposition suddenly loses any custom ambients provided from the parent (they all "reset" back to their default values). If I add a second Android view, sibling to the first one, with its own subcomposition, it also works on the first composition pass and then loses its ambients on the next one. There's a lot going on here so I apologise if my explanation is confusing. I can pull the code in question into a separate repository, but wanted to get a quick sanity check here first. Am I just too far in the weeds here, and I should stop wasting my time until the API to do this is actually public?
    After another two days of investigating, I am basically as confused as before, although I think I’ve got it working. I haven’t had any luck reproducing this in an isolated codebase, but I can get two different exceptions in my main codebase – in one case a
    LayoutNode
    is complaining about not being measured, and in another an NPE because
    DensityAmbient.current
    seems to be returning null, which as far as I can tell shouldn’t even be possible. I think the issue has something to do with how Android views and LayoutNodes are initialized when they are nested recursively, alternating between the two (i.e. Subcomposition inside View inside Composition inside View). This seems to be more problematic if the “View inside Composition” is initialized inside a composable before passing it to
    AndroidView
    . If I use a custom view and add that to the composition before trying to initialize the child android view, it works. The only case I haven’t figured out so far is that I get the LayoutNode measure exception when forcing a recomposition of a “root” composition (no parent CompositionReference). It seems like compositions with a parent must be recomposed explicitly, but compositions without a parent must not be composed more than once. I have to handle those cases separately – if there’s a parent, I call
    setContent
    on every update, and if there’s no parent, I call
    setContent
    only once and use a
    MutableState
    to pass updates in. I have no idea why any of this works, or why ambients will sometimes disappear if things are done the wrong way, but at least all this weirdness can be hidden and when it works, it’s pretty magical.
    l

    Leland Richardson [G]

    2 years ago
    Hmm. I feel bad for you feeling down this rabbit hole. I think this also might influence some ways we think about subcomposition apis. Maybe later today we can hop on a video call and I can try and help you get it figured out? I'm not sure I understand all of the issues you're seeing
    Zach Klippenstein (he/him) [MOD]

    Zach Klippenstein (he/him) [MOD]

    2 years ago
    Don't feel bad about it, I know what I signed up for 🙂 I am definitely finding it hard to get all the context across in writing, I'll DM you about a call.
    At least one of the issues seemed to be that calling
    setContent
    multiple times crashes, but it should not. I’ve narrowed this down to probably having something to do with capturing lambdas, filed as https://issuetracker.google.com/issues/157430448.