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

Update WR (font instance API). #18322

Merged
merged 1 commit into from Sep 1, 2017
Merged

Conversation

@glennw
Copy link
Member

glennw commented Aug 31, 2017

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.


This change is Reviewable

@highfive
Copy link

highfive commented Aug 31, 2017

Heads up! This PR modifies the following files:

  • @emilio: components/layout/webrender_helpers.rs
@highfive
Copy link

highfive commented Aug 31, 2017

warning Warning warning

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

glennw commented Aug 31, 2017

@highfive highfive assigned pcwalton and unassigned metajack Aug 31, 2017
@glennw
Copy link
Member Author

glennw commented Aug 31, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

Trying commit aa748dc with merge 4c35024...

bors-servo added a commit that referenced this pull request Aug 31, 2017
Update WR (font instance API).

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.
@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

💔 Test failed - mac-rel-wpt1

@glennw
Copy link
Member Author

glennw commented Aug 31, 2017

@bors-servo retry

  • infra
@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

Trying commit aa748dc with merge 1607517...

bors-servo added a commit that referenced this pull request Aug 31, 2017
Update WR (font instance API).

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.

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

bors-servo commented Aug 31, 2017

💔 Test failed - windows-msvc-dev

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.
@glennw glennw force-pushed the glennw:update-wr-fonts-2 branch from aa748dc to b015e93 Aug 31, 2017
@glennw
Copy link
Member Author

glennw commented Aug 31, 2017

I also removed the font cache unit test. It now requires a WR API instance, so it's not really amenable to unit testing in its current state. I think the existing test was of very dubious value, anyway.

@glennw
Copy link
Member Author

glennw commented Aug 31, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

Trying commit b015e93 with merge 0481e09...

bors-servo added a commit that referenced this pull request Aug 31, 2017
Update WR (font instance API).

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.

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

bors-servo commented Aug 31, 2017

💔 Test failed - mac-rel-wpt1

@emilio
emilio approved these changes Aug 31, 2017
@emilio
Copy link
Member

emilio commented Aug 31, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

📌 Commit b015e93 has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Aug 31, 2017

Testing commit b015e93 with merge 6db5e17...

bors-servo added a commit that referenced this pull request Aug 31, 2017
Update WR (font instance API).

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.

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

bors-servo commented Aug 31, 2017

💔 Test failed - mac-rel-wpt3

@jdm
Copy link
Member

jdm commented Aug 31, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Sep 1, 2017

Testing commit b015e93 with merge f7d238d...

bors-servo added a commit that referenced this pull request Sep 1, 2017
Update WR (font instance API).

WR now has a concept of font templates and font instances. This
makes the WR font interfaces closer to Cairo and Gecko, and also
makes some future performance optimizations possible.

A font template is the font family, and data backing the font.
A font instance is a reference to a font template and per-instance
options, such as font size, anti-aliasing settings etc.

To update Servo in a minimally invasive way, I added a new font
cache call, that creates a font instance. This means that when
a font is created, and doesn't exist in the cache there are now
two calls to the font cache thread. We could refactor the font
cache to make this work in one call, which we should do in the
future. However, refactoring the font cache is a large chunk of
work by itself. The extra call is only when a font doesn't already
exist in the font context cache, so it should have minimal
performance impact.

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

bors-servo commented Sep 1, 2017

@bors-servo bors-servo merged commit b015e93 into servo:master Sep 1, 2017
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
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

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