Bug 1529260 - Match shader and CPU glyph rasterization logic for glyphs in local space. r=lsalzman, a=RyanVM
authorAndrew Osmond <aosmond@mozilla.com>
Thu, 13 Feb 2020 20:40:11 +0000
changeset 575732 33e592c555fec0f7e3db46f207968f052a6e867d
parent 575731 04a0b8a0c7f2da89260cdbefd08aa266a9552ff7
child 575733 9cf0faa2144b87c885c76641c1ad881f94bb05a8
push id12707
push userryanvm@gmail.com
push dateFri, 14 Feb 2020 16:56:21 +0000
treeherdermozilla-beta@33e592c555fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman, RyanVM
bugs1529260
milestone74.0
Bug 1529260 - Match shader and CPU glyph rasterization logic for glyphs in local space. r=lsalzman, a=RyanVM 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],