Implement CSS calc()

[paulrouget]

https://drafts.csswg.org/css-values/#calc-notation

[boghison]

I am interested in this, but I've never done anything CSS related, how does that work in servo? Could anyone elaborate?

[jdm]

The code that interprets the output of the CSS parser lives in http://mxr.mozilla.org/servo/source/components/style/ . Given It can be used wherever <length>, <frequency>, <angle>, <time>, <number>, or <integer> values are allowed from the spec, that will likely mean modifying lots of methods for parsing particular value types (like Angle::parse, for example). There will also need to be new code that parses a calc function when a Function token is encountered in these places (https://github.com/servo/rust-cssparser/blob/master/src/tokenizer.rs#L120). @SimonSapin probably has further ideas.

[boghison]

OMG this is too horrifying

[SimonSapin]

IRC chat with @dzbarsky today:

<dzbarsky> SimonSapin: I'm looking at adding calc() support, and it seems like we need a calc token in the tokenizer to parse calc expressions. it seems a little weird since every token type has a link to the syntax spec but calc is separate, but it still makes sense
<SimonSapin> dzbarsky: I’m not convinced it should be a token
<SimonSapin> dzbarsky: We already have Length::parse and similar methods. These should handle calc. And more should be added like parse_integer which should be used instead of looking for number tokens directly
<dzbarsky> SimonSapin: I looked at Length::parse, it examines the tokens. So without making a calc token, calc will be a function, right? I haven't looked at how those are handled yet, maybe that's ok
<~jdm> dzbarsky: correct
<~jdm> dzbarsky: then there should be a parse_calc that deals with all of the tokens inside of it and order of operations
<SimonSapin> dzbarsky: Length::parse will need to look for Function tokens and use https://servo.github.io/rust-cssparser/cssparser/struct.Parser.html#method.parse_nested_block
<SimonSapin> (function arguments, like bare parens, are considered a "block" since the nesting rules are the same as with { … } and [ … ])
<dzbarsky> SimonSapin: hmm, this is tricky. so I added a Calc variant to LengthOrPercentage, where Calc is stuct { left: LengthOrPercentage, right: LengthOrPercentage}
<dzbarsky> SimonSapin: now the problem is Calc needs to implement Copy for LengthOrPercentage to implement Copy. Calc also needs to stores boxes to avoid recursive definitions, which means it can't implement Copy (i think?)
<dzbarsky> SimonSapin: I guess I can try to implement Copy myself instead of auto-deriving?
<SimonSapin> dzbarsky: No, Copy is a marker, it has no method. If derive(Copy) doesn’t work, a type just can’t be Copy (but it might be Clone)
<dzbarsky> nevermind that doesn't make sense
<dzbarsky> so why do we need to have values implement Copy anyway?
<dzbarsky> I guess it's more efficient than Clone
<SimonSapin> dzbarsky: that said, you probably don’t need to keep the full AST around. Since multiplication and division can only by a unitless number, you can simplify everything to a sum.
<SimonSapin> for computed::LengthOrPercentage in particular, it a sum of length and a percentage, so two scalar fields
<SimonSapin> (specified values are the same principle, just with more fields since there are more units whose relation to each other is not known yet)
<dzbarsky> SimonSapin: for computed yes, for specified i'm not sure how that would work
<dzbarsky> I guess you have one for each possible dimension unit?
<dzbarsky> and a percent
<SimonSapin> yes
<SimonSapin> one for each "recursive" variant of specified::Length
<dzbarsky> right
<SimonSapin> https://drafts.csswg.org/css-values/#calc-type-checking describes how to determine a resolved type from an arbitrary calc() expression and then see if it’s valid in context, but in our current parser I think it makes more sense to start from what type is expected in that context and parse accordingly
<dzbarsky> SimonSapin: any guesses what |div.style.width = 'calc(50% + calc(30px + 5%))'; alert(div.style.width)| does in firefox and chrome?
<~jdm> dzbarsky: presumably something different than the spec says serialization should do?
<dzbarsky> (I was wondering if we can actually get away with simplifying the AST)
<dzbarsky> "calc(100% + 30px)" in Firefox, "" in Chrome :)
<SimonSapin> dzbarsky: https://drafts.csswg.org/css-values/#calc-serialize says what they should do :)
<dzbarsky> yeah, i saw
<SimonSapin> or must do, in RFC2119 terms
<dzbarsky> heh

[dzbarsky]

<dzbarsky> SimonSapin: woohoo! min-width: calc(100px + 60%); renders correctly
now i just need to write an actual calc parser ;)
<pcwalton> dzbarsky: that was fast
<pcwalton> I figured calc would take forever and we'd just need to tell paul to work around it :)
<dzbarsky> I made the parser only look for 2 components separated by a +
but the actual layout parts are pretty easy
<pcwalton> ship it
<dzbarsky> also it only works on LengthOrPercentage so unless you want to calc min-width or font-size you're outta luck
<pcwalton> ship it

https://github.com/dzbarsky/servo/tree/calc

[dzbarsky]

Also implemented for LengthOrPercentageOrAuto, so it should work with most sizing properties except max-width and max-height. @paulrouget let me know if you need anything else for browser.html

[paulrouget]

That was fast :)

A typical use of calc in browser.html: calc(100vw - 10px).

Apparently, supporting vw and vh won't be part of the first implementation. We can work around that though.

[dzbarsky]

I added support for viewport relative and font relative units, and also rewrote the parser so your typical use should now work :)

[paulrouget]

@dzbarsky your branch doesn't compile:

/Users/paul/github/servo/components/style/values.rs:526:51: 526:58 error: mismatched types:
 expected `values::specified::CalcProductNode`,
    found `&values::specified::CalcProductNode`
(expected struct `values::specified::CalcProductNode`,
    found &-ptr) [E0308]
/Users/paul/github/servo/components/style/values.rs:526                 match try!(Calc::simplify_product(product)) {

[dzbarsky]

@paulrouget Sorry, made a list minute change and didn't bother compiling. It builds now, but you'll probably need to force pull.

[paulrouget]

@dzbarsky It looks to work pretty well. What would it take to land that?

[SimonSapin]

#7185 is in my review queue