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

allow custom gamma and contrast for DWrite fonts #2896

Merged
merged 1 commit into from Jul 16, 2018

Conversation

@lsalzman
Copy link
Contributor

lsalzman commented Jul 16, 2018

This is to resolve Gecko bug https://bugzilla.mozilla.org/show_bug.cgi?id=1474968

This passes in gamma and contrast so we can parameterize preblending more closely to how Skia is doing so in Gecko.


This change is Reviewable

}

#[cfg(target_os = "windows")]
impl Default for FontInstancePlatformOptions {
fn default() -> FontInstancePlatformOptions {
FontInstancePlatformOptions {
unused: 0,
gamma: 220,

This comment has been minimized.

@gw3583

gw3583 Jul 16, 2018

Collaborator

The previous default constant used was 230 - is 220 correct? Also, could we add a comment here describing why this is the default?

@gw3583
Copy link
Collaborator

gw3583 commented Jul 16, 2018

Just one minor comment - otherwise looks good!

@lsalzman lsalzman force-pushed the lsalzman:dwrite-gamma branch from 3264233 to dcef8e5 Jul 16, 2018
Copy link
Contributor Author

lsalzman left a comment

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @lsalzman)


webrender_api/src/font.rs, line 279 at r1 (raw file):

Previously, gw3583 (Glenn Watson) wrote…

The previous default constant used was 230 - is 220 correct? Also, could we add a comment here describing why this is the default?

It was 1.8 (=180), so for now I just put it back at 180. But the Gecko default in many places when no value is specified can also be 2.2 (=220). It's kind of arguable which one to use, since neither is really right as a fixed value. I just put it back to 180 for now since that is what it was before.

@lsalzman lsalzman force-pushed the lsalzman:dwrite-gamma branch from dcef8e5 to 2e770b2 Jul 16, 2018
@lsalzman lsalzman force-pushed the lsalzman:dwrite-gamma branch from 2e770b2 to 05c2ce8 Jul 16, 2018
@gw3583
Copy link
Collaborator

gw3583 commented Jul 16, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 16, 2018

📌 Commit 05c2ce8 has been approved by gw3583

@bors-servo
Copy link
Contributor

bors-servo commented Jul 16, 2018

Testing commit 05c2ce8 with merge 915ec3a...

bors-servo added a commit that referenced this pull request Jul 16, 2018
allow custom gamma and contrast for DWrite fonts

This is to resolve Gecko bug https://bugzilla.mozilla.org/show_bug.cgi?id=1474968

This passes in gamma and contrast so we can parameterize preblending more closely to how Skia is doing so in Gecko.

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

bors-servo commented Jul 16, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing 915ec3a to master...

@bors-servo bors-servo merged commit 05c2ce8 into servo:master Jul 16, 2018
3 of 4 checks passed
3 of 4 checks passed
code-review/reviewable 2 files, 1 discussion left (gw3583)
Details
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.

None yet

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