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

Fix intermittent macOS reftest failures #24347

Merged
merged 1 commit into from Oct 3, 2019
Merged

Conversation

@jdm
Copy link
Member

jdm commented Oct 2, 2019

CTCreateFontWithName intermittently returns the wrong font for a given postscript name. These changes rely on obtaining a font descriptor, using it to get the path to the actual font file on disk, getting the bytes that make up the font, and using Core Graphics to create a font handle and obtaining the Core Graphics font from the CGFont reference. This is the same strategy that font-kit uses.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #23290
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Oct 2, 2019

warning Warning warning

  • These commits modify gfx code, but no tests are modified. Please consider adding a test!
@jdm
Copy link
Member Author

jdm commented Oct 2, 2019

@bors-servo try=wpt-mac

@bors-servo
Copy link
Contributor

bors-servo commented Oct 2, 2019

Trying commit ffa4018 with merge 60184d6...

bors-servo added a commit that referenced this pull request Oct 2, 2019
Fix intermittent macOS reftest failures

CTCreateFontWithName intermittently returns the wrong font for a given postscript name. These changes rely on obtaining a font descriptor, using it to get the path to the actual font file on disk, getting the bytes that make up the font, and using Core Graphics to create a font handle and obtaining the Core Graphics font from the CGFont reference. This is the same strategy that font-kit uses.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #23290
- [x] There are tests for these changes

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

bors-servo commented Oct 2, 2019

💔 Test failed - status-taskcluster

@jdm jdm force-pushed the jdm:mac-font-load branch from ffa4018 to a774e51 Oct 2, 2019
@highfive highfive removed the S-tests-failed label Oct 2, 2019
@jdm
Copy link
Member Author

jdm commented Oct 2, 2019

r? @pcwalton since this code is cribbed from font-kit.

@highfive highfive assigned pcwalton and unassigned ferjm Oct 2, 2019
@@ -86,7 +94,33 @@ impl FontTemplateData {
Err(_) => None,
}
},
None => core_text::font::new_from_name(&*self.identifier, clamped_pt_size).ok(),
None => {

This comment has been minimized.

Copy link
@pcwalton

pcwalton Oct 2, 2019

Contributor

Add a comment linking to the issue. r=me with that.

@jdm jdm force-pushed the jdm:mac-font-load branch from a774e51 to 1529436 Oct 2, 2019
@jdm
Copy link
Member Author

jdm commented Oct 2, 2019

@bors-servo r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Oct 2, 2019

📌 Commit 1529436 has been approved by pcwalton

@@ -1,2 +1,2 @@
[control-chars-089.html]
expected: FAIL
sexpected: FAIL

This comment has been minimized.

Copy link
@atouchet

atouchet Oct 3, 2019

Contributor

Typo?

This comment has been minimized.

Copy link
@jdm

jdm Oct 3, 2019

Author Member

Whoops.

…t incorrect behaviour.
@jdm jdm force-pushed the jdm:mac-font-load branch from 1529436 to ddbd7ae Oct 3, 2019
@jdm
Copy link
Member Author

jdm commented Oct 3, 2019

@bors-servo r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Oct 3, 2019

📌 Commit ddbd7ae has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Oct 3, 2019

Testing commit ddbd7ae with merge 7843811...

bors-servo added a commit that referenced this pull request Oct 3, 2019
Fix intermittent macOS reftest failures

CTCreateFontWithName intermittently returns the wrong font for a given postscript name. These changes rely on obtaining a font descriptor, using it to get the path to the actual font file on disk, getting the bytes that make up the font, and using Core Graphics to create a font handle and obtaining the Core Graphics font from the CGFont reference. This is the same strategy that font-kit uses.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #23290
- [x] There are tests for these changes

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

bors-servo commented Oct 3, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: pcwalton
Pushing 7843811 to master...

@bors-servo bors-servo merged commit ddbd7ae into servo:master Oct 3, 2019
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
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.

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