Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRefined text glyph snapping #1465
Conversation
|
Fixed to pass test-wpt now. The fix was to keep the old snapping behavior for the non-subpixel text. |
|
This looks good, but ideally I'd like to hold off until we get the WR update landed with the font size / hinting changes - just so I can verify all the changes work nicely together. |
| ], | ||
| // The sub-pixel offset has already been taken into account | ||
| // by the glyph rasterizer, thus the truncating here. | ||
| FontRenderMode::Subpixel => [ |
This comment has been minimized.
This comment has been minimized.
glennw
Jul 12, 2017
Member
Instead of a conditional here, can we do this in the prepare_prim_for_render when the glyph_instances array is created? That would mean only doing it once, not each time it needs to get added to the cache.
|
|
|
@kvark Looks good. We have zero coverage in Servo tests for alpha / subpixel text - we currently only run the tests with text AA disabled. We should add some test coverage to wrench for the various render modes and snap configurations. @bors-servo r+ |
|
|
Refined text glyph snapping This PR truncates the sub-pixel offset of the glyphs coming to GPU in order to fix the snapping. With `FontRenderMode::Subpixel`, that offset is already taken into account when the glyph is rasterized. With the other rendering modes, I'm not completely sure if it's fine to truncate the fractional part, but I also don't see how the current snapping code is better anyway. r? @glennw <!-- 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/1465) <!-- Reviewable:end -->
|
|
kvark commentedJul 10, 2017
•
edited by larsbergstrom
This PR truncates the sub-pixel offset of the glyphs coming to GPU in order to fix the snapping. With
FontRenderMode::Subpixel, that offset is already taken into account when the glyph is rasterized. With the other rendering modes, I'm not completely sure if it's fine to truncate the fractional part, but I also don't see how the current snapping code is better anyway.r? @glennw
This change is