Implement `TaffyLayout` widget #140

nicoburns · 2023-11-01T20:58:55Z

Objective

Enable Taffy based layout to be used with Xilem

Changes made

Gitignore .DS_Store files
Add compute_max_intrinsic method to Pod (this method already existed on the Widget trait)
Add downcast_ref method to Pod (there was already a downcast_mut method)
Added cx.request_paint() in the layout method of the button widget (otherwise it wasn't repainting on window resize)
Create TaffyLayout widget and corresponding view (adapted from the LinearLayout). Constructors for the view currently include flex_row, flex_column, and div.
Create taffy example which uses the new view/widget.

Notes

For the most part this could have been implemented as a 3rd party widget. The only changes required outside of creating the view/widget themselves were minor and not taffy-specific.
This is currently depending on it's own branch of Taffy. But the changes are relatively minor (making Taffy pass along which Axis it is measuring when requesting child measurement), and I don't anticipate major issues getting them merged. (Pass axis information when measuring a child's size DioxusLabs/taffy#575) Changes now merged to Taffy's main branch. I would anticipate a few further minor changes, but the integration works as-is.
Caching is important for performance and is not implemented yet. There are no known blockers, just haven't gotten around to this yet.
There is scope for the API to be improved. Currently it just stores a whole taffy::style::Style on each widget. But it would possible to create more specialised widgets to reduce memory overhead.

Screenshot

jaredoconnell

Is it possible to reduce the redundancy between layout and compute_max_intrinsic?
Seems like a good start to get stuff moving. The cache will be very important in the future.

azymohliad

It works great! Really looking forward to have this merged!

Cargo.toml

src/view/taffy_layout.rs

examples/taffy.rs

giannissc · 2023-11-21T09:37:06Z

I will finally get around to reviewing this today!

Caching is important for performance and is not implemented yet. There are no known blockers, just haven't gotten around to this yet.

Is this something that will be in a separate PR? @nicoburns

nicoburns · 2023-11-21T10:43:22Z

Caching is important for performance and is not implemented yet. There are no known blockers, just haven't gotten around to this yet.

Is this something that will be in a separate PR? @nicoburns

I had a quick think about how this might be implemented, and I think it will need to be. Adding the cache is relatively straightforward. The thing I am currently blocked on is cache invalidation. If the styles of a view/widget change, then the cache would need to be invalidated for that widget and all parents. And I'm not currently sure how to do that? (perhaps a message / event - but how does one send one of those?). It would also be possible to forgo caching between multiple layout runs (and only have it within a single run) and somehow have a "prelayout" or "postlayout" event in which we drop the caches.

DJMcNab

I'd be mostly happy to land this if the taffy feature were added.

I think it would be fine as not optional, but in that case a few things need to be addressed

.gitignore

DJMcNab · 2023-11-22T12:43:22Z

src/widget/button.rs

@@ -108,6 +108,7 @@ impl Widget for Button {
        );
        self.layout = Some(layout);
        //(Size::new(10.0, min_height), size)
+        cx.request_paint();


Should this be automatic/implied?

If the widget is being re-layed out, it seems surprising that it would ever want to not be repainted.

I guess the case would be if it hasn't resized. But in that case, should we be resizing here?

Possibly... I was calling it unconditionally, but now that I've added layout caching it actually doesn't get called if there is a cache hit. I'm not sure whether this would ever save a meaningful amount of work.

Cargo.toml

src/view/taffy_layout.rs

DJMcNab · 2023-11-22T12:52:06Z

src/view/taffy_layout.rs

+}
+
+/// creates a horizontal [`TaffyLayout`].
+pub fn div<T, A, VT: ViewSequence<T, A>>(children: VT) -> TaffyLayout<T, A, VT> {


Div isn't a very compelling name. If there's an alternative you can think of, I'd like to change to it

Hmm... it's called div because that's the name on web, and I can't think of why you'd want to use block layout unless you're emulating web. I could just remove it entirely? Or call it something like raw_taffy_layout?

src/widget/taffy_layout.rs

nicoburns · 2023-11-22T14:04:51Z

@giannissc

Caching is important for performance and is not implemented yet. There are no known blockers, just haven't gotten around to this yet.

Is this something that will be in a separate PR?

Some basic caching is now implemented. It really ought to be implemented for the LinearLayout and Text widgets too, but I think those implementations can be in follow up PRs.

Implement compute_max_intrinsic for Box<dyn AnyWidget>

nicoburns · 2023-11-23T21:21:11Z

Is it possible to reduce the redundancy between layout and compute_max_intrinsic? Seems like a good start to get stuff moving. The cache will be very important in the future.

Not easily. As things stand you could combine them into one method with a parameter instead. But I would expect these methods to further diverge over time, and IMO this would already be more confusing than a small amount of duplication.

nicoburns · 2023-11-23T22:03:41Z

I believe I've addressed all of the review feedback, so this is ready for re-review. I've also made #146 independent of this PR if anybody has time to review that (which is hopefully a little less controversial than this one).

richard-uk1

Fab. Sorry no-one has reviewed until now.

nicoburns added the enhancement New feature or request label Nov 1, 2023

nicoburns mentioned this pull request Nov 1, 2023

Pass axis information when measuring a child's size DioxusLabs/taffy#575

Merged

nicoburns force-pushed the taffy-layout branch 5 times, most recently from 3c0edfd to 7eae31b Compare November 2, 2023 13:21

jaredoconnell reviewed Nov 9, 2023

View reviewed changes

nicoburns force-pushed the taffy-layout branch 2 times, most recently from 6a6ff39 to 07372b7 Compare November 11, 2023 00:39

azymohliad reviewed Nov 15, 2023

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

src/view/taffy_layout.rs Outdated Show resolved Hide resolved

examples/taffy.rs Outdated Show resolved Hide resolved

nicoburns mentioned this pull request Nov 17, 2023

Implement scroll_view widget #146

Closed

nicoburns force-pushed the taffy-layout branch from fbc74c0 to 196ee30 Compare November 21, 2023 10:39

nicoburns force-pushed the taffy-layout branch from 196ee30 to c61936c Compare November 21, 2023 10:44

nicoburns mentioned this pull request Nov 22, 2023

Convert mutable layout getters to layout setters DioxusLabs/taffy#583

Merged

nicoburns force-pushed the taffy-layout branch from 39ccbbb to 33e03c1 Compare November 22, 2023 12:25

DJMcNab reviewed Nov 22, 2023

View reviewed changes

nicoburns force-pushed the taffy-layout branch from 33e03c1 to 63fe0dd Compare November 22, 2023 13:03

nicoburns force-pushed the taffy-layout branch 4 times, most recently from d9ee16f to fde5adb Compare November 23, 2023 19:26

nicoburns added 4 commits November 23, 2023 19:59

Fix text widget layout invalidation

e9bc376

Gitignore .DS_Store files

22789d4

Add downcast_ref method to Pod

c1a44fe

Add compute_max_intrinsic method to Pod

caa0b36

Implement compute_max_intrinsic for Box<dyn AnyWidget>

nicoburns force-pushed the taffy-layout branch from fde5adb to b280f83 Compare November 23, 2023 19:59

nicoburns added 3 commits November 23, 2023 21:54

Add TaffyLayout view and widget

7bf7a67

Add background_color support to TaffyLayout

1c9f944

Add taffy example

9dccf90

nicoburns force-pushed the taffy-layout branch from b280f83 to 9dccf90 Compare November 23, 2023 21:55

richard-uk1 approved these changes Nov 28, 2023

View reviewed changes

nicoburns added this pull request to the merge queue Nov 28, 2023

Merged via the queue into main with commit c439885 Nov 28, 2023
7 checks passed

DJMcNab deleted the taffy-layout branch April 28, 2024 16:29

jaredoconnell mentioned this pull request Oct 17, 2024

Taffy integration #682

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `TaffyLayout` widget #140

Implement `TaffyLayout` widget #140

nicoburns commented Nov 1, 2023 •

edited

Loading

jaredoconnell left a comment

azymohliad left a comment

giannissc commented Nov 21, 2023 •

edited

Loading

nicoburns commented Nov 21, 2023

DJMcNab left a comment

DJMcNab Nov 22, 2023

nicoburns Nov 22, 2023 •

edited

Loading

DJMcNab Nov 22, 2023

nicoburns Nov 22, 2023

nicoburns commented Nov 22, 2023 •

edited

Loading

nicoburns commented Nov 23, 2023

nicoburns commented Nov 23, 2023

richard-uk1 left a comment

Implement TaffyLayout widget #140

Implement TaffyLayout widget #140

Conversation

nicoburns commented Nov 1, 2023 • edited Loading

Objective

Changes made

Notes

Screenshot

jaredoconnell left a comment

Choose a reason for hiding this comment

azymohliad left a comment

Choose a reason for hiding this comment

giannissc commented Nov 21, 2023 • edited Loading

nicoburns commented Nov 21, 2023

DJMcNab left a comment

Choose a reason for hiding this comment

DJMcNab Nov 22, 2023

Choose a reason for hiding this comment

nicoburns Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

DJMcNab Nov 22, 2023

Choose a reason for hiding this comment

nicoburns Nov 22, 2023

Choose a reason for hiding this comment

nicoburns commented Nov 22, 2023 • edited Loading

nicoburns commented Nov 23, 2023

nicoburns commented Nov 23, 2023

richard-uk1 left a comment

Choose a reason for hiding this comment

Implement `TaffyLayout` widget #140

Implement `TaffyLayout` widget #140

nicoburns commented Nov 1, 2023 •

edited

Loading

giannissc commented Nov 21, 2023 •

edited

Loading

nicoburns Nov 22, 2023 •

edited

Loading

nicoburns commented Nov 22, 2023 •

edited

Loading