Allow sum of percent and points in Dimension

[ghost]

What problem does this solve or what need does it fill?

This allows using percentage with an offset in one dimension.

What solution would you like?

/// A unit of linear measurement
///
/// This is commonly combined with [`Rect`], [`Point`](crate::geometry::Point) and [`Size<T>`].
/// The default value is [`Dimension::Undefined`].
#[derive(Copy, Clone, PartialEq, Debug)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub enum Dimension {
    /// The dimension is not given
    Undefined,
    /// The dimension should be automatically computed
    Auto,
    /// The dimension is stored in [points](https://en.wikipedia.org/wiki/Point_(typography))
    ///
    /// Each point is about 0.353 mm in size.
    Points(f32),
    /// The dimension is stored in percentage relative to the parent item.
    Percent(f32),
    /// The dimension is stored as the sum of percentage relative to the parent item and point.
    PercentPoints(f32, f32),
}

impl Dimension {
    /// Is this value defined?
    pub(crate) fn is_defined(self) -> bool {
        matches!(self, Dimension::Points(_) | Dimension::Percent(_) | Dimension::PercentPoints(_, _))
    }
}

impl MaybeResolve<Option<f32>> for Dimension {
    /// Converts the given [`Dimension`] into a concrete value of points
    ///
    /// Can return `None`
    fn maybe_resolve(self, context: Option<f32>) -> Option<f32> {
        match self {
            Dimension::Points(points) => Some(points),
            // parent_dim * percent
            Dimension::Percent(percent) => context.map(|dim| dim * percent),
            Dimension::PercentPoints(percent, points) => Some(context.map_or(points, |dim| dim * percent + points)),
            _ => None,
        }
    }
}

What alternative(s) have you considered?

I've considered something similar to #225 but thought this was a better option.

[alice-i-cecile]

I like this better than relying more on measure functions for this common case.

[Weibye]

I agree, this seems like a good direction :)

[TimJentzsch]

Personally, I would prefer to add "proper" support with Add, Sub, Mult and Div variants. I'd assume that this is not much harder to implement than this special case and it opens up even more use cases.

That would also allow us to line up closer to the CSS spec.

Additionally, I think the names would be clearer to read then PercentPoints, you kind of have to look at the docs first to know what exactly it means.

[nicoburns]

@TimJentzsch I think "proper" support is a fair bit more complex, because it needs to support nesting. Which in turn requires allocation and then a recursive algorithm.

[alice-i-cecile]

Yep, we fundamentally can't model "proper" support using Rust's Add etc traits: we need extra information that there's no parameter for (unless we're storing parent size on every instance).

[geom3trik]

@TimJentzsch I think "proper" support is a fair bit more complex, because it needs to support nesting. Which in turn requires allocation and then a recursive algorithm.

What is meant by nesting here?

[nicoburns]

What is meant by nesting here?

Nesting of expressions. CSS calc doesn't just support 30px + 30%, it supports things like ((30px + max(30%, 20px)) / 2) + 2em. To represent that using Rust types you'd need some kind of tree.

[TimJentzsch]

We can probably do something like Add(Box<Dimension>, Box<Dimension>).

The recursive value resolving should be fairly straightforward, we just have to resolve the left-hand side and right-hand side and then add them together.

[nicoburns]

@TimJentzsch Yeah, that'd be the simplest way to do it. It seems non-ideal to force two allocations for this feature though.

[TimJentzsch]

Hmm, trying to implement this, the biggest problem seems to be that Box doesn't implement Copy, so FlexboxLayout wouldn't be able to implement Copy anymore. That's probably quite bad for ergonomics.

[nicoburns]

Hmm, trying to implement this, the biggest problem seems to be that Box doesn't implement Copy, so FlexboxLayout wouldn't be able to implement Copy anymore. That's probably quite bad for ergonomics.

FWIW, I had to remove Copy from FlexboxLayout in my CSS Grid branch, because some of the Grid styles take Vec's (this could potentially be reinstated with ArrayVec, but only by limiting the size of Grids that Taffy supports and greatly inflating the size of FlexboxLayout). IIRC there was only 1 non-test line of code I had to update inside Taffy itself, and it was as simple as adding a .clone().

Not quite sure what the implications on the ergonomics for embedders of taffy would be. But I feel like FlexboxLayout perhaps ought not to be Copy anyway merely based on it's size?

[alice-i-cecile]

But I feel like FlexboxLayout perhaps ought not to be Copy anyway merely based on it's size?

Yeah, I'm fine with cutting this trait.

[TimJentzsch]

Ok thanks, I'll go ahead with prototyping an implementation then :)

[TimJentzsch]

@alice-i-cecile @nicoburns I have prototyped an implementation at https://github.com/DioxusLabs/taffy/pull/232, feedback would be much appreciated!

Unfortunately, Dimension loses Copy as well, which seems to have a much bigger impact with additional .clone() calls. I'm not yet sure if that's gonna be a problem on performance or anything else.

I do like though how it enables us to properly implement the Add, Sub, Mul and Div traits for ergonomic calculations though.