Optimise layout performance

[nicoburns]

Objective

• Make layout faster.
• Enable Advanced layout strategies (grid, floating text, etc.) #34 to be implemented with decent performance.

Context

#34 (comment)

Changes made

• Makes the layout method on the Widget trait take &mut self instead of &self (this enables caching the resultant layout)
• Makes the draw method on the Widget trait take &mut self instead of &self (this was necessary because the tooltip component calls layout from draw)
• Adds a measure method to the Widget trait which has the semantics "compute the size of this widget without performing a full recursive layout unless necessary". This method has a default implementation with just calls layout.
• Implement optimised versions of measure for row, column, and `container.
• Adds set_size (&mut self, size: Size) and into_children(self) -> Vec<Node> methods to layout::Node

This PR (along with associated changes in iced_taffy) improves the performance of an iced_taffy stress test of a 7000+ node nested grid by 20x (from 200ms for layout down to 12ms).

Further work

Further work will be needed to make things really fast for realistic layouts. Including:

• Add optimised measure methods to other widgets where appropriate (esp. other container widgets if there are any)
• Add layout caching to row/column/container and other widgets where appropriate
• Add layout caching for text (at least caching the results of shaping)

Feedback wanted

• In general, are the Iced maintainers amenable to accepting changes along these lines?
• Is changing the layout method to take &mut self acceptable? Converting the widgets to work with this was fairly easy, and it seems like the cleanest solution to enabling caching to me. But caching could also be implemented using RefCell without this change.
• Is changing the draw method to take &mut self acceptable? I didn't look into why tooltip is calling layout in draw, but it seems to me that it probably shouldn't be? If tooltip was modified to not do this then this change could be reverted.

[hecrj]

Hey, thanks for this exploration!

Important changes to the library (like changes to the Widget API) need to go through the RFC process.

There is a lot to unpack here. As far as I understand, the motivation behind these changes is an external crate that you are working on.

If we are going to let an external crate drive further design of the library, we need to find an agreement on the use cases that this external crate is tackling and why it is the best approach for the library.

If the point being made is that these changes will benefit the library independently of the external crate, then we need to specify exactly how (e.g. listing use cases and examples, etc.) and consider any tradeoffs.

This should be the first step before we can consider writing any code. Feel free to open a discussion on Discord!

---

That said, have you considered using an extension trait to add the measure method in your own crate?

[nicoburns]

Important changes to the library (like changes to the Widget API) need to go through the RFC process.

Ah right. I'll look into creating an RFC then.

That said, have you considered using an extension trait to add the measure method in your own crate?

That won't work unfortunately, because I need the measure method to be implemented by Iced's built-in types. Otherwise mixing and matching say an iced_taffy::Grid with an iced_native::Row would not benefit from the performance improvement. One alternative option could be to add a parameter to the layout trait instead of a separate method. I'll put details about that in the RFC.

Some quick responses, as it might take a while to write up an RFC:

There is a lot to unpack here. As far as I understand, the motivation behind these changes is an external crate that you are working on. If we are going to let an external crate drive further design of the library, we need to find an agreement on the use cases that this external crate is tackling and why it is the best approach for the library.

Yes. These changes are driven by the (performance) needs of iced_taffy, a library that implements Taffy's Flexbox and CSS Grid layout modes as Iced widgets. Not everyone likes CSS layout modes, but lots of people do, so it seems sensible to offer it as an external library that people can choose to use if they wish to. The Grid functionality would be particularly useful as Iced doesn't doesn't currently have a grid layout widget (and the one in iced_aw only allows evenly sized columns and gives no control over row size).

If the point being made is that these changes will benefit the library independently of the external crate, then we need to specify exactly how (e.g. listing use cases and examples, etc.) and consider any tradeoffs.

Indeed it is. Nothing in this PR is taffy-specific. It just contains performance optimisations for layout which allow the initial passes of multi-pass layout modes (which would include Iced's existing Row and Column widgets) to be computed more efficiently, and to be cached for subsequent relayouts. This benefits any user of Iced that has a large complex widget layout by allowing their app to render more quickly. It could also allow for things like addressing #1569 without regressing performance.

The tradeoff is an extra method to implement (although it could be given a default implementation to avoid the need to implement it for widgets where full layout is fast (e.g. an image widget)).

[hecrj]

Indeed it is. Nothing in this PR is taffy-specific. It just contains performance optimisations for layout which allow the initial passes of multi-pass layout modes (which would include Iced's existing Row and Column widgets) to be computed more efficiently, and to be cached for subsequent relayouts. This benefits any user of Iced that has a large complex widget layout by allowing their app to render more quickly. It could also allow for things like addressing #1569 without regressing performance.

I'm not sure we want to invest effort in making multi-pass layout modes a thing. If anything, I'd prefer to see efforts that go in the opposite direction! The current layout engine is inspired by druid, which in turn was inspired by Flutter: "Constraints go down. Sizes go up. Parent sets position."

We should explore all of the possibilities of the single-pass approach and understand its limitations. Defaulting to support multi-pass because Flexbox and CSS Grid do it and lots of people like it isn't a good enough argument.

Caching also opens a brand new can of worms. We need to talk cache invalidation! How are layouts reused? How do we efficiently detect when to reuse them? We have changed layout to be mutable here, but there is no use of it as far as I can tell.

Keep all of this in mind for the RFC!

[alexculea]

Defaulting to support multi-pass because Flexbox and CSS Grid do it and lots of people like it isn't a good enough argument.

Actually it is a great argument IMO. Flexbox/Grid, simplifies the layout a lot versus the 2000's era aged system of constraints.

[hecrj]

I disagree. Flexbox and CSS are full of pitfalls and suffering. Brittle, full of redundancy, and a bunch of structure must be in place to make everything work.

Web developers are so used to the pain that they have become numb to the torture devices they use every day. I was one of them for 15 years. It wasn't until I tried Elm that I found how much happier I was with a better set of tools.

iced takes a lot of inspiration from elm-ui, which has the slogan: "What if you never had to write CSS again?". I recommend you to watch this amazing talk by the author of the library.

the 2000's era aged system of constraints.

I am not proposing we go back to using tables to layout your UI. The example I provided above is Flutter, which was originally released in 2017.

[alexculea]

Web developers are so used to the pain that they have become numb to the torture devices they use every day.

😆😆
Funny, maybe true at times.

I think we are getting a little distracted with CSS here, what I meant is these days a good UI toolkit should support items wrapping on the next row and would be great if styles could conditionally apply at various view port sizes (similar to CSS @media). Any respectable UI needs to be responsive at any view port size, these days apps go in car dashboard screens and TVs and foldables and what not.

I've worked on web apps where things animated into place as you were resizing the browser window and have been easily ported to large number of platforms because of the doctrine, design once, run everywhere. Not having this feels like a step backward.

[hecrj]

Funny, maybe true at times.

Hah, of course. I was being hyperbolic. Or was I? Most of the experienced web developers I know that have seen the grass on the other side would agree on the utter insanity of the web stack.

In any case, a Row that wraps elements should be possible to implement with the current layout architecture. We just don't have such a widget yet (actually, iced_aw has a Wrap widget!). We don't need a generic layout engine that can handle everything at once! Every widget has total control over the layout of its contents.

Responsiveness can also be achieved already with the Responsive widget.

[nicoburns]

I think fundamentally my point of view is: let people lay things out however they like. The framework doesn't need to be opinionated about layout, so why should it. If people want to use a less efficient method then they can. That's on them. Chances are it won't cause problems in practice, and if it does then they can optimise that code. If they want to use a more efficient method from the start then they also can.

It seems that your view is that Iced ought to pick One True Way to do layout. Which which I suppose fits given it's stated influence from Elm. What I would point out is that this is exactly why very few people use Elm these days: they were overly opinionated about their stack and refused to include escape hatches to allow people to deal with real-world constraints pragmatically. So people used something else that would let them do that.

Perhaps that will be less of an issue on Desktop where you have more control over the underlying platform. But it's definitely possible to create a framework that's both opinionated and flexible by exposing opinionated high-level APIs on top of less opinionated low-level APIs. Laravel is my favourite example of this. IMO it's the best of both worlds, and I would definitely encourage you to go down that route (while respecting that it's your framework, and you can choose whichever approach you want to).

Most of the experienced web developers I know that have seen the grass on the other side would agree on the utter insanity of the web stack.

I think I can somewhat agree that web layout is unnecessarily quirky. I would be pretty happy with Flutter style layout. However, Iced doesn't currently have that, and there are lots of things that can't currently be done with Iced's layout widgets. For example, percentage based sizing or complex grid layouts. I want these kind of things available ASAP so I can get on with laying out my app. And wiring up the web layout that already exists as a library is going to be by far the quickest way to get there.

[hecrj]

It seems that your view is that Iced ought to pick One True Way to do layout.

Not at all. The current layout system isn't opinionated. This isn't the reason I have issues with the changes.

My first comment reflects my view:

If we are going to let an external crate drive further design of the library, we need to find an agreement on the use cases that this external crate is tackling and why it is the best approach for the library.

If the point being made is that these changes will benefit the library independently of the external crate, then we need to specify exactly how (e.g. listing use cases and examples, etc.) and consider any tradeoffs.

Before we write any generic code, we need to look at multiple use cases individually to evaluate if their similarities warrant a generic solution. Maybe the best approach is to tackle each specific use case directly instead of writing a generic solution!

Use cases come first, then specific solutions. A pattern may arise and a generic solution may be adequate, but it's not guaranteed.

You argued that nothing in this PR is specific to your use case. Therefore, it is supposed to be a generic change. In order to justify a generic solution, we need to prove it is arising somewhat organically from a bunch of use cases (not just yours!).

We can't let a single use case drive a change to the generic Widget API. Does that make sense?

For example, percentage based sizing or complex grid layouts. I want these kind of things available ASAP so I can get on with laying out my app. And wiring up the web layout that already exists as a library is going to be by far the quickest way to get there.

Percentage-based sizing should be possible with Length::FillPortion and the current Row and Column. Still, custom widgets could be built easily to lay things out in more complex ways.

"Complex grid layouts" sounds too generic. We have a PaneGrid widget that satisfies a specific kind of use case. Similar widgets could be built to satisfy other grid use cases.

What is probably very hard to build is a completely generic and powerful solution for any kind of "complex grid layout" (a la Flexbox grid). This is why I'd like to see more specific use cases tackled before we conclude we need to change the generic Widget API to facilitate multi-pass layouts.

[nicoburns]

@hecrj Still intending to submit an RFC, but see this benchmark (cargo run --release --example huge_nested_row_column in the iced_taffy repo) which demonstrates the significant performance benefits from introducing caching with Iced's existing Row/Column widgets, the fact that Iced already uses multipass layout in some cases, and that Iced's existing Row/Column widget's layout currently has superlinear (exponential? quadratic?) time complexity with respect to the depth of the widget tree in these cases.

The benchmark measures a nested layout which alternates between Row / Column widgets at each level (until the bottom level which contains a custom fixed size (20px x 20px) rectangle widget) and uses width(Length::Shrink), height(Length::Shrink) and align_items(Alignment::Fill) on every row/column. The Alignment::Fill is important here as this is what triggers the multipass code path in Iced's flex layout. Timings and node counts are printed to the console. Results below are measured in release mode.

• With commit 28e26505f28867585f8b9bbd2ecb0bfdf7096665 a layout that is 12 layers deep with 2 child elements at each level (for a total of 8190 elements) takes over 500ms to compute layout. Similar slow behaviour (taking ~70ms) can be triggered by a layout that is 20 layers deep with 1 child element at each level (for a total of only 20 elements).
• With commit 8165eb37f4678a9bbedc36270cc0cca414a4ea12 which updates to an iced revision that implements caching of measure child sizes on top of the changes that were already contained in this PR (see these two commits for implementation (nicoburns@486d1cb and nicoburns@237db49) that is reduced to ~0.8ms

In a production-ready implementation you'd want to invalidate the cache based on the passed in Limits. I have logic for this, but I haven't included it here for demo purposes. I wouldn't expect that to significantly affect performance.

[hecrj]

@nicoburns As I said, I just prefer to remove Alignment::Fill and explore other single-pass options. Why aren't our efforts in that direction? Why are we assuming multi-pass is necessary?

[hecrj]

Just opened #1735 to remove Alignment::Fill.

[nicoburns]

@nicoburns As I said, I just prefer to remove Alignment::Fill and explore other single-pass options. Why aren't our efforts in that direction? Why are we assuming multi-pass is necessary?

I think it's fair to conclude based on the success of Flutter that multipass layout absolutely isn't necessary, even though it can be convenient at times. I guess I figured it was a pretty small change to support it, so why not. But if you'd rather not then I don't think I have a good argument against that.

I do disagree that it's a large effort to do so - I've spent maybe 4 hours on this and it's basically done (the only remaining change would be optimising/caching text layout, which shouldn't be too hard but I'm loathe to work on while the text implementation is otherwise in flux due to https://github.com/iced-rs/iced/tree/advanced-text). It does introduce caching (with an associated memory overhead) which wouldn't be necessary within a single frame for single-pass layout. If you ever wanted to introduce layout that is incremental between frames than that would require all that logic anyway, but I suppose if layout is fast enough then perhaps you don't need that.

[hecrj]

@nicoburns It's not about the amount of work it takes to get there. A new design encourages certain new practices, new APIs need to be maintained, future ideas need to be reconsidered, etc.

There is a lot more to consider than the work needed for an implementation. Code is the easy part!