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

Debugging information for mac fonts #23375

Closed
wants to merge 16 commits into from
Closed

Debugging information for mac fonts #23375

wants to merge 16 commits into from
+140 −60

Conversation

@jdm
Copy link
Member

jdm commented May 13, 2019 •
edited by SimonSapin

This change is Reviewable
@jdm jdm unassigned ferjm May 13, 2019
@highfive
Copy link

highfive commented May 13, 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 May 13, 2019

@bors-servo try=wpt-mac
@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2019

⌛ Trying commit 48fddd3 with merge c80b4e2...
bors-servo added a commit that referenced this pull request May 13, 2019
@bors-servo
Auto merge of #23375 - jdm:mac-font-select, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
c80b4e2 
Debugging information for mac fonts

<!-- 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/23375)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 13, 2019

☀️ Test successful - mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, status-taskcluster
State: approved= try=True
@ghost
Copy link

ghost commented May 13, 2019

Submitting the task to Taskcluster failed. Details

InterpreterError at template.tasks.payload.env["NON_MERGE_GIT_SHA"]: object has no property commits
@jdm jdm force-pushed the jdm:mac-font-select branch 2 times, most recently from defd2f5 to 7cd5609 May 14, 2019
@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2019

☔ The latest upstream changes (presumably #23355) made this pull request unmergeable. Please resolve the merge conflicts.
@jdm jdm force-pushed the jdm:mac-font-select branch 2 times, most recently from 48fddd3 to fda980d May 14, 2019
@jdm
Copy link
Member Author

jdm commented May 14, 2019

@bors-servo try=wpt-mac
bors-servo added a commit that referenced this pull request May 14, 2019
@bors-servo
Auto merge of #23375 - jdm:mac-font-select, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
6182281 
Debugging information for mac fonts

<!-- 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/23375)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2019

⌛ Trying commit fda980d with merge 6182281...
@jdm
Copy link
Member Author

jdm commented May 14, 2019

@bors-servo try=wpt-mac
@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2019

⌛ Trying commit 5295d0f with merge 8298cb6...
bors-servo added a commit that referenced this pull request May 14, 2019
@bors-servo
Auto merge of #23375 - jdm:mac-font-select, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
8298cb6 
Debugging information for mac fonts

<!-- 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/23375)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2019

☀️ Test successful - mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, status-taskcluster
State: approved= try=True
@jdm jdm mentioned this pull request May 15, 2019
Open
@jdm
Copy link
Member Author

jdm commented May 15, 2019

@bors-servo try=wpt-mac
@bors-servo
Copy link
Contributor

bors-servo commented May 15, 2019

⌛ Trying commit 66a2855 with merge 6e711b0...
bors-servo added a commit that referenced this pull request May 15, 2019
@bors-servo
Auto merge of #23375 - jdm:mac-font-select, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
6e711b0 
Debugging information for mac fonts

<!-- 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/23375)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 15, 2019

💔 Test failed - status-taskcluster
@jdm jdm force-pushed the jdm:mac-font-select branch from 66a2855 to 927ab29 May 15, 2019
@bors-servo
Copy link
Contributor

bors-servo commented Sep 18, 2019

⌛ Trying commit 4b13337 with merge dc1b2ec...
bors-servo added a commit that referenced this pull request Sep 18, 2019
@bors-servo
Auto merge of #23375 - jdm:mac-font-select, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
dc1b2ec 
Debugging information for mac fonts

<!-- 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/23375)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 18, 2019

☀️ Test successful - status-taskcluster
State: approved= try=True
jdm added 16 commits May 13, 2019
@jdm
Debugging information for mac fonts
09a9e1b
@jdm
fixup! Debugging information for mac fonts
6073c1f
@jdm
Properly encode codepoints for glyph queries.
9353294
@jdm
reduce unnecessary work
45f6ea1
@jdm
more debugging info
70ce2a1
@jdm
more debugging info
d65179b
@jdm
Update core-text.
6f2fb78
@jdm
debug
8577a1a
@jdm
more debug
e95336e
@jdm
more debug
f4f544e
@jdm
more debug
452c934
@jdm
reduce work
c9054b3
@jdm
fix build
6f94eaa
@jdm
fix build harder
dbb9843
@jdm
less debugging
8b32896
@jdm
less debugging pt 2
Loading status checks…
9c80823
@jdm jdm force-pushed the jdm:mac-font-select branch from 4b13337 to 9c80823 Sep 25, 2019
@jdm
Copy link
Member Author

jdm commented Sep 25, 2019

@bors-servo try=wpt-mac
bors-servo added a commit that referenced this pull request Sep 25, 2019
@bors-servo
Auto merge of #23375 - jdm:mac-font-select, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
98a2947 
Debugging information for mac fonts

<!-- 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/23375)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 25, 2019

⌛ Trying commit 9c80823 with merge 98a2947...
@jdm
Copy link
Member Author

jdm commented Sep 25, 2019

Here's where the behaviour goes off the rails:
Good:

"[2019-09-25T17:39:55Z INFO  gfx::font_template] successfully instantiated Atom('TimesNewRomanPS-BoldMT' type=dynamic) with Some(FontTemplateDescriptor { weight: FontWeight(600.0), stretch: FontStretch(NonNegative(Percentage(1.0))), style: Normal })"

Bad:

"[2019-09-25T17:47:02Z INFO  gfx::font_template] successfully instantiated Atom('TimesNewRomanPS-BoldMT' type=dynamic) with Some(FontTemplateDescriptor { weight: FontWeight(400.0), stretch: FontStretch(NonNegative(Percentage(1.0))), style: Normal })"

That originates from this code:

        self.descriptor = Some(FontTemplateDescriptor::new(
            handle.boldness(),
            handle.stretchiness(),
            handle.style(),
        ));
        info!("successfully instantiated {:?} with {:?}", self.identifier, self.descriptor);

where handle comes from FontHandleMethods::new_from_template. Boldness comes from getting the traits of the underlying CTFont, so this is super weird.
@bors-servo
Copy link
Contributor

bors-servo commented Sep 25, 2019

💔 Test failed - status-taskcluster
@jdm
Copy link
Member Author

jdm commented Sep 25, 2019

Apparently this is a problem that pcwalton has run into in the past, and he pointed me at: https://github.com/pcwalton/font-kit/blob/1d4bc46089e9f1d707a54b233a317890e8910b9a/src/sources/core_text.rs#L67-L98
@bors-servo
Copy link
Contributor

bors-servo commented Sep 28, 2019

☔ The latest upstream changes (presumably #24308) made this pull request unmergeable. Please resolve the merge conflicts.
@jdm
Copy link
Member Author

jdm commented Oct 9, 2019

No longer necessary.
@jdm jdm closed this Oct 9, 2019
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
S-awaiting-review S-needs-rebase S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@jdm @highfive @bors-servo @ferjm
You can’t perform that action at this time.