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

Add basic subpixel AA text support on Linux. #527

Merged
merged 1 commit into from
Nov 7, 2016

Conversation

glennw
Copy link
Member

@glennw glennw commented Nov 4, 2016

This is some preliminary support for subpixel AA. Specifically:

  • Only on Linux for now.
  • No gamma correction.
  • Disabled by default (use -Z subpixel-aa to test).
  • Doesn't deal with rotations, subpixel positioning etc.

This change is Reviewable

@jrmuizel
Copy link
Collaborator

jrmuizel commented Nov 4, 2016

Re: gamma correction. I've elaborated some of my thoughts in #230.

@glennw glennw force-pushed the subpixel branch 3 times, most recently from 293294b to dd2f5fb Compare November 6, 2016 21:31
@glennw glennw mentioned this pull request Nov 6, 2016
@glennw
Copy link
Member Author

glennw commented Nov 6, 2016

r? @pcwalton

BlendMode::Alpha
} else {
BlendMode::None
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could be

_ if needs_blending => BlendMode::Alpha,
_ => BlendMode::None

Up to you

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left as is for now - will tidy up when I expand this function today for a different PR.

@@ -215,6 +217,9 @@ impl FontContext {
}
}
FT_PIXEL_MODE_LCD => {
// Extra subpixel on each side of the glyph.
glyph_width += 2;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugly. :( Oh well, probably can't be helped.

@@ -92,6 +92,7 @@ const GPU_TAG_PRIM_BORDER: GpuProfileTag = GpuProfileTag { label: "Border", colo
pub enum BlendMode {
None,
Alpha,
Subpixel(ColorF),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment saying that the ColorF is the color of the text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

This is some preliminary support for subpixel AA. Specifically:
 * Only on Linux for now.
 * No gamma correction.
 * Disabled by default (use -Z subpixel-aa to test).
 * Doesn't deal with rotations, subpixel positioning etc.
@glennw
Copy link
Member Author

glennw commented Nov 7, 2016

@bors-servo r=pcwalton (irc)

@bors-servo
Copy link
Contributor

📌 Commit bab4731 has been approved by pcwalton

@bors-servo
Copy link
Contributor

⌛ Testing commit bab4731 with merge 4572743...

bors-servo pushed a commit that referenced this pull request Nov 7, 2016
Add basic subpixel AA text support on Linux.

This is some preliminary support for subpixel AA. Specifically:
 * Only on Linux for now.
 * No gamma correction.
 * Disabled by default (use -Z subpixel-aa to test).
 * Doesn't deal with rotations, subpixel positioning etc.

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

☀️ Test successful - status-travis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants