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

Snap text baseline to nearest pixel before positioning glyphs #12270

Closed
wants to merge 1 commit into from

Conversation

@mbrubeck
Copy link
Contributor

mbrubeck commented Jul 5, 2016

Fixes a test failure on Mac caused by sub-pixel-positioned glyphs being rounded in different directions depending on the baseline offset. r? @pcwalton


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@pcwalton
Copy link
Contributor

pcwalton commented Jul 5, 2016

@bors-servo: r+

Wow, this looks like an annoying bug.

@bors-servo
Copy link
Contributor

bors-servo commented Jul 5, 2016

📌 Commit 038aae5 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

The latest upstream changes (presumably #12233) made this pull request unmergeable. Please resolve the merge conflicts.

@mbrubeck mbrubeck force-pushed the mbrubeck:baseline-snap branch from 038aae5 to b4186f9 Jul 6, 2016
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jul 6, 2016

@bors-servo r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

📌 Commit b4186f9 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

Testing commit b4186f9 with merge f54a365...

bors-servo added a commit that referenced this pull request Jul 6, 2016
Snap text baseline to nearest pixel before positioning glyphs

Fixes a test failure on Mac caused by sub-pixel-positioned glyphs being rounded in different directions depending on the baseline offset. r? @pcwalton

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12270)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 6, 2016

💔 Test failed - mac-rel-wpt

@highfive
Copy link

highfive commented Jul 6, 2016

  ▶ FAIL [expected PASS] /html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested.html
  └   → /html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested.html 7f7497f5e2dcc217d5b8346811e7b90f821dcb2d
/html/semantics/text-level-semantics/the-bdi-element/bdi-neutral-nested-ref.html 0bf15428edeeb4c8a89af3ed21f44ea3b41665b8
Testing 7f7497f5e2dcc217d5b8346811e7b90f821dcb2d == 0bf15428edeeb4c8a89af3ed21f44ea3b41665b8

  ▶ FAIL [expected PASS] /_mozilla/css/linebreak_inline_span_a.html
  └   → /_mozilla/css/linebreak_inline_span_a.html f31373081a431ec2a3c46a417de2a5744d188fe9
/_mozilla/css/linebreak_inline_span_b.html 773387c053153835bfa79e2b2f8a46722f91fcb8
Testing f31373081a431ec2a3c46a417de2a5744d188fe9 == 773387c053153835bfa79e2b2f8a46722f91fcb8

  ▶ FAIL [expected PASS] /_mozilla/css/pseudo_element_a.html
  └   → /_mozilla/css/pseudo_element_a.html b79ac791c067d47506a75a3ebe60798193ecc3e2
/_mozilla/css/pseudo_element_b.html 89d3099b0b355148372151df447eb36eaa27b472
Testing b79ac791c067d47506a75a3ebe60798193ecc3e2 == 89d3099b0b355148372151df447eb36eaa27b472

  ▶ FAIL [expected PASS] /_mozilla/css/quotes_simple_a.html
  └   → /_mozilla/css/quotes_simple_a.html 7151f0436ee39bdef9907b9cf9f1adeeb6810031
/_mozilla/css/quotes_simple_ref.html 7f30834a86fea6b15f1dfe941f1904219beb1dc3
Testing 7151f0436ee39bdef9907b9cf9f1adeeb6810031 == 7f30834a86fea6b15f1dfe941f1904219beb1dc3
@emilio
Copy link
Member

emilio commented Nov 10, 2016

@bors-servo try

Should this be closed or we still want this patch @mbrubeck?

@bors-servo
Copy link
Contributor

bors-servo commented Nov 10, 2016

Trying commit b4186f9 with merge a0e3130...

bors-servo added a commit that referenced this pull request Nov 10, 2016
Snap text baseline to nearest pixel before positioning glyphs

Fixes a test failure on Mac caused by sub-pixel-positioned glyphs being rounded in different directions depending on the baseline offset. r? @pcwalton

---

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->

---

This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12270)

<!-- Reviewable:end -->
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Nov 10, 2016

We still need something like this patch, but I'm not sure whether this is the exact right solution. I don't currently have a working macOS dev environment, so it would help if someone can take this over. (If not, I can work on getting a Mac set up again…)

@bors-servo
Copy link
Contributor

bors-servo commented Nov 11, 2016

💥 Test timed out

@jdm
Copy link
Member

jdm commented Jan 27, 2017

Tracking it in #15272.

@jdm jdm closed this Jan 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.