Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-values] Consider requiring same relative <length> units in atan2() #8169

Closed
emilio opened this issue Dec 1, 2022 · 24 comments
Closed

Comments

@emilio
Copy link
Collaborator

emilio commented Dec 1, 2022

Consider atan2(10em, 13vh). Should that work? Per spec, it should because the two units are <length>s, and the spec says:

The atan2(A, B) function contains two comma-separated calculations, A and B. A and B can resolve to any <number>, <dimension>, or <percentage>, but must have the same type, or else the function is invalid.

If we allow different units, we can't resolve atan() at parse-time (because it starts depending on resolving those <length>s to pixels). That introduces a new set of property dependencies which didn't exist before. For example, we'd need to define how ex/etc units resolve on font-weight, since font-weight can affect the primary font, and with this you could do font-weight: atan2(2ex, 10px).

My implementation in Firefox doesn't allow mixed relative units in atan2() (because I misread the spec). This is also problematic for Chromium implementation-wise (even without all these new dependency shenanigans).

Unless this is extremely useful, it might be worth restricting relative lengths to enforce same-unit-ness.

cc: @tabatkins @DevSDK @dbaron

@emilio emilio changed the title [css-values] Consider requiring same <length> units in atan2() [css-values] Consider requiring same relative <length> units in atan2() Dec 1, 2022
@tabatkins
Copy link
Member

Yeah, I think this is pretty important - for example, atan2(10vw, 10vh) makes complete sense to me.

But more importantly, this isn't an atan2()-specific issue; it applies to everything taking a calculation. font-weight: calc(2ex / 10px), for example, introduces the exact same problem. So I'm strongly against any atan2()-targeted change here; if we want to disallow this we need to instead apply some more powerful restriction to all calculations.

@Loirooriol
Copy link
Contributor

Not sure if I follow, is the "primary font" the same as the "first available font"? Then, what about font-weight: 2ex without calculations, the 2ex resolves depending on the "first available font", which depends on font-weight?

@xiaochengh
Copy link
Contributor

We can't plug relative length units into font-weight without calculations, because font-weight takes number values only.

I think we can just use the same idea how we resolve font-size: 1ex: when a font-relative length unit is used in a font-related property, it refers to the the parent element's font.

@Loirooriol
Copy link
Contributor

Ah, sure, time to go to sleep I guess XD. But I think this would also need to address custom properties registered as <integer> or <number> (https://drafts.css-houdini.org/css-properties-values-api/#dependency-cycles):

--foo: calc(2ex / 10px);
font-weight: var(--foo);

@Loirooriol
Copy link
Contributor

Also:

line-height: 1ex;
font-weight: calc(1lh / 1px);

So, generalizing #2115, I guess the idea would be:

  1. Compute font-size, resolving unknown font units with the parent. This sets the em unit.
  2. Compute font-weight, resolving unknown font units with the parent. This sets the ex, cap, ch and ic units.
  3. Compute line-height, resolving unknown font units with the parent. This sets the lh unit.

Or:

  1. Compute font-weight, resolving unknown font units with the parent.
  2. Compute font-size, resolving unknown font units with the parent. This sets the em, ex, cap, ch and ic units.
  3. Compute line-height, resolving unknown font units with the parent. This sets the lh unit.

@Loirooriol Loirooriol added css-values-4 Current Work and removed css-values-3 labels Dec 2, 2022
@Loirooriol
Copy link
Contributor

Retagging to values-4 since values-3 doesn't have atan2 nor unit algebra.

@emilio
Copy link
Collaborator Author

font-style needs to also be special cased along with font-weight at least, since you would be allowed to set a slant anglebased on any unit. Probably others as well.

@Loirooriol
Copy link
Contributor

font-stretch too I guess. https://drafts.csswg.org/css-fonts-4/#font-prop-desc

@Loirooriol
Copy link
Contributor

All of this seems problematic:

font-weight: calc(1ex / 1px);
font-style: calc(1ex / 1px * 1deg);
font-stretch: calc(1ex / 1px * 1%);
font-variation-settings: "wght" calc(1ex / 1px);
font-feature-settings: "subs" calc(1ex / 1px);
font-size-adjust: ex-height calc(1ex / 1px);

@fantasai fantasai added this to By Theme in Agenda Scratchpad Dec 6, 2022
@yukulele
Copy link

Unless this is extremely useful, it might be worth restricting relative lengths to enforce same-unit-ness.

In this case the units would be useless because atan2(X, Y) = atan2(Xunit, Yunit):

rotate: atan2(12, 7)     /* = 1.04272rad */
rotate: atan2(12%, 7%)   /* = 1.04272rad */
rotate: atan2(12em, 7em) /* = 1.04272rad */
rotate: atan2(12vw, 7vw) /* = 1.04272rad */
rotate: atan2(12ms, 7ms) /* = 1.04272rad */
...

@yukulele
Copy link

possible duplicate of 7482

@tabatkins
Copy link
Member

Nah, #7482 was a question about what the behavior was. This is an issue asking if we should change the behavior.

@astearns astearns added this to Overflow in March 2023 VF2F Mar 9, 2023
@xiaochengh
Copy link
Contributor

Related: what if we use them in animation properties? For example

.test {
  animation-duration: calc(sin(atan2(1em, 20px)) * 1s);
  animation-name: anim;
}

@keyframes anim {
  from { font-size: 10px; }
  to { font-size: 30px; }
}

Do they need the same "animation-tainted" treatment as in custom properties?

@emilio
Copy link
Collaborator Author

sign() has the same issue.

@tabatkins
Copy link
Member

tabatkins commented May 11, 2023

Yes, as I said in my earlier comment, it applies to every function taking a calculation, including calc() itself. This is a generic consequence of unit algebra letting you use units in the input that don't appear in the output - calc(1ex / 1em) trips the exact same issue.

So this issue is moot; there is no meaningful restriction we can place on these functions that would actually solve the problem. This should be brought up as a question of what precisely the consequences are of using a unit in a math function in these funky cases.

@DevSDK
Copy link

@Loirooriol

I'm trying to understand given example why has circular dependency:

line-height: 1ex;
font-weight: calc(1lh / 1px);

Can I ask the spec references that the font-weight affects x-height?

@emilio
Copy link
Collaborator Author

@DevSDK any font which affects the primary font selection can change the first available font, which at least in theory can affects font metrics, including x-height...

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-values] Consider requiring same relative <length> units in atan2(), and agreed to the following:

  • RESOLVED: font-relative units resolve against the parent element when used in any font-* property (and anything else that affects font selection)
  • RESOLVED: Do something similar for line-height (details TBD)
