Introducing intrinsics

[sjoshid]

This PR adds compute_max_intrinsic function to Widget trait. Along with the usual layout params, this method also takes an Axis param that controls which intrinsic is expected.

Axis::Horizontal = Max width intrinsic is expected.
Axis::Vertical = Max height intrinsic is expected.

Each widget gets to decide what its intrinsic max width/height is.

For some non-container widgets like Label it is the width the label would take without any line breaks.

For container widget like AspectRatioBox it depends on its width/height irrespective of child's intrinsic width/height. Child's intrinsic is calculated only if AspectRatioBox width/height is infinite.

There are some todos Im planning to fix before I mark this ready for review. But I wanted to get other main points across with this draft PR.
Addressed all todos.

[Zarenor]

Here's a review of this PR; once we've talked through these suggestions, I'll be happy to review again and approve for merging. I'm excited to have this functionality.
I do wonder - is 'max' redundant in the function names? As we've defined it, the intrinsic width is width a widget wishes to be if it has no width constraint - max or min, given the height constraint it has, and vice-versa for height... I'm not sure it's a maximum or a minimum?

[sjoshid]

@Zarenor I agree with your naming suggestions. Actually, anything than what I have makes sense. Im horrible at naming.
I'll clean it up today.

[sjoshid]

I forgot to address this point

I do wonder - is 'max' redundant in the function names? As we've defined it, the intrinsic width is width a widget wishes to be if it has no width constraint - max or min, given the height constraint it has, and vice-versa for height... I'm not sure it's a maximum or a minimum?

max is used just so we can, down the road, introduce a min. Also, Flutter (inspiration behind this PR) has both.

[jneem]

Nice! I only have nitpicking things left. I think @Zarenor was also taking a look so I'm not marking as "Approved" just yet, but it looks fine to me.

[Zarenor]

Looks good to me! Excited to have this feature implemented.

[xStrom]

I did a quick review for some polish. I didn't really do deep analysis of the algorithms though.

Most of my comments are about use import ordering. Sorry about that. I would love to have this automated in the future. There has been a rustfmt proposal for this since 2018 but nobody has invested the time to get it fully done yet. There does seem to be some progress in that rustfmt now has a group_imports option, but it's still unstable. I'll see if we can have a better solution to this, or at least some sort of style guide document. But yeah, for now just remember that the general grouping/ordering of imports should be:

use std::foo;

use some_extrnal_crate::foo;

use crate::my_foo;

[xStrom]

Please also add a changelog entry.

[xStrom]

I'll also add for the record, that for me personally compute_max_intrinsic seems a bit verbose of a name, specifically the compute part. For example, we don't have compute_layout - we just have layout. However I recongize that this is based on the Flutter method computeMaxIntrinsicWidth and historically we've had the rule of thumb that when in doubt, do what Flutter does.

So I don't think this is a blocker to get this merged. We can bikeshed over the name later, perhaps as part of the Xilem API rework.

[xStrom]

Thanks for your continued effort to get this in!

[sjoshid]

Any time. Thank you, @Zarenor and @jneem for spending time and giving valuable feedback. :)