Bug 1585713 - disable subpixel positioning for oversized WR fonts. r=jnicol

Differential Revision: https://phabricator.services.mozilla.com/D51746

--HG--
extra : moz-landing-system : lando

gfx/wr/webrender/src/glyph_rasterizer/mod.rs

@@ -534,23 +534,6 @@ impl FontInstance {
             0
         }
     }
-
-    pub fn oversized_scale_factor(&self, x_scale: f64, y_scale: f64) -> f64 {
-        // If the scaled size is over the limit, then it will need to
-        // be scaled up from the size limit to the scaled size.
-        // However, this should only occur when the font isn't using any
-        // features that would tie it to device space, like transforms or
-        // subpixel AA.
-        let max_size = self.size.to_f64_px() * x_scale.max(y_scale);
-        if max_size > FONT_SIZE_LIMIT &&
-           self.transform.is_identity() &&
-           self.render_mode != FontRenderMode::Subpixel
-        {
-            max_size / FONT_SIZE_LIMIT
-        } else {
-            1.0
-        }
-    }
 }
 
 #[repr(u32)]

gfx/wr/webrender/src/platform/macos/font.rs

@@ -491,8 +491,7 @@ impl FontContext {
 
     pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult {
         let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
-        let scale = font.oversized_scale_factor(x_scale, y_scale);
-        let size = font.size.scale_by((y_scale / scale) as f32);
+        let size = font.size.scale_by(y_scale as f32);
         let ct_font = self.get_ct_font(font.font_key, size, &font.variations).ok_or(GlyphRasterError::LoadFailed)?;
         let glyph_type = if is_bitmap_font(&ct_font) {
             GlyphType::Bitmap
@@ -538,13 +537,13 @@ impl FontContext {
             (x_scale, y_scale / x_scale)
         };
 
-        let extra_strikes = font.get_extra_strikes(strike_scale / scale);
+        let extra_strikes = font.get_extra_strikes(strike_scale);
         let metrics = get_glyph_metrics(
             &ct_font,
             transform.as_ref(),
             glyph,
-            x_offset / scale,
-            y_offset / scale,
+            x_offset,
+            y_offset,
             extra_strikes as f64 * pixel_step,
         );
         if metrics.rasterized_width == 0 || metrics.rasterized_height == 0 {
@@ -634,8 +633,8 @@ impl FontContext {
 
             // CG Origin is bottom left, WR is top left. Need -y offset
             let mut draw_origin = CGPoint {
-                x: -metrics.rasterized_left as f64 + x_offset / scale,
-                y: metrics.rasterized_descent as f64 - y_offset / scale,
+                x: -metrics.rasterized_left as f64 + x_offset,
+                y: metrics.rasterized_descent as f64 - y_offset,
             };
 
             if let Some(transform) = transform {
@@ -719,8 +718,8 @@ impl FontContext {
             width: metrics.rasterized_width,
             height: metrics.rasterized_height,
             scale: match glyph_type {
-                GlyphType::Bitmap => (scale / y_scale) as f32,
-                GlyphType::Vector => scale as f32,
+                GlyphType::Bitmap => y_scale.recip() as f32,
+                GlyphType::Vector => 1.0,
             },
             format: match glyph_type {
                 GlyphType::Bitmap => GlyphFormat::ColorBitmap,

gfx/wr/webrender/src/platform/unix/font.rs

@@ -420,14 +420,13 @@ impl FontContext {
         load_flags |= FT_LOAD_IGNORE_GLOBAL_ADVANCE_WIDTH;
 
         let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
-        let scale = font.oversized_scale_factor(x_scale, y_scale);
         let req_size = font.size.to_f64_px();
         let face_flags = unsafe { (*face).face_flags };
         let mut result = if (face_flags & (FT_FACE_FLAG_FIXED_SIZES as FT_Long)) != 0 &&
                             (face_flags & (FT_FACE_FLAG_SCALABLE as FT_Long)) == 0 &&
                             (load_flags & FT_LOAD_NO_BITMAP) == 0 {
             unsafe { FT_Set_Transform(face, ptr::null_mut(), ptr::null_mut()) };
-            self.choose_bitmap_size(face, req_size * y_scale / scale)
+            self.choose_bitmap_size(face, req_size * y_scale)
         } else {
             let mut shape = font.transform.invert_scale(x_scale, y_scale);
             if font.flags.contains(FontInstanceFlags::FLIP_X) {
@@ -452,8 +451,8 @@ impl FontContext {
                 FT_Set_Transform(face, &mut ft_shape, ptr::null_mut());
                 FT_Set_Char_Size(
                     face,
-                    (req_size * x_scale / scale * 64.0 + 0.5) as FT_F26Dot6,
-                    (req_size * y_scale / scale * 64.0 + 0.5) as FT_F26Dot6,
+                    (req_size * x_scale * 64.0 + 0.5) as FT_F26Dot6,
+                    (req_size * y_scale * 64.0 + 0.5) as FT_F26Dot6,
                     0,
                     0,
                 )
@@ -505,7 +504,7 @@ impl FontContext {
                 let y_size = unsafe { (*(*(*slot).face).size).metrics.y_ppem };
                 Some((slot, req_size as f32 / y_size as f32))
             }
-            FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => Some((slot, scale as f32)),
+            FT_Glyph_Format::FT_GLYPH_FORMAT_OUTLINE => Some((slot, 1.0)),
             _ => {
                 error!("Unsupported format");
                 debug!("format={:?}", format);

gfx/wr/webrender/src/platform/windows/font.rs

@@ -505,9 +505,8 @@ impl FontContext {
     }
 
     pub fn rasterize_glyph(&mut self, font: &FontInstance, key: &GlyphKey) -> GlyphRasterResult {
-        let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
-        let scale = font.oversized_scale_factor(x_scale, y_scale);
-        let size = (font.size.to_f64_px() * y_scale / scale) as f32;
+        let (_, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
+        let size = (font.size.to_f64_px() * y_scale) as f32;
         let bitmaps = is_bitmap_font(font);
         let (mut shape, (x_offset, y_offset)) = if bitmaps {
             (FontTransform::identity(), (0.0, 0.0))
@@ -532,8 +531,8 @@ impl FontContext {
                 m12: shape.skew_y,
                 m21: shape.skew_x,
                 m22: shape.scale_y,
-                dx: (x_offset / scale) as f32,
-                dy: (y_offset / scale) as f32,
+                dx: x_offset as f32,
+                dy: y_offset as f32,
             })
         } else {
             None
@@ -577,7 +576,7 @@ impl FontContext {
             top: -bounds.top as f32,
             width,
             height,
-            scale: (if bitmaps { scale / y_scale } else { scale }) as f32,
+            scale: (if bitmaps { y_scale.recip() } else { 1.0 }) as f32,
             format,
             bytes: bgra_pixels,
         })

gfx/wr/webrender/src/prim_store/text_run.rs

@@ -242,24 +242,26 @@ impl TextRunPrimitive {
         // Get the current font size in device pixels
         let mut device_font_size = specified_font.size.scale_by(surface.device_pixel_scale.0 * raster_scale);
 
-        // Determine if rasterizing glyphs in local or screen space.
-        let transform_glyphs = if raster_space != RasterSpace::Screen {
-            // Ensure the font is supposed to be rasterized in screen-space.
-            false
-        } else if transform.has_perspective_component() || !transform.has_2d_inverse() {
-            // Only support transforms that can be coerced to simple 2D transforms.
-            false
+        // Check there is a valid transform that doesn't exceed the font size limit.
+        // Ensure the font is supposed to be rasterized in screen-space.
+        // Only support transforms that can be coerced to simple 2D transforms.
+        let (transform_glyphs, oversized) = if raster_space != RasterSpace::Screen ||
+            transform.has_perspective_component() || !transform.has_2d_inverse() {
+            (false, device_font_size.to_f64_px() > FONT_SIZE_LIMIT)
         } else if transform.exceeds_2d_scale(FONT_SIZE_LIMIT / device_font_size.to_f64_px()) {
+            (false, true)
+        } else {
+            (true, false)
+        };
+
+        if oversized {
             // Font sizes larger than the limit need to be scaled, thus can't use subpixels.
             // In this case we adjust the font size and raster space to ensure
             // we rasterize at the limit, to minimize the amount of scaling.
             let max_scale = (FONT_SIZE_LIMIT / device_font_size.to_f64_px()) as f32;
             raster_space = RasterSpace::Local(max_scale * raster_scale);
             device_font_size = device_font_size.scale_by(max_scale);
-            false
-        } else {
-            true
-        };
+        }
 
         // Get the font transform matrix (skew / scale) from the complete transform.
         let font_transform = if transform_glyphs {
@@ -316,6 +318,15 @@ impl TextRunPrimitive {
             !transform_glyphs
         {
             self.used_font.disable_subpixel_aa();
+
+            // Disable subpixel positioning for oversized glyphs to avoid
+            // thrashing the glyph cache with many subpixel variations of
+            // big glyph textures. A possible subpixel positioning error
+            // is small relative to the maximum font size and thus should
+            // not be very noticeable.
+            if oversized {
+                self.used_font.disable_subpixel_position();
+            }
         }
 
         cache_dirty