naming of methods on blink::Length for detecting percentages, etc.

[David Baron]

There's currently work on (or consideration of) a few related features that make the space of what can be represented by a blink::Length a little bit more interesting:

1. CSS anchor positioning introduces the anchor() function which allows the offset properties to use positions relative to an anchor
2. The calc-size() function, whose purpose is to allow animations to/from auto and similar heights, allows the use of intrinsic size keywords within expressions like calc-size(auto, size * 0.5) and also allows the use of percentages in expressions like calc-size(any, 50%) that are not conceptually percentage-typed.
3. The progress() function and the ability to divide by values with units in calc() allow a type to be present within an expression but not part of the final type of the value.  (By the current spec, I think that "percentage"-ness is not erased, though.)

There are a few interesting characteristics of these features.

First, historically, Length::IsPercentOrCalc() has been used to mean "does this value have percentages", since any calc() expressions that were still present in computed values were those with percentages in them.  Both anchor() and calc-size() make this no longer true, since they introduce layout-dependent calc() values that are not percentages.

Second, there are places in layout code that intend to behave in a substantively different way when a length has percentages.  (I'll talk about this as the length's "type".)  The current spec for calc-size() offers what is effectively type erasure for percentageness:  it allows a value that has a percentage and does, in fact, depend on the container's size (or other percentage basis), but should not be treated for the purpose of any of these layout checks.  (The intrinsic sizing keywords are also each their own "type" for this sort of analysis, but things are a little bit simpler since there are slightly fewer features involved.)  It's possible that the progress() function and unit division could also offer this sort of type erasure, but I think the current spec says that they don't erase percentage-ness.

As part of cleaning up the callers of Length::IsPercentOrCalc, I realized that there's a somewhat subtle distinction between two different sorts of "this value has a percentage" tests:

1. There are tests that ask "is this value percentage-dependent" because if it is, they need to treat it as though the value were the default (commonly because the size used as the basis for percentages in indefinite).  The callers need this question to be answered correctly; it's a web-exposed behavior difference.
2. There are tests that ask "is this value percentage-dependent" because they are either (a) optimizing away computing the percentage basis if it isn't needed or (b) handling a change of the percentage basis and figuring out whether work needs to be redone.  Since these are optimizations, the callers need an answer to this question without false negatives (which an answer to the first question would produce in some cases), but false positives would just be missed optimizations and may be acceptable.

There are cases, definitely for calc-size() and maybe also for some of the other new calc() features, where the answer to question (1) is no but the answer to question (2) is yes.

(I'd also note that blink code often calls the percentage basis max_value, but I find that name misleading.)

The question I'm currently interested in opinions on is how to name two methods on Length so that the difference between these two concepts is clear and so that people writing new code that needs one or the other one do the right thing.

We could have callers who need the second one of these questions use the existing IsPercentOrCalc.  However, I think that's somewhat dangerous because callers would likely be tempted to use the method for the first question, which would be substantively wrong in some cases.  It also produces false positives that are relatively easily avoidable and might be relevant optimizations in the future if these features are widely used, and these uses might get tangled up with uses that actually mean Length::IsPercent() || Length::IsCalculated() for entirely different reasons.  So I'd like to eliminate Length::IsPercentOrCalc() since I think it's too easily misused.

One possible option for naming is something like Length::IsTypePercent() and Length::HasPercentDependence().  I'm curious if other folks (particularly folks who might be writing or reviewing layout code that cares about the distinction) have better ideas or thoughts.

I'd additionally note that IsTypePercent would have parallel methods for the intrinsic size keywords, e.g., IsTypeAuto, IsTypeFitContent, etc.  For example, IsTypeAuto() would be true  for either auto or for calc-size(auto, size * 0.5).

-David

[Tab Atkins Jr.]

> It's possible that the progress() function and unit division could also offer this sort of type erasure, but I think the current spec says that they don't erase percentage-ness.

Right, they shouldn't erase percentage-ness. calc-size() does so (when
the % is in the calc argument, not the basis argument) just to avoid
having the value depend on *two* intrinsic values, which can
potentially be impossible to resolve due to divergent behavior in the
two cases.

~TJ

[Morten Stenshorne]

David Baron <dba...@chromium.org> writes:

> There's currently work on (or consideration of) a few related features that make the space of what can be represented by a
> blink::Length a little bit more interesting:
>

> 1 CSS anchor positioning introduces the anchor() function which allows the offset properties to use positions relative to an
> anchor
> 2 The calc-size() function, whose purpose is to allow animations to/from auto and similar heights, allows the use of intrinsic size

> keywords within expressions like calc-size(auto, size * 0.5) and also allows the use of percentages in expressions like calc-size
> (any, 50%) that are not conceptually percentage-typed.

> 3 The progress() function and the ability to divide by values with units in calc() allow a type to be present within an expression

