Skip to content

/ servo

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

Use Harfbuzz 1.0 and unicode-script for text shaping #7786

Merged
merged 4 commits into from Sep 29, 2015
Merged

Use Harfbuzz 1.0 and unicode-script for text shaping #7786

merged 4 commits into from Sep 29, 2015

Conversation

@mbrubeck
Copy link
Contributor

mbrubeck commented Sep 29, 2015

Depends on servo/rust-harfbuzz#53 and introduces a dependency on the new servo/unicode-script crate. r? @pcwalton

Review on Reviewable
@mbrubeck mbrubeck force-pushed the mbrubeck:harfbuzz-sys branch from ca949d8 to 71b45e3 Sep 29, 2015
@jdm jdm added the S-fails-tidy label Sep 29, 2015
@mbrubeck mbrubeck force-pushed the mbrubeck:harfbuzz-sys branch from 71b45e3 to 3f68fa5 Sep 29, 2015
@pcwalton
Copy link
Contributor

pcwalton commented Sep 29, 2015

I assume that Gecko also breaks text runs by Unicode script using the same algorithm?
@pcwalton
Copy link
Contributor

pcwalton commented Sep 29, 2015

Oh oops, didn't see your comment.

In that case, this looks good. r=me once @larsbergstrom reviews the HarfBuzz upgrade.
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Sep 29, 2015

HarfBuzz landed.
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Sep 29, 2015

@bors-servo r=pcwalton
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

📌 Commit 3f68fa5 has been approved by pcwalton
@mbrubeck mbrubeck removed the S-fails-tidy label Sep 29, 2015
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

⌛ Testing commit 3f68fa5 with merge 687e15e...
bors-servo pushed a commit that referenced this pull request Sep 29, 2015
bors-servo
Auto merge of #7786 - mbrubeck:harfbuzz-sys, r=pcwalton
687e15e 
Use Harfbuzz 1.0 and unicode-script for text shaping

Depends on servo/rust-harfbuzz#53 and introduces a dependency on the new servo/unicode-script crate.  r? @pcwalton

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7786)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

💔 Test failed - mac-dev-ref-unit
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Sep 29, 2015

mac-rel-wpt hit #7731:

/dom/nodes/Document-createElement-namespace.html
------------------------------------------------
TIMEOUT [Parent]

mac-dev-ref-unit hit #7785:

ol_japanese_iroha_a.html == ol_japanese_iroha_ref.html

mac-rel-css had two unexpected results which were previously a Linux-only PASS and Linux-only FAIL (now apparently the same on both platforms):

/css-flexbox-1_dev/html/css-flexbox-column-reverse.htm
------------------------------------------------------
FAIL [Parent]
/css21_dev/html4/bidi-glyph-mirroring-002.htm
---------------------------------------------
PASS expected FAIL [Parent]

linux-rel had a test-css failure that I can't reproduce locally (at least in a debug build), but I do see jumpiness when loading the test, so I suspect a race condition and/or incremental layout bug:

/css21_dev/html4/c62-percent-000.htm
------------------------------------
FAIL [Parent]
mbrubeck added 4 commits Sep 25, 2015
@mbrubeck
Upgrade to the new harfbuzz-sys crate
062493f
@mbrubeck
Break text runs by unicode script
371e689
@mbrubeck
Fix text-transform:capitalize across text runs
6304186
@mbrubeck
Fix shaping of tab characters with Harfbuzz 1.0
Loading status checks…
569b434 
Harfbuzz now renders tabs with a "missing character" glyph by default, so we
need to filter them out ourselves after computing an advance.
@mbrubeck mbrubeck force-pushed the mbrubeck:harfbuzz-sys branch from 3f68fa5 to 569b434 Sep 29, 2015
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Sep 29, 2015

@bors-servo r=pcwalton

Adjusted test expectations for Mac CSS results. Checking to see whether the Linux CSS failure was intermittent or not.
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

📌 Commit 569b434 has been approved by pcwalton
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

⌛ Testing commit 569b434 with merge a144d08...
bors-servo pushed a commit that referenced this pull request Sep 29, 2015
bors-servo
Auto merge of #7786 - mbrubeck:harfbuzz-sys, r=pcwalton
Loading status checks…
a144d08 
Use Harfbuzz 1.0 and unicode-script for text shaping

Depends on servo/rust-harfbuzz#53 and introduces a dependency on the new servo/unicode-script crate.  r? @pcwalton

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7786)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

💔 Test failed - linux-rel
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Sep 29, 2015

This time linux-rel got different results:

/css21_dev/html4/first-letter-dynamic-003a.htm
----------------------------------------------
PASS expected FAIL [Parent]

Since there were no code changes since the previous run, both results must be intermittent.
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Sep 29, 2015

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

⚡ Previous build results for android, gonk, linux-dev, mac-dev-ref-unit, mac-rel-css are reusable. Rebuilding only linux-rel, mac-rel-wpt...
This was referenced Sep 29, 2015
Closed
Closed
@bors-servo
Copy link
Contributor

bors-servo commented Sep 29, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo mentioned this pull request Sep 29, 2015
Merged
@bors-servo bors-servo merged commit 569b434 into servo:master Sep 29, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Sep 29, 2015
Merged
@mbrubeck mbrubeck deleted the mbrubeck:harfbuzz-sys branch May 11, 2016
@mbrubeck mbrubeck mentioned this pull request May 20, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@mbrubeck @pcwalton @larsbergstrom @bors-servo @jdm @SimonSapin @highfive
You can’t perform that action at this time.