Enable subpixel text for dwrite fonts #798
Conversation
|
Why is the subpixel offset not part of GlyphOptions? That's already part of the hash key, no? |
|
It's part of the GlyphKey but not the GlyphOptions. GlyphOptions are force gdi rendering and embedded bitmaps. |
|
But that stuff changes the rendering, so it should affect how the glyph is
cached no? Or are those constant per cache?
|
|
Err sorry. It's constant per glyph run. When it's looked up in the RenderedGlyphKey, it's being cached with the glyph options. But the GlyphKey, also has the subpixel offsets, so the cache won't hit if it's not being quantized to the same subpixel offset. |
|
☔ The latest upstream changes (presumably #790) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Needs rebasing. |
changm
force-pushed
the
subpixel-dwrite
branch
from
9425eb9 to
2a65376
Compare
|
Rebased. |
|
@bors-servo retry I think this is actually passing. I see this same travis-ci error in a couple of PRs and this PR didn't touch wrench at all. It doesn't even seem to run on windows? |
|
Yup, that CI failure is an intermittent which should be fixed by #807 |
|
@vvuk Could you take a look over this? |
changm
force-pushed
the
subpixel-dwrite
branch
from
2a65376 to
349da8c
Compare
glennw
left a comment
glennw
left a comment
There was a problem hiding this comment.
Let's get this merged, since it's blocking other PRs.
| // XX use the xform to handle subpixel positioning (what skia does), I believe that keeps | ||
| //let xform = dwrote::DWRITE_MATRIX { m11: 1.0, m12: 0.0, m21: 0.0, m22: 1.0, dx: 0.0, dy: 0.0 }; | ||
| dwrote::GlyphRunAnalysis::create(&glyph_run, 1.0, None, | ||
| let x_offset: f32 = Into::<f64>::into(x_suboffset) as f32; |
There was a problem hiding this comment.
This seems a bit over complicated - perhaps convert it to a local f64 and then cast to f32 when creating the matrix or something like that. Can be done as a follow up anyway, it's not major.
|
@bors-servo r+ |
|
📌 Commit 349da8c has been approved by |
Enable subpixel text for dwrite fonts Depends on #795 (comment) The only real changes here are in webrender/src/platform/windows/font.rs <!-- 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/798) <!-- Reviewable:end -->
|
@bors-servo r-
|
|
@bors-servo r+ force clean |
|
@bors-servo r+ |
|
📌 Commit 349da8c has been approved by |
Enable subpixel text for dwrite fonts Depends on #795 (comment) The only real changes here are in webrender/src/platform/windows/font.rs <!-- 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/798) <!-- Reviewable:end -->
|
@bors-servo ping |
|
😪 I'm awake I'm awake |
|
@bors-servo r+ |
|
💡 This pull request was already approved, no need to approve it again.
|
|
📌 Commit 349da8c has been approved by |
|
😪 I'm awake I'm awake |
Enable subpixel text for dwrite fonts Depends on #795 (comment) The only real changes here are in webrender/src/platform/windows/font.rs <!-- 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/798) <!-- Reviewable:end -->
|
😪 I'm awake I'm awake |
|
I have no idea what's going on with bors / travis etc, but maybe this PR is what's messing it up. I finally convinced travis to try and build this PR, but it fails saying that the git SHA is not a valid reference. Want to close this, and re-open as a new PR and I'll merge it manually first thing in the morning? |
|
Merging manually due to bors problems, travis passed. |
5 participants
Depends on #795 (comment)
The only real changes here are in webrender/src/platform/windows/font.rs
This change is