Layout root nodes once per loop

[HalfWhitt]

Fixes #2938

Prior discussion on #2955; I've followed the design outline suggested by @mhsmith in #2955 (comment) and @freakboy3742 in #2955 (review).

I think I have this mostly cracked, but I'm currently stumped.

At the moment I haven't dug into how to update the tests/probes yet — I'm only "testing" by launching toga-demo and seeing what happens. Most of the time it works, and I see this:

Log:

Adding SplitContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding SplitContainer to dirty set
Adding SplitContainer to dirty set

Loop(
    #1. Laying out OptionContainer
    #2. Laying out ScrollContainer
    #3. Laying out SplitContainer
        #4. Laying out OptionContainer
        #5. Laying out ScrollContainer
)

Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding Table to dirty set
Adding Tree to dirty set
Adding Box to dirty set
Adding Table to dirty set
Adding Tree to dirty set
Adding Box to dirty set

Loop(
    #6. Laying out Box
    #7. Laying out Table
    #8. Laying out ScrollContainer
    #9. Laying out Tree
    #10. Laying out OptionContainer
)

Adding Box to dirty set
Adding Table to dirty set
Adding Tree to dirty set

Loop(
    #11. Laying out Box
    #12. Laying out Table
    #13. Laying out Tree
)

That's less than a quarter of the layout calculations done by the non-deferred behavior, which is 55.

However, sometimes (maybe one in five or six), the SplitContainer is smaller on the left side:

The log is almost the same:

Adding SplitContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding SplitContainer to dirty set
Adding SplitContainer to dirty set

Loop(
    #1. Laying out OptionContainer
    #2. Laying out ScrollContainer
    #3. Laying out SplitContainer
        #4. Laying out OptionContainer
        #5. Laying out ScrollContainer
)

Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set
Adding OptionContainer to dirty set
Adding ScrollContainer to dirty set

# In the working version, the above pair of additions are generated an additional three times each.

Adding Table to dirty set
Adding Tree to dirty set
Adding Box to dirty set
Adding Table to dirty set
Adding Tree to dirty set
Adding Box to dirty set

Loop(
    #6. Laying out Box
    #7. Laying out Table
    #8. Laying out ScrollContainer
    #9. Laying out Tree
    #10. Laying out OptionContainer
)

Adding Box to dirty set
Adding Table to dirty set
Adding Tree to dirty set

Loop(
    #11. Laying out Box
    #12. Laying out Table
    #13. Laying out Tree
)

All the same layouts are being performed, in the same order. (Interestingly, while popping from the set of dirty widgets is "random", it appears to be deterministically the same every time.) But the OptionContainer and ScrollContainer aren't getting marked dirty as many times. Of course that doesn't matter in and of itself — presumably it's a matter of the events causing those refresh requests. I'm having trouble tracking those down; these ObjCMethod.__call__ and send_message layers in the debugger stack trace are pretty opaque.

What I do know is that all of these refresh requests come, in the immediate sense, fromTogaSplitView.splitViewDidResizeViews_. For the two always-present pairs as well as the first "ephemeral" pair, that's all I can make sense of — the rest is Objective C, and basic event loop stuff. However, for the last two pairs, behind splitViewDidResizeViews_ in the stack is TogaSplitView.applySplit.

So it appears that when things go awry, it's because (somehow) applySplit isn't being called.

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[freakboy3742]

All the same layouts are being performed, in the same order. (Interestingly, while popping from the set of dirty widgets is "random", it appears to be deterministically the same every time.)

That's not entirely surprising; especially for small sets, it's highly likely for the order to end up the same when driven by the same insertion order. That's not something you should rely on, but it's the sort of thing that is likely reliable enough that you think you can rely on it... right up until you can't.

So it appears that when things go awry, it's because (somehow) applySplit isn't being called.

My guess is that this will be related to the exact order of event firing, and in particular, the chain of set_content and set_bounds calls. It's not clear to me from your debug output if you're looked into set_bounds in particular; but historically, that's where this sort of bug will manifest in my experience. If the widget is laid out with a size allocation of 0x0, then the two sub-containers of the split will be set to minimum size, and the split will be set at the smallest viable position. When the "real" widget size is then applied, the previous split position will be preserved, leading to the wrong placement of the split that you're seeing. IIRC, this is why the call to applySplit is a message passed over ObjC, rather than an explicit Python call - it explicitly needs to be deferred so that the "real" widget size can take effect before any split is applied.

Also - is applySplit not being called at all, or is it being invoked as a no-op because _split_location has been cleared? I'm If it's not being called at all, then I wonder if there might be something raising an exception in the set_bounds() preventing the applySplit message from being sent, but then swallowing the exception for some reason.

Another possibility - does introducing a delay into the applySplit call fix things? That's not a good long-term fix, but adding delays like this might help isolate which sequence of events is the source of the problem.

[HalfWhitt]

Thank you for the insights, those are some good leads to look into. (I'm still learning how the nuts and bolts of the layout mechanism fit together.) Will update with findings once I've prodded some more.