Bug 1512010 - don't snap text positions in local raster space. r=gw
authorLee Salzman <lsalzman@mozilla.com>
Sun, 13 Jan 2019 22:26:26 -0500
changeset 453706 9cf58f64e2c204f48a38f69c1c5ec272f65e0bb1
parent 453705 25b80743d6504a24c2e9a981e38cd7ed0fd3b7d5
child 453707 58b0b6fba4c388a12b98954772074ffbfc81c784
push id35371
push userncsoregi@mozilla.com
push dateMon, 14 Jan 2019 17:23:11 +0000
treeherdermozilla-central@b8f96f4d584a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgw
bugs1512010
milestone66.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 1512010 - don't snap text positions in local raster space. r=gw
gfx/wr/webrender/res/ps_text_run.glsl
gfx/wr/webrender/src/batch.rs
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
@@ -58,33 +58,29 @@ struct TextRun {
 
 TextRun fetch_text_run(int address) {
     vec4 data[3] = fetch_from_gpu_cache_3(address);
     return TextRun(data[0], data[1], data[2].xy);
 }
 
 VertexInfo write_text_vertex(RectWithSize local_clip_rect,
                              float z,
+                             bool should_snap,
                              Transform transform,
                              PictureTask task,
                              vec2 text_offset,
                              vec2 glyph_offset,
                              RectWithSize glyph_rect,
                              vec2 snap_bias) {
     // The offset to snap the glyph rect to a device pixel
     vec2 snap_offset = vec2(0.0);
     mat2 local_transform;
 
-#ifdef WR_FEATURE_GLYPH_TRANSFORM
-    bool remove_subpx_offset = true;
-#else
-    bool remove_subpx_offset = transform.is_axis_aligned;
-#endif
     // Compute the snapping offset only if the scroll node transform is axis-aligned.
-    if (remove_subpx_offset) {
+    if (should_snap) {
         // Transform from local space to device space.
         float device_scale = task.common_data.device_pixel_scale / transform.m[3].w;
         mat2 device_transform = mat2(transform.m) * device_scale;
 
         // Ensure the transformed text offset does not contain a subpixel translation
         // such that glyph snapping is stable for equivalent glyph subpixel positions.
         vec2 device_text_pos = device_transform * text_offset + transform.m[3].xy * device_scale;
         snap_offset = floor(device_text_pos + 0.5) - device_text_pos;
@@ -173,23 +169,28 @@ void main(void) {
 #ifdef WR_FEATURE_GLYPH_TRANSFORM
     // Transform from local space to glyph space.
     mat2 glyph_transform = mat2(transform.m) * task.common_data.device_pixel_scale;
 
     // Compute the glyph rect in glyph space.
     RectWithSize glyph_rect = RectWithSize(res.offset + glyph_transform * (text.offset + glyph.offset),
                                            res.uv_rect.zw - res.uv_rect.xy);
 
+    // Since the glyph is pre-transformed, snapping is both forced and does not depend on the transform.
+    bool should_snap = true;
 #else
     // Scale from glyph space to local space.
     float scale = res.scale / task.common_data.device_pixel_scale;
 
     // Compute the glyph rect in local space.
     RectWithSize glyph_rect = RectWithSize(scale * res.offset + text.offset + glyph.offset,
                                            scale * (res.uv_rect.zw - res.uv_rect.xy));
+
+    // Check if the primitive is actually safe to snap.
+    bool should_snap = ph.user_data.x != 0;
 #endif
 
     vec2 snap_bias;
     // In subpixel mode, the subpixel offset has already been
     // accounted for while rasterizing the glyph. However, we
     // must still round with a subpixel bias rather than rounding
     // to the nearest whole pixel, depending on subpixel direciton.
     switch (subpx_dir) {
@@ -209,16 +210,17 @@ void main(void) {
             break;
         case SUBPX_DIR_MIXED:
             snap_bias = vec2(0.125);
             break;
     }
 
     VertexInfo vi = write_text_vertex(ph.local_clip_rect,
                                       ph.z,
+                                      should_snap,
                                       transform,
                                       task,
                                       text.offset,
                                       glyph.offset,
                                       glyph_rect,
                                       snap_bias);
     glyph_rect.p0 += vi.snap_offset;
 
--- a/gfx/wr/webrender/src/batch.rs
+++ b/gfx/wr/webrender/src/batch.rs
@@ -824,17 +824,17 @@ impl AlphaBatchBuilder {
                             GlyphFormat::ColorBitmap => {
                                 (
                                     BlendMode::PremultipliedAlpha,
                                     ShaderColorMode::ColorBitmap,
                                 )
                             }
                         };
 
-                        let prim_header_index = prim_headers.push(&prim_header, z_id, [0; 3]);
+                        let prim_header_index = prim_headers.push(&prim_header, z_id, [run.should_snap as i32, 0, 0]);
                         let key = BatchKey::new(kind, blend_mode, textures);
                         let base_instance = GlyphInstance::new(
                             prim_header_index,
                         );
                         let batch = alpha_batch_list.set_params_and_get_batch(
                             key,
                             bounding_rect,
                             z_id,
--- a/gfx/wr/webrender/src/prim_store/text_run.rs
+++ b/gfx/wr/webrender/src/prim_store/text_run.rs
@@ -60,16 +60,17 @@ impl AsInstanceKind<TextRunDataHandle> f
         &self,
         data_handle: TextRunDataHandle,
         prim_store: &mut PrimitiveStore,
     ) -> PrimitiveInstanceKind {
         let run_index = prim_store.text_runs.push(TextRunPrimitive {
             used_font: self.font.clone(),
             glyph_keys_range: storage::Range::empty(),
             shadow: self.shadow,
+            should_snap: true,
         });
 
         PrimitiveInstanceKind::TextRun{ data_handle, run_index }
     }
 }
 
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
@@ -222,16 +223,17 @@ impl IsVisible for TextRun {
     }
 }
 
 #[derive(Debug)]
 pub struct TextRunPrimitive {
     pub used_font: FontInstance,
     pub glyph_keys_range: storage::Range<GlyphKey>,
     pub shadow: bool,
+    pub should_snap: bool,
 }
 
 impl TextRunPrimitive {
     pub fn update_font_instance(
         &mut self,
         specified_font: &FontInstance,
         device_pixel_scale: DevicePixelScale,
         transform: &LayoutToWorldTransform,
@@ -257,16 +259,21 @@ impl TextRunPrimitive {
         // Get the font transform matrix (skew / scale) from the complete transform.
         let font_transform = if transform_glyphs {
             // Quantize the transform to minimize thrashing of the glyph cache.
             FontTransform::from(transform).quantize()
         } else {
             FontTransform::identity()
         };
 
+        // We can snap only if the transform is axis-aligned and in screen-space.
+        self.should_snap =
+            transform.preserves_2d_axis_alignment() &&
+            raster_space == RasterSpace::Screen;
+
         // If the transform or device size is different, then the caller of
         // this method needs to know to rebuild the glyphs.
         let cache_dirty =
             self.used_font.transform != font_transform ||
             self.used_font.size != device_font_size;
 
         // Construct used font instance from the specified font instance
         self.used_font = FontInstance {