Bug 1529260 - Match shader and CPU glyph rasterization logic for glyphs in local space. r=lsalzman
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 13 Feb 2020 20:40:11 +0000
changeset 513814 9bd4120bc9779fe1c42fb45246044a23f2e95d25
parent 513813 931d03e3922d0a4db8ebaf29821adafebed33ab2
child 513815 36ba18ac3a682d10537e3bb481c6b06532c6ec7e
push id37122
push usercsabou@mozilla.com
push dateFri, 14 Feb 2020 10:05:11 +0000
treeherdermozilla-central@ccbbd26e4bec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1529260
milestone75.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1529260 - Match shader and CPU glyph rasterization logic for glyphs in local space. r=lsalzman This patch makes the CPU side incorporate the raster scale when calculating the subpixel position of a glyph. It also makes the shader side not include the glyph scale factor when recalculating the glyph position (since it was not known/used when determining the subpixel position in the first place). This makes the subpixel position stable when we transition between Screen and Local(raster_scale) spaces. Differential Revision: https://phabricator.services.mozilla.com/D62812
gfx/wr/webrender/res/ps_text_run.glsl
gfx/wr/webrender/src/prim_store/text_run.rs
--- a/gfx/wr/webrender/res/ps_text_run.glsl
+++ b/gfx/wr/webrender/res/ps_text_run.glsl
@@ -181,35 +181,37 @@ void main(void) {
     // the device space glyph rect to reduce overdraw of clipped pixels in the fragment shader.
     // Otherwise, fall back to clamping the glyph's local rect to the local clip rect.
     if (rect_inside_rect(local_rect, ph.local_clip_rect)) {
         local_pos = glyph_transform_inv * (glyph_rect.p0 + glyph_rect.size * aPosition.xy);
     }
 #else
     float raster_scale = float(ph.user_data.z) / 65535.0;
 
+    // Scale in which the glyph is snapped when rasterized.
+    float glyph_raster_scale = raster_scale * task.device_pixel_scale;
+
     // Scale from glyph space to local space.
-    float glyph_scale_inv = res.scale / (raster_scale * task.device_pixel_scale);
-    float glyph_scale = 1.0 / glyph_scale_inv;
+    float glyph_scale_inv = res.scale / glyph_raster_scale;
 
     // Glyph raster pixels do not include the impact of the transform. Instead it was
     // replaced with an identity transform during glyph rasterization. As such only the
     // impact of the raster scale (if in local space) and the device pixel scale (for both
     // local and screen space) are included.
     //
     // This implies one or more of the following conditions:
     // - The transform is an identity. In that case, setting WR_FEATURE_GLYPH_TRANSFORM
     //   should have the same output result as not. We just distingush which path to use
     //   based on the transform used during glyph rasterization. (Screen space).
     // - The transform contains an animation. We will imply local raster space in such
     //   cases to avoid constantly rerasterizing the glyphs.
     // - The transform has perspective or does not have a 2d inverse (Screen or local space).
     // - The transform's scale will result in result in very large rasterized glyphs and
     //   we clamped the size. This will imply local raster space.
-    vec2 raster_glyph_offset = floor(glyph.offset * glyph_scale + snap_bias);
+    vec2 raster_glyph_offset = floor(glyph.offset * glyph_raster_scale + snap_bias) / res.scale;
 
     // Compute the glyph rect in local space.
     //
     // The transform may be animated, so we don't want to do any snapping here for the
     // text offset to avoid glyphs wiggling. The text offset should have been snapped
     // already for axis aligned transforms excluding any animations during frame building.
     RectWithSize glyph_rect = RectWithSize(glyph_scale_inv * (res.offset + raster_glyph_offset) + text_offset,
                                            glyph_scale_inv * (res.uv_rect.zw - res.uv_rect.xy));
--- a/gfx/wr/webrender/src/prim_store/text_run.rs
+++ b/gfx/wr/webrender/src/prim_store/text_run.rs
@@ -356,20 +356,25 @@ impl TextRunPrimitive {
             subpixel_mode,
             raster_space,
             spatial_tree,
         );
 
         if self.glyph_keys_range.is_empty() || cache_dirty {
             let subpx_dir = self.used_font.get_subpx_dir();
 
+            let transform = match self.raster_space {
+                RasterSpace::Local(scale) => FontTransform::new(scale, 0.0, 0.0, scale),
+                RasterSpace::Screen => self.used_font.transform,
+            };
+
             self.glyph_keys_range = scratch.glyph_keys.extend(
                 glyphs.iter().map(|src| {
                     let src_point = src.point + prim_offset;
-                    let world_offset = self.used_font.transform.transform(&src_point);
+                    let world_offset = transform.transform(&src_point);
                     let device_offset = surface.device_pixel_scale.transform_point(world_offset);
                     GlyphKey::new(src.index, device_offset, subpx_dir)
                 }));
         }
 
         resource_cache.request_glyphs(
             self.used_font.clone(),
             &scratch.glyph_keys[self.glyph_keys_range],