The full IRC log of that discussion <fantasai> scriben+
<fantasai> scribe+
<fantasai> TabAtkins: Emilio brought up originally, if your two calculations in atan2() have different units with potentially different calculation times (e.g. 'em' vs 'px'), can cause circular dependency issues
<fantasai> TabAtkins: this washes the units away into plain numbers, allowing them to be used in more places
<fantasai> TabAtkins: but if you use this property in other properties that affect resolution of those units it's a problem
<fantasai> TabAtkins: agree it's an issue, but not specific to atan2()
<fantasai> TabAtkins: it's a property of calc() unit division in general
<fantasai> TabAtkins: e.g. calc(1em/1ex) is a number
<fantasai> TabAtkins: Agree this is an issue, but we need to solve generally
<fantasai> TabAtkins: not address atan2() specifically
<fantasai> emilio: I agree
<fantasai> emilio: Firefox considers atan2() of two different lengths that aren't absolute, we consider that invalid and don't parse it
<TabAtkins> yeah that exact restriciton is *nonsensical*
<fantasai> emilio: I guess my question is, at least WebKit is completely broken and just ignores the units
<fantasai> emilio: this is an issue, but we shouldn't parse different units if we don't support the units
<fantasai> emilio: agree it's a general issue
<fantasai> TabAtkins: for the font-dependent units, when used directly where they can affect the resolution
<fantasai> TabAtkins: we resolve against parent element instead, and this resolves the circular dependency
<fantasai> TabAtkins: Can we do that here?
<fantasai> emilio: Using the parent makes sense
<fantasai> emilio: It's a little bit tricky
<fantasai> emilio: Note that implementation-wise this isn't only issue
<astearns> q+
<TabAtkins> 1) font-dependent units are invalid in proeprties where they're problematic, or 2) font-dependent units resolve based on parent metrics instead
<fantasai> emilio: also now things that can resolve to a number resolve eagerly, but with these units cannot
<fantasai> emilio: I agree resolving to parent font is less ugly solution
<fantasai> emilio: whether that's enough for custom properties etc, I need to think... probably it is?
<fantasai> emilio: Would be great to have a central place for these dependencies
<fantasai> astearns: Wanted clarification on using parent value for these things
<fantasai> astearns: that's in all contexts, not just when using calc() in problematic property?
<fantasai> TabAtkins: No, that would be fundamentally incompatible with existing usage of calc()
<fantasai> TabAtkins: using width in 'em' uses element's own font size, correct and necessary to maintain
<fantasai> TabAtkins: this is wrt font-dependent units, or other property-dependent units
<fantasai> astearns: Concern that if you use same function in two different properties, you might get different results
<astearns> ack astearns
<fantasai> TabAtkins: if you use font-size: 2em; and width: 2em they're different
<fantasai> TabAtkins: this just expands the set of properties affected, because unit algebra allows putting them in places not currently accepting units
<astearns> ack fantasai
<TabAtkins> fantasai: i agree - ideally doing same as font-size makes the most sense. probablyl should say generally for font-relative units in the font-* properties
<TabAtkins> fantasai: dont' think there's anything reasonable to do besides making them invalid, which is less godd
<TabAtkins> fantasai: other a little complex is line-height unit
<TabAtkins> fantasai: unlike font-size, line-height accepts font-relative units, but produces the lh unit. forget how we resolved that circularity
<dbaron> s/accepts font-relative units/treats font-relative units relative to the current element/
<fantasai> TabAtkins: Emilio also wanted a centralized location for all the dependencies we have
<fantasai> TabAtkins: We do have a wiki page tracking these, could move into appendix of Values?
<emilio> wfm
<TabAtkins> https://wiki.csswg.org/spec/property-dependencies
<emilio> fantasai: not all of the dependencies are unit dependencies
<emilio> ... not sure those make sense in values
<fantasai> astearns: put in definition of units?
<fantasai> emilio: discussing things like position affecting display, etc.
<fantasai> TabAtkins: tracking unit dependencies is what we need to define for this, could limit to addressing that
<fantasai> fantasai: That's easy, it's just everything in font-relative units section
<fantasai> astearns: So first is to define set of properties that affect font-relative units?
<TabAtkins> fantasai: so that's all the font-* properties, anything affecting font selection, affects 'ex' and similar units
<TabAtkins> fantasai: I think we just generally say all font properties need to look up the parent
<TabAtkins> fantasai: we didn't say this before because only font-size could take a numeric value, now many can
<TabAtkins> astearns: so you're arguing against a list of properties?
<TabAtkins> fantasai: Yeah we're gonna add more props over time. should literally just say "all the font-* props"
<TabAtkins> fantasai: So no fancy table needed, just say "all the font properties".
<TabAtkins> TabAtkins: and do something for line-height
<TabAtkins> fantasai: yes
<TabAtkins> astearns: so proposed reoslution is if you use the font-relative units in a math fucntion...
<TabAtkins> fantasai: No, not just in a function. Any usage. We might expand usage in the future.
<TabAtkins> astearns: concerns with the reoslution?
<TabAtkins> TabAtkins: i'll run the exact test by emilio when i get some text down
<TabAtkins> emilio: what happens with length-valued custom props?
<TabAtkins> emilio: if you use ems in a custom prop that takes <length>, and then use it in font-size...
<TabAtkins> fantasai: the substiution happens before computation...
<fantasai> s/happens/should happen/
<TabAtkins> emilio: but it has to compute to something
<TabAtkins> emilio: if you getComputedStyle() it has to return a length
<TabAtkins> TabAtkins: yeah there might be an issue there, i'll review
<TabAtkins> astearns: so proposed resolution is if you use font-relative units in font-* properties, you get the value from teh parent
<TabAtkins> astearns: objections?
<fantasai> ACTION TabAtkins: Figure out what to do on custom properties typed as lengths
<TabAtkins> RESOLVED: font-relative units resolve against the parent element when used in any font-* property (and anything else that affects font selection)
<fantasai> astearns: line-height?
<fantasai> TabAtkins: We need to dig into exactly what that works out to
<fantasai> RESOLVED: Do something similar for line-height (details TBD)
<fantasai> astearns: container query units?
<fantasai> TabAtkins: by definition, they're not circular because already resolvign against an ancestor eleent
<fantasai> s/eleent/element

@Loirooriol
Copy link
Contributor

ACTION TabAtkins: Figure out what to do on custom properties typed as lengths

The problem is not just for custom properties registered as lengths, see w3c/css-houdini-drafts#1080

@Loirooriol
Copy link
Contributor

Loirooriol commented Jul 12, 2023

Mild concern with the resolution: this creates an inconsistency with #2115, i.e.

font-size: 2lh; /* resolves using the parent */
line-height: 3em; /* resolves using the current element */
font-size: 2ex; /* resolves using the parent */
font-weight: calc(3em / 1px); /* resolves using the parent */

Since font-size always resolves font-relative units with the parent, it should be possible to resolve em with the current element both for line-height and font-sizefont-weight, so why do different things?

@fantasai
Copy link
Collaborator

@Loirooriol (I assume you mean font-weigh in your last sentence.) It would be really bad if, in font-weight, 3em resolved against the element itself but 3ex resolved against the parent. Since font-weight can affect the calculation of ex, ex has to resolve against the parent.

@Loirooriol
Copy link
Contributor

@fantasai Why would resolving em and ex against different elements be really bad, but resolving em and lh against different elements is fine? Should all units resolve against the parent when there is a cycle?

@tabatkins
Copy link
Member

We can't change the behavior of line-height: 1em; in general (it uses current-element ems), and special-casing it to change behavior if and only if font-size was in lh is a difficult-to-predict, action-at-a-distance sort of thing that we generally want to avoid. But we have to break the cycle somehow, so having font-size: 1lh; always resolve against the parent was the most reasonable thing to do.

Digging in a little deeper, tho, while line-height and font-size usually establish a dependency, they're not semantically connected - the font size and the height of a line don't readily appear to be different aspect of the same concept, at least imo. It's thus more acceptable to have them differ on which element they refer to, in these cases.

On the other hand, em and ex (and all the other explicitly font-based units) are conceptually related; they're all sizing according to some text, and it's not uncommon for them to be used in parallel fashions. Having them differ on what element they refer to would make the grouping conceptually incoherent.

@fantasai fantasai removed this from By Theme in Agenda Scratchpad Dec 13, 2023
@yisibl
Copy link
Contributor

yisibl commented Dec 28, 2023

The ability to convert between different units in atan2() is very useful, E.g. we can now do a pure CSS real-time display of the container width(px).

Demo: https://codepen.io/xboxyan/pen/MWZLYZz

2023-12-28.20-07-41.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

9 participants