> but not part of the final type of the value. (By the current spec, I think that "percentage"-ness is not erased, though.)
>
> There are a few interesting characteristics of these features.
>
> First, historically, Length::IsPercentOrCalc() has been used to mean "does this value have percentages", since any calc()
> expressions that were still present in computed values were those with percentages in them. Both anchor() and calc-size() make
> this no longer true, since they introduce layout-dependent calc() values that are not percentages.
>
> Second, there are places in layout code that intend to behave in a substantively different way when a length has percentages.
> (I'll talk about this as the length's "type".) The current spec for calc-size() offers what is effectively type erasure for
> percentageness: it allows a value that has a percentage and does, in fact, depend on the container's size (or other percentage
> basis), but should not be treated for the purpose of any of these layout checks. (The intrinsic sizing keywords are also each their
> own "type" for this sort of analysis, but things are a little bit simpler since there are slightly fewer features involved.) It's
> possible that the progress() function and unit division could also offer this sort of type erasure, but I think the current spec says
> that they don't erase percentage-ness.
>
> As part of cleaning up the callers of Length::IsPercentOrCalc, I realized that there's a somewhat subtle distinction between two
> different sorts of "this value has a percentage" tests:
>

> 1 There are tests that ask "is this value percentage-dependent" because if it is, they need to treat it as though the value were the

> default (commonly because the size used as the basis for percentages in indefinite). The callers need this question to be
> answered correctly; it's a web-exposed behavior difference.

> 2 There are tests that ask "is this value percentage-dependent" because they are either (a) optimizing away computing the

> percentage basis if it isn't needed or (b) handling a change of the percentage basis and figuring out whether work needs to be
> redone. Since these are optimizations, the callers need an answer to this question without false negatives (which an answer to
> the first question would produce in some cases), but false positives would just be missed optimizations and may be
> acceptable.
>
> There are cases, definitely for calc-size() and maybe also for some of the other new calc() features, where the answer to question
> (1) is no but the answer to question (2) is yes.
>
> (I'd also note that blink code often calls the percentage basis max_value, but I find that name misleading.)

(max_value and maximum_value aren't good names, and I usually have to
look at the FloatValueForLength() implementation to figure out what it
actually means (especially when it's been a few years since last time -
the length resolution machinery in layout is pretty well self-contained
these days, so I usually don't have to worry about such details on a
daily basis, as long as I set up my ConstraintSpace correctly). :) Would
containing_block_size do? It's not quite simply a percentage basis,
since "auto" also uses it.)

> The question I'm currently interested in opinions on is how to name two methods on Length so that the difference between these
> two concepts is clear and so that people writing new code that needs one or the other one do the right thing.
>
> We could have callers who need the second one of these questions use the existing IsPercentOrCalc. However, I think that's
> somewhat dangerous because callers would likely be tempted to use the method for the first question, which would be
> substantively wrong in some cases. It also produces false positives that are relatively easily avoidable and might be relevant
> optimizations in the future if these features are widely used, and these uses might get tangled up with uses that actually mean
> Length::IsPercent() || Length::IsCalculated() for entirely different reasons. So I'd like to eliminate Length::IsPercentOrCalc() since
> I think it's too easily misused.

Yes, we should prevent ourselves from introducing suprises because of
old and bad habits, and new behavior. We need a new name as part of this
work.

> One possible option for naming is something like Length::IsTypePercent() and Length::HasPercentDependence(). I'm curious if
> other folks (particularly folks who might be writing or reviewing layout code that cares about the distinction) have better ideas or
> thoughts.
>
> I'd additionally note that IsTypePercent would have parallel methods for the intrinsic size keywords, e.g., IsTypeAuto,
> IsTypeFitContent, etc. For example, IsTypeAuto() would be true for either auto or for calc-size(auto, size * 0.5).

But IsTypePercent() would still include calc() values with percentages
in them, right? Which means that IsTypePercent() would return true in
other cases than <percentage>? Or is actually e.g. calc(5px + 70%)
considered to simply be a <percentage>?

Regarding HasPercentDependence(), this may still return false positives,
right? In that case, MayHavePercentDependence(), or something like that,
would be a better name.

Most importantly, this is tricky and subtle, and the new functions need
a few lines of documentation, since I don't think it's possible to come
up with self-explanatory names (that are still good and typeable names).

--
Morten Stenshorne, Software developer,
Blink/Layout, Google, Oslo, Norway

[Fredrik Söderquist]

I'd imagine the former (but maybe David can clarify his intention if not). I think the type has to seen as a set of types, which I guess would yield an IsTypePercent() being something like:

bool IsTypePercent() const {

  return type_ == kPercent || (type_ == kCalculated && GetCalculationValue().GetTypeSet().Contains(kPercent));

}

(inventing a little new interface there for illustrative purposes).

If there is potential for "overlap" between the IsType...() predicates (i.e multiple can return true for a given Length), then maybe that should be expressed in the name as well - I'm thinking Has...() vs. Is...() (as in "in the set" - Contains...() perhaps being a bit too long).

Regarding HasPercentDependence(), this may still return false positives,
right? In that case, MayHavePercentDependence(), or something like that,
would be a better name.

Agreed. The "uncertainty" in the name should help steer type-1 usage towards the other option(s).

/fs

[David Baron]

On Fri, Apr 5, 2024 at 5:26 AM Fredrik Söderquist <f...@opera.com> wrote:

On Friday, April 5, 2024 at 9:59:49 AM UTC+2 Morten Stenshorne wrote:

(max_value and maximum_value aren't good names, and I usually have to
look at the FloatValueForLength() implementation to figure out what it
actually means (especially when it's been a few years since last time -
the length resolution machinery in layout is pretty well self-contained
these days, so I usually don't have to worry about such details on a
daily basis, as long as I set up my ConstraintSpace correctly). :) Would
containing_block_size do? It's not quite simply a percentage basis,
since "auto" also uses it.)

The behavior of auto/stretch is often slightly different from the behavior of percentages (for example, handling of border and padding).  So I don't think the maximum_value passed in is used for any of these.  I think (and hope) the auto handling is separate.

 

> The question I'm currently interested in opinions on is how to name two methods on Length so that the difference between these
> two concepts is clear and so that people writing new code that needs one or the other one do the right thing.
>
> We could have callers who need the second one of these questions use the existing IsPercentOrCalc. However, I think that's
> somewhat dangerous because callers would likely be tempted to use the method for the first question, which would be
> substantively wrong in some cases. It also produces false positives that are relatively easily avoidable and might be relevant
> optimizations in the future if these features are widely used, and these uses might get tangled up with uses that actually mean
> Length::IsPercent() || Length::IsCalculated() for entirely different reasons. So I'd like to eliminate Length::IsPercentOrCalc() since
> I think it's too easily misused.

Yes, we should prevent ourselves from introducing suprises because of
old and bad habits, and new behavior. We need a new name as part of this
work.

Yes, I'm definitely planning to rename it.  I just wanted to get some opinions on the name first, so I don't end up renaming it three times within a week or two. :-)

 

> One possible option for naming is something like Length::IsTypePercent() and Length::HasPercentDependence(). I'm curious if
> other folks (particularly folks who might be writing or reviewing layout code that cares about the distinction) have better ideas or
> thoughts.
>
> I'd additionally note that IsTypePercent would have parallel methods for the intrinsic size keywords, e.g., IsTypeAuto,
> IsTypeFitContent, etc. For example, IsTypeAuto() would be true for either auto or for calc-size(auto, size * 0.5).

But IsTypePercent() would still include calc() values with percentages
in them, right? Which means that IsTypePercent() would return true in
other cases than <percentage>? Or is actually e.g. calc(5px + 70%)
considered to simply be a <percentage>?

I'd imagine the former (but maybe David can clarify his intention if not). I think the type has to seen as a set of types, which I guess would yield an IsTypePercent() being something like:

bool IsTypePercent() const {

  return type_ == kPercent || (type_ == kCalculated && GetCalculationValue().GetTypeSet().Contains(kPercent));

}

(inventing a little new interface there for illustrative purposes).

If there is potential for "overlap" between the IsType...() predicates (i.e multiple can return true for a given Length), then maybe that should be expressed in the name as well - I'm thinking Has...() vs. Is...() (as in "in the set" - Contains...() perhaps being a bit too long).

So the method for (1) is already implemented as Length::HasPercent() (working in a manner similar to what fs suggests), and I've already landed changes to make many callers use that.  However, when I was converting those callers I realized that many callers wanted a method for checking percentage dependence that was not HasPercent, which is what led me to start this thread.  My concern with HasPercent is that callers who want HasPercentDependence (or whatever we call it) will use HasPercent instead, and that's incorrect for the cases where calc-size() does type-erasure of percentages inside of it.

 

Regarding HasPercentDependence(), this may still return false positives,
right? In that case, MayHavePercentDependence(), or something like that,
would be a better name.

Agreed. The "uncertainty" in the name should help steer type-1 usage towards the other option(s).

Yes, I think if the implementation stays as it is now, MayHavePercentDependence is good.  Following a CL that I submitted yesterday (after sending the original message in this thread) I believe all remaining callers of Length::IsPercentOrCalc are the ones that want Length::MayHavePercentDependence, so a simple step I could take now is (I think) renaming IsPercentOrCalc to MayHavePercentDependence, and leaving comments both explaining what it means and explaining that we could make it exact (HasPercentDependence) later if we think it's worthwhile.

 

Most importantly, this is tricky and subtle, and the new functions need
a few lines of documentation, since I don't think it's possible to come
up with self-explanatory names (that are still good and typeable names).

Yes, I plan to document them (and partly already have).  But I think the names are important too, since not everyone will read the documentation. :-/

-David