Bug 1641671 - represent font size as f32 to avoid quantization issues. r=aosmond

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

gfx/webrender_bindings/src/bindings.rs

@@ -2130,7 +2130,7 @@ pub extern "C" fn wr_resource_updates_add_font_instance(
     txn.add_font_instance(
         key,
         font_key,
-        Au::from_f32_px(glyph_size),
+        glyph_size,
         unsafe { options.as_ref().cloned() },
         unsafe { platform_options.as_ref().cloned() },
         variations.convert_into_vec::<FontVariation>(),

gfx/webrender_bindings/src/moz2d_renderer.rs

@@ -851,7 +851,7 @@ impl Moz2dBlobImageHandler {
                         AddBlobFont(
                             font.font_instance_key,
                             instance.font_key,
-                            instance.size.to_f32_px(),
+                            instance.size,
                             instance.options.as_ref(),
                             instance.platform_options.as_ref(),
                             instance.variations.as_ptr(),

gfx/wr/examples/multiwindow.rs

@@ -117,7 +117,7 @@ impl Window {
         txn.add_raw_font(font_key, font_bytes, 0);
 
         let font_instance_key = api.generate_font_instance_key();
-        txn.add_font_instance(font_instance_key, font_key, Au::from_px(32), None, None, Vec::new());
+        txn.add_font_instance(font_instance_key, font_key, 32.0, None, None, Vec::new());
 
         api.send_transaction(document_id, txn);
 

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

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use api::{FontInstanceFlags, BaseFontInstance};
+use api::{FontInstanceFlags, FontSize, BaseFontInstance};
 use api::{FontKey, FontRenderMode, FontTemplate};
 use api::{ColorU, GlyphIndex, GlyphDimensions, SyntheticItalics};
 use api::units::*;
@@ -428,12 +428,9 @@ pub struct FontInstance {
     // useful when one anticipates the glyph will need to be scaled
     // when rendered.
     pub texture_padding: bool,
-    // The font size is in *device* pixels, not logical pixels.
-    // It is stored as an Au since we need sub-pixel sizes, but
-    // can't store as a f32 due to use of this type as a hash key.
-    // TODO(gw): Perhaps consider having LogicalAu and DeviceAu
-    //           or something similar to that.
-    pub size: Au,
+    // The font size is in *device/raster* pixels, not logical pixels.
+    // It is stored as an f32 since we need sub-pixel sizes.
+    pub size: FontSize,
 }
 
 impl Hash for FontInstance {
@@ -561,6 +558,12 @@ impl FontInstance {
     pub fn synthesize_italics(&self, transform: FontTransform, size: f64) -> (FontTransform, (f64, f64)) {
         transform.synthesize_italics(self.synthetic_italics, size, self.flags.contains(FontInstanceFlags::VERTICAL))
     }
+
+    #[allow(dead_code)]
+    pub fn get_transformed_size(&self) -> f64 {
+        let (_, y_scale) = self.transform.compute_scale().unwrap_or((1.0, 1.0));
+        self.size.to_f64_px() * y_scale
+    }
 }
 
 #[repr(u32)]
@@ -1068,9 +1071,9 @@ mod test_glyph_rasterizer {
         use crate::render_task_cache::RenderTaskCache;
         use crate::render_task_graph::{RenderTaskGraph, RenderTaskGraphCounters};
         use crate::profiler::TextureCacheProfileCounters;
-        use api::{FontKey, FontInstanceKey, FontTemplate, FontRenderMode,
+        use api::{FontKey, FontInstanceKey, FontSize, FontTemplate, FontRenderMode,
                   IdNamespace, ColorU};
-        use api::units::{Au, DevicePoint};
+        use api::units::DevicePoint;
         use crate::render_backend::FrameId;
         use std::sync::Arc;
         use crate::glyph_rasterizer::{FORMAT, FontInstance, BaseFontInstance, GlyphKey, GlyphRasterizer};
@@ -1098,7 +1101,7 @@ mod test_glyph_rasterizer {
         let font = FontInstance::from_base(Arc::new(BaseFontInstance {
             instance_key: FontInstanceKey(IdNamespace(0), 0),
             font_key,
-            size: Au::from_px(32),
+            size: FontSize::from_f32_px(32.0),
             bg_color: ColorU::new(0, 0, 0, 0),
             render_mode: FontRenderMode::Subpixel,
             flags: Default::default(),

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

@@ -2,9 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use api::{ColorU, FontKey, FontRenderMode, GlyphDimensions};
+use api::{ColorU, FontKey, FontRenderMode, FontSize, GlyphDimensions};
 use api::{FontInstanceFlags, FontVariation, NativeFontHandle};
-use api::units::Au;
 use core_foundation::array::{CFArray, CFArrayRef};
 use core_foundation::base::TCFType;
 use core_foundation::dictionary::CFDictionary;
@@ -34,7 +33,7 @@ const INITIAL_CG_CONTEXT_SIDE_LENGTH: u32 = 32;
 
 pub struct FontContext {
     cg_fonts: FastHashMap<FontKey, CGFont>,
-    ct_fonts: FastHashMap<(FontKey, Au, Vec<FontVariation>), CTFont>,
+    ct_fonts: FastHashMap<(FontKey, FontSize, Vec<FontVariation>), CTFont>,
     #[allow(dead_code)]
     graphics_context: GraphicsContext,
     #[allow(dead_code)]
@@ -322,20 +321,21 @@ impl FontContext {
 
     pub fn delete_font_instance(&mut self, instance: &FontInstance) {
         // Remove the CoreText font corresponding to this instance.
-        self.ct_fonts.remove(&(instance.font_key, instance.size, instance.variations.clone()));
+        let size = FontSize::from_f64_px(instance.get_transformed_size());
+        self.ct_fonts.remove(&(instance.font_key, size, instance.variations.clone()));
     }
 
     fn get_ct_font(
         &mut self,
         font_key: FontKey,
-        size: Au,
+        size: f64,
         variations: &[FontVariation],
     ) -> Option<CTFont> {
-        match self.ct_fonts.entry((font_key, size, variations.to_vec())) {
+        match self.ct_fonts.entry((font_key, FontSize::from_f64_px(size), variations.to_vec())) {
             Entry::Occupied(entry) => Some((*entry.get()).clone()),
             Entry::Vacant(entry) => {
                 let cg_font = self.cg_fonts.get(&font_key)?;
-                let ct_font = new_ct_font_with_variations(cg_font, size.to_f64_px(), variations);
+                let ct_font = new_ct_font_with_variations(cg_font, size, variations);
                 entry.insert(ct_font.clone());
                 Some(ct_font)
             }
@@ -346,7 +346,7 @@ impl FontContext {
         let character = ch as u16;
         let mut glyph = 0;
 
-        self.get_ct_font(font_key, Au::from_px(16), &[])
+        self.get_ct_font(font_key, 16.0, &[])
             .and_then(|ref ct_font| {
                 unsafe {
                     let result = ct_font.get_glyphs_for_characters(&character, &mut glyph, 1);
@@ -366,7 +366,7 @@ impl FontContext {
         key: &GlyphKey,
     ) -> Option<GlyphDimensions> {
         let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
-        let size = font.size.scale_by(y_scale as f32);
+        let size = font.size.to_f64_px() * y_scale;
         self.get_ct_font(font.font_key, size, &font.variations)
             .and_then(|ref ct_font| {
                 let glyph = key.index() as CGGlyph;
@@ -387,7 +387,7 @@ impl FontContext {
                 }
                 let (mut tx, mut ty) = (0.0, 0.0);
                 if font.synthetic_italics.is_enabled() {
-                    let (shape_, (tx_, ty_)) = font.synthesize_italics(shape, size.to_f64_px());
+                    let (shape_, (tx_, ty_)) = font.synthesize_italics(shape, size);
                     shape = shape_;
                     tx = tx_;
                     ty = ty_;
@@ -502,7 +502,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 size = font.size.scale_by(y_scale as f32);
+        let size = font.size.to_f64_px() * y_scale;
         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
@@ -527,7 +527,7 @@ impl FontContext {
         }
         let (mut tx, mut ty) = (0.0, 0.0);
         if font.synthetic_italics.is_enabled() {
-            let (shape_, (tx_, ty_)) = font.synthesize_italics(shape, size.to_f64_px());
+            let (shape_, (tx_, ty_)) = font.synthesize_italics(shape, size);
             shape = shape_;
             tx = tx_;
             ty = ty_;

gfx/wr/webrender/src/scene_building.rs

@@ -5,7 +5,7 @@
 use api::{AlphaType, BorderDetails, BorderDisplayItem, BuiltDisplayListIter, PrimitiveFlags};
 use api::{ClipId, ColorF, CommonItemProperties, ComplexClipRegion, ComponentTransferFuncType, RasterSpace};
 use api::{DisplayItem, DisplayItemRef, ExtendMode, ExternalScrollId, FilterData, SharedFontInstanceMap};
-use api::{FilterOp, FilterPrimitive, FontInstanceKey, GlyphInstance, GlyphOptions, GradientStop};
+use api::{FilterOp, FilterPrimitive, FontInstanceKey, FontSize, GlyphInstance, GlyphOptions, GradientStop};
 use api::{IframeDisplayItem, ImageKey, ImageRendering, ItemRange, ColorDepth, QualitySettings};
 use api::{LineOrientation, LineStyle, NinePatchBorderSource, PipelineId, MixBlendMode, StackingContextFlags};
 use api::{PropertyBinding, ReferenceFrame, ReferenceFrameKind, ScrollFrameDisplayItem, ScrollSensitivity};
@@ -3037,7 +3037,7 @@ impl<'a> SceneBuilder<'a> {
             };
 
             // Trivial early out checks
-            if font_instance.size.0 <= 0 {
+            if font_instance.size <= FontSize::zero() {
                 return;
             }
 

gfx/wr/webrender_api/src/api.rs

@@ -522,7 +522,7 @@ impl Transaction {
         &mut self,
         key: font::FontInstanceKey,
         font_key: font::FontKey,
-        glyph_size: Au,
+        glyph_size: f32,
         options: Option<font::FontInstanceOptions>,
         platform_options: Option<font::FontInstancePlatformOptions>,
         variations: Vec<font::FontVariation>,
@@ -769,7 +769,7 @@ pub struct AddFontInstance {
     /// The font resource's key.
     pub font_key: font::FontKey,
     /// Glyph size in app units.
-    pub glyph_size: Au,
+    pub glyph_size: f32,
     ///
     pub options: Option<font::FontInstanceOptions>,
     ///

gfx/wr/webrender_api/src/font.rs

@@ -2,7 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use app_units::Au;
 #[cfg(target_os = "macos")]
 use core_foundation::string::CFString;
 #[cfg(target_os = "macos")]
@@ -23,6 +22,45 @@ use crate::api::IdNamespace;
 use crate::color::ColorU;
 use crate::units::LayoutPoint;
 
+/// Hashable floating-point storage for font size.
+#[repr(C)]
+#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, PartialOrd, Deserialize, Serialize)]
+pub struct FontSize(pub f32);
+
+impl Ord for FontSize {
+    fn cmp(&self, other: &FontSize) -> Ordering {
+        self.partial_cmp(other).unwrap_or(Ordering::Equal)
+    }
+}
+
+impl Eq for FontSize {}
+
+impl Hash for FontSize {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.0.to_bits().hash(state);
+    }
+}
+
+impl From<f32> for FontSize {
+    fn from(size: f32) -> Self { FontSize(size) }
+}
+
+impl From<FontSize> for f32 {
+    fn from(size: FontSize) -> Self { size.0 }
+}
+
+impl FontSize {
+    pub fn zero() -> Self { FontSize(0.0) }
+
+    pub fn from_f32_px(size: f32) -> Self { FontSize(size) }
+
+    pub fn to_f32_px(&self) -> f32 { self.0 }
+
+    pub fn from_f64_px(size: f64) -> Self { FontSize(size as f32) }
+
+    pub fn to_f64_px(&self) -> f64 { self.0 as f64 }
+}
+
 /// Immutable description of a font instance requested by the user of the API.
 ///
 /// `BaseFontInstance` can be identified by a `FontInstanceKey` so we should
@@ -36,7 +74,7 @@ pub struct BaseFontInstance {
     ///
     pub font_key: FontKey,
     ///
-    pub size: Au,
+    pub size: FontSize,
     ///
     pub bg_color: ColorU,
     ///
@@ -79,7 +117,7 @@ impl SharedFontInstanceMap {
         match self.map.read().unwrap().get(&key) {
             Some(instance) => Some(FontInstanceData {
                 font_key: instance.font_key,
-                size: instance.size,
+                size: instance.size.into(),
                 options: Some(FontInstanceOptions {
                   render_mode: instance.render_mode,
                   flags: instance.flags,
@@ -109,7 +147,7 @@ impl SharedFontInstanceMap {
         &mut self,
         instance_key: FontInstanceKey,
         font_key: FontKey,
-        size: Au,
+        size: f32,
         options: Option<FontInstanceOptions>,
         platform_options: Option<FontInstancePlatformOptions>,
         variations: Vec<FontVariation>,
@@ -125,7 +163,7 @@ impl SharedFontInstanceMap {
         let instance = Arc::new(BaseFontInstance {
             instance_key,
             font_key,
-            size,
+            size: size.into(),
             bg_color,
             render_mode,
             flags,
@@ -525,7 +563,7 @@ impl FontInstanceKey {
 #[derive(Clone)]
 pub struct FontInstanceData {
     pub font_key: FontKey,
-    pub size: Au,
+    pub size: f32,
     pub options: Option<FontInstanceOptions>,
     pub platform_options: Option<FontInstancePlatformOptions>,
     pub variations: Vec<FontVariation>,

gfx/wr/wrench/src/wrench.rs

@@ -353,7 +353,7 @@ impl Wrench {
         font_key: FontKey,
         instance_key: FontInstanceKey,
         text: &str,
-        size: Au,
+        size: f32,
         origin: LayoutPoint,
         flags: FontInstanceFlags,
     ) -> (Vec<u32>, Vec<LayoutPoint>, LayoutRect) {
@@ -399,7 +399,7 @@ impl Wrench {
                     // Extract the advances from the metrics. The get_glyph_dimensions API
                     // has a limitation that it can't currently get dimensions for non-renderable
                     // glyphs (e.g. spaces), so just use a rough estimate in that case.
-                    let space_advance = size.to_f32_px() / 3.0;
+                    let space_advance = size / 3.0;
                     cursor += direction * space_advance;
                 }
             }
@@ -543,7 +543,7 @@ impl Wrench {
 
     pub fn add_font_instance(&mut self,
         font_key: FontKey,
-        size: Au,
+        size: f32,
         flags: FontInstanceFlags,
         render_mode: Option<FontRenderMode>,
         bg_color: Option<ColorU>,

gfx/wr/wrench/src/yaml_frame_reader.rs

@@ -336,7 +336,7 @@ pub struct YamlFrameReader {
     image_map: HashMap<(PathBuf, Option<i64>), (ImageKey, LayoutSize)>,
 
     fonts: HashMap<FontDescriptor, FontKey>,
-    font_instances: HashMap<(FontKey, Au, FontInstanceFlags, Option<ColorU>, SyntheticItalics), FontInstanceKey>,
+    font_instances: HashMap<(FontKey, FontSize, FontInstanceFlags, Option<ColorU>, SyntheticItalics), FontInstanceKey>,
     font_render_mode: Option<FontRenderMode>,
     allow_mipmaps: bool,
 
@@ -823,7 +823,7 @@ impl YamlFrameReader {
     fn get_or_create_font_instance(
         &mut self,
         font_key: FontKey,
-        size: Au,
+        size: f32,
         bg_color: Option<ColorU>,
         flags: FontInstanceFlags,
         synthetic_italics: SyntheticItalics,
@@ -832,7 +832,7 @@ impl YamlFrameReader {
         let font_render_mode = self.font_render_mode;
 
         *self.font_instances
-            .entry((font_key, size, flags, bg_color, synthetic_italics))
+            .entry((font_key, size.into(), flags, bg_color, synthetic_italics))
             .or_insert_with(|| {
                 wrench.add_font_instance(
                     font_key,
@@ -1497,7 +1497,7 @@ impl YamlFrameReader {
         item: &Yaml,
         info: &mut CommonItemProperties,
     ) {
-        let size = item["size"].as_pt_to_au().unwrap_or(Au::from_f32_px(16.0));
+        let size = item["size"].as_pt_to_f32().unwrap_or(16.0);
         let color = item["color"].as_colorf().unwrap_or(ColorF::BLACK);
         let bg_color = item["bg-color"].as_colorf().map(|c| c.into());
         let synthetic_italics = if let Some(angle) = item["synthetic-italics"].as_f32() {

gfx/wr/wrench/src/yaml_helper.rs

@@ -25,8 +25,8 @@ pub trait YamlHelper {
     fn as_transform(&self, transform_origin: &LayoutPoint) -> Option<LayoutTransform>;
     fn as_colorf(&self) -> Option<ColorF>;
     fn as_vec_colorf(&self) -> Option<Vec<ColorF>>;
-    fn as_px_to_au(&self) -> Option<Au>;
-    fn as_pt_to_au(&self) -> Option<Au>;
+    fn as_px_to_f32(&self) -> Option<f32>;
+    fn as_pt_to_f32(&self) -> Option<f32>;
     fn as_vec_string(&self) -> Option<Vec<String>>;
     fn as_border_radius_component(&self) -> LayoutSize;
     fn as_border_radius(&self) -> Option<BorderRadius>;
@@ -267,18 +267,12 @@ impl YamlHelper for Yaml {
         }
     }
 
-    fn as_px_to_au(&self) -> Option<Au> {
-        match self.as_force_f32() {
-            Some(fv) => Some(Au::from_f32_px(fv)),
-            None => None,
-        }
+    fn as_px_to_f32(&self) -> Option<f32> {
+        self.as_force_f32()
     }
 
-    fn as_pt_to_au(&self) -> Option<Au> {
-        match self.as_force_f32() {
-            Some(fv) => Some(Au::from_f32_px(fv * 16. / 12.)),
-            None => None,
-        }
+    fn as_pt_to_f32(&self) -> Option<f32> {
+        self.as_force_f32().map(|fv| fv * 16. / 12.)
     }
 
     fn as_rect(&self) -> Option<LayoutRect> {