style: Refactor and add infrastructure for font metrics in style. #14174

emilio · 2016-11-11T19:04:40Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors

These changes do not require tests because moves stuff around without adding functionality.

This commit itself only moves things around and adds an extra parameter to the
apply_declarations function to eventually handle #14079 correctly.

Probably needs a more granular API to query fonts, á la nsFontMetrics, but
that's trivial to do once this is landed.

Then we should make the font provider mandatory, and implement the missing stylo
bits.

This change is

emilio · 2016-11-11T19:05:00Z

r? @bholley (or @SimonSapin or @Manishearth).

emilio · 2016-11-11T19:05:29Z

@bors-servo try

bors-servo · 2016-11-11T19:05:33Z

⌛ Trying commit 67de820 with merge 6a01ab8...


        Auto merge of #14174 - emilio:font-provider, r=<try>

style: Refactor and add infrastructure for font metrics in style.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors  - [x] These changes do not require tests because moves stuff around without adding functionality.  This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14174)

emilio · 2016-11-11T19:22:53Z

Bobby said he'd need to do a bit of investigation to review this, r? @Manishearth

emilio · 2016-11-11T19:24:35Z

@bors-servo try

bors-servo · 2016-11-11T19:24:37Z

⌛ Trying commit 42946f7 with merge 4a5a652...


        Auto merge of #14174 - emilio:font-provider, r=<try>

style: Refactor and add infrastructure for font metrics in style.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors  - [x] These changes do not require tests because moves stuff around without adding functionality.  This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14174)

bors-servo · 2016-11-11T19:32:34Z

⛄ The build was interrupted to prioritize another pull request.

bors-servo · 2016-11-11T19:32:36Z

⌛ Trying commit 42946f7 with merge abd659c...


        Auto merge of #14174 - emilio:font-provider, r=<try>

style: Refactor and add infrastructure for font metrics in style.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors  - [x] These changes do not require tests because moves stuff around without adding functionality.  This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14174)

bors-servo · 2016-11-11T19:32:41Z

💔 Test failed - linux-rel-css

emilio · 2016-11-11T19:59:24Z

@bors-servo retry

Git failure

bors-servo · 2016-11-11T19:59:26Z

⌛ Trying commit 42946f7 with merge d486b87...


        Auto merge of #14174 - emilio:font-provider, r=<try>

style: Refactor and add infrastructure for font metrics in style.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors  - [x] These changes do not require tests because moves stuff around without adding functionality.  This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14174)

emilio · 2016-11-11T21:48:02Z

@bors-servo try

Git failure again, rebased

bors-servo · 2016-11-11T21:48:05Z

⌛ Trying commit ec0450a with merge bc75819...


        Auto merge of #14174 - emilio:font-provider, r=<try>

style: Refactor and add infrastructure for font metrics in style.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors  - [x] These changes do not require tests because moves stuff around without adding functionality.  This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14174)

bors-servo · 2016-11-11T23:10:37Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

Manishearth

r=me | nits

Manishearth · 2016-11-13T04:03:49Z

components/style/font_metrics.rs

+
+/// Represents the font metrics that style needs from a font to compute the
+/// value of certain CSS units like `ex`.
+#[derive(Debug, PartialEq, Clone)]


Probably Copy too

Nope, that's intentional, just in case it becomes bigger, or a RefPtr to a gecko struct, or whatnot.

Manishearth · 2016-11-13T04:03:49Z

components/style/values/specified/length.rs

+
+                // FIXME(emilio): text-orientation: upright is only in Gecko
+                // right now.
+                let vertical = wm.is_vertical() || wm.is_vertical_lr();


vertical implies vertical_lr iirc

I think sideways values for both writing-mode and text-orientation will count as not-vertical here, since the text itself is in line with the inline flow. Not sure. We don't really support this yet (#14144). But perhaps we should leave a comment here to do that when this lands.

Added a comment pointing to that issue.


        style: Refactor and add infrastructure for font metrics in style.

This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.

emilio · 2016-11-13T10:55:00Z

@bors-servo r=Manishearth

bors-servo · 2016-11-13T10:55:01Z

📌 Commit 6c34587 has been approved by Manishearth

bors-servo · 2016-11-13T10:55:03Z

⌛ Testing commit 6c34587 with merge 57c4db7...


        Auto merge of #14174 - emilio:font-provider, r=Manishearth

style: Refactor and add infrastructure for font metrics in style.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors  - [x] These changes do not require tests because moves stuff around without adding functionality.  This commit itself only moves things around and adds an extra parameter to the `apply_declarations` function to eventually handle #14079 correctly. Probably needs a more granular API to query fonts, á la nsFontMetrics, but that's trivial to do once this is landed. Then we should make the font provider mandatory, and implement the missing stylo bits.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14174)

bors-servo · 2016-11-13T12:04:33Z

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev

highfive assigned bholley Nov 11, 2016

highfive assigned Manishearth and unassigned bholley Nov 11, 2016

emilio force-pushed the emilio:font-provider branch from 67de820 to 42946f7 Nov 11, 2016

highfive added the S-awaiting-review label Nov 11, 2016

highfive added the S-tests-failed label Nov 11, 2016

emilio removed the S-tests-failed label Nov 11, 2016

emilio force-pushed the emilio:font-provider branch from 42946f7 to ec0450a Nov 11, 2016

Manishearth approved these changes Nov 13, 2016

View changes

emilio force-pushed the emilio:font-provider branch from ec0450a to 6c34587 Nov 13, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Nov 13, 2016

bors-servo merged commit 6c34587 into servo:master Nov 13, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Nov 13, 2016

bors-servo mentioned this pull request Nov 13, 2016

implement support for the width query #14192

Merged

4 of 5 tasks complete

emilio mentioned this pull request Nov 18, 2016

Style needs a way to get font metrics while computing style data. #14079

Open

emilio deleted the emilio:font-provider branch Jan 23, 2017

servo / servo

style: Refactor and add infrastructure for font metrics in style. #14174

style: Refactor and add infrastructure for font metrics in style. #14174

emilio commented Nov 11, 2016 •

edited by larsbergstrom

emilio commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

emilio commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

Manishearth left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

emilio commented Nov 13, 2016

bors-servo commented Nov 13, 2016

bors-servo commented Nov 13, 2016

bors-servo commented Nov 13, 2016

servo / servo

Join GitHub today

style: Refactor and add infrastructure for font metrics in style. #14174

style: Refactor and add infrastructure for font metrics in style. #14174

Conversation

emilio commented Nov 11, 2016 • edited by larsbergstrom

emilio commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

emilio commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

emilio commented Nov 11, 2016

bors-servo commented Nov 11, 2016

bors-servo commented Nov 11, 2016

Manishearth left a comment

This comment has been minimized.

Manishearth Nov 13, 2016

This comment has been minimized.

emilio Nov 13, 2016

This comment has been minimized.

Manishearth Nov 13, 2016

This comment has been minimized.

Manishearth Nov 13, 2016 • edited

This comment has been minimized.

emilio Nov 13, 2016

emilio commented Nov 13, 2016

bors-servo commented Nov 13, 2016

bors-servo commented Nov 13, 2016

bors-servo commented Nov 13, 2016

emilio commented Nov 11, 2016 •

edited by larsbergstrom

Manishearth Nov 13, 2016 •

edited