fonts: Remove the per-FontGroup cached fallback font (#35705)

Instead of keeping a per-FontGroup cache of the previously used fallback
font, cache this value in the caller of `FontGroup::find_by_codepoint`.
The problem with caching this value in the `FontGroup` is that it can
make one layout different from the next.

Still, it is important to cache the value somewhere so that, for
instance, Chinese character don't have to continuously walk through the
entire fallback list when laying out. The heuristic here is to try to
last used font first if the `Script`s match. At the very least this
should make one layout consistent with the next.

Fixes #35704.
Fixes #35697.
Fixes #35689.
Fixes #35679.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/canvas/canvas_data.rs

@@ -710,7 +710,7 @@ impl<'a> CanvasData<'a> {
             // TODO: This should ultimately handle emoji variation selectors, but raqote does not yet
             // have support for color glyphs.
             let script = Script::from(character);
-            let font = font_group.find_by_codepoint(&self.font_context, character, None);
+            let font = font_group.find_by_codepoint(&self.font_context, character, None, None);
 
             if !current_text_run.script_and_font_compatible(script, &font) {
                 let previous_text_run = mem::replace(

components/fonts/font.rs

@@ -568,8 +568,6 @@ pub type FontRef = Arc<Font>;
 pub struct FontGroup {
     descriptor: FontDescriptor,
     families: SmallVec<[FontGroupFamily; 8]>,
-    #[ignore_malloc_size_of = "This measured in the FontContext font cache."]
-    last_matching_fallback: Option<FontRef>,
 }
 
 impl FontGroup {
@@ -584,7 +582,6 @@ impl FontGroup {
         FontGroup {
             descriptor,
             families,
-            last_matching_fallback: None,
         }
     }
 
@@ -597,6 +594,7 @@ impl FontGroup {
         font_context: &FontContext,
         codepoint: char,
         next_codepoint: Option<char>,
+        first_fallback: Option<FontRef>,
     ) -> Option<FontRef> {
         // Tab characters are converted into spaces when rendering.
         // TODO: We should not render a tab character. Instead they should be converted into tab stops
@@ -642,11 +640,11 @@ impl FontGroup {
             return font_or_synthesized_small_caps(font);
         }
 
-        if let Some(ref last_matching_fallback) = self.last_matching_fallback {
-            if char_in_template(last_matching_fallback.template.clone()) &&
-                font_has_glyph_and_presentation(last_matching_fallback)
+        if let Some(ref first_fallback) = first_fallback {
+            if char_in_template(first_fallback.template.clone()) &&
+                font_has_glyph_and_presentation(first_fallback)
             {
-                return font_or_synthesized_small_caps(last_matching_fallback.clone());
+                return font_or_synthesized_small_caps(first_fallback.clone());
             }
         }
 
@@ -656,7 +654,6 @@ impl FontGroup {
             char_in_template,
             font_has_glyph_and_presentation,
         ) {
-            self.last_matching_fallback = Some(font.clone());
             return font_or_synthesized_small_caps(font);
         }
 

components/fonts/tests/font_context.rs

@@ -276,7 +276,7 @@ mod font_context {
 
         let font = group
             .write()
-            .find_by_codepoint(&mut context.context, 'a', None)
+            .find_by_codepoint(&mut context.context, 'a', None, None)
             .unwrap();
         assert_eq!(&font_face_name(&font.identifier()), "csstest-ascii");
         assert_eq!(
@@ -290,7 +290,7 @@ mod font_context {
 
         let font = group
             .write()
-            .find_by_codepoint(&mut context.context, 'a', None)
+            .find_by_codepoint(&mut context.context, 'a', None, None)
             .unwrap();
         assert_eq!(&font_face_name(&font.identifier()), "csstest-ascii");
         assert_eq!(
@@ -304,7 +304,7 @@ mod font_context {
 
         let font = group
             .write()
-            .find_by_codepoint(&mut context.context, 'á', None)
+            .find_by_codepoint(&mut context.context, 'á', None, None)
             .unwrap();
         assert_eq!(&font_face_name(&font.identifier()), "csstest-basic-regular");
         assert_eq!(
@@ -328,7 +328,7 @@ mod font_context {
 
         let font = group
             .write()
-            .find_by_codepoint(&mut context.context, 'a', None)
+            .find_by_codepoint(&mut context.context, 'a', None, None)
             .unwrap();
         assert_eq!(
             &font_face_name(&font.identifier()),
@@ -338,7 +338,7 @@ mod font_context {
 
         let font = group
             .write()
-            .find_by_codepoint(&mut context.context, 'á', None)
+            .find_by_codepoint(&mut context.context, 'á', None, None)
             .unwrap();
         assert_eq!(
             &font_face_name(&font.identifier()),

components/layout/text.rs

@@ -210,7 +210,7 @@ impl TextRunScanner {
                     .unwrap_or_else(|| {
                         let space_width = font_group
                             .write()
-                            .find_by_codepoint(font_context, ' ', None)
+                            .find_by_codepoint(font_context, ' ', None, None)
                             .and_then(|font| {
                                 font.glyph_index(' ')
                                     .map(|glyph_id| font.glyph_h_advance(glyph_id))
@@ -252,10 +252,12 @@ impl TextRunScanner {
                 let (mut start_position, mut end_position) = (0, 0);
                 for (byte_index, character) in text.char_indices() {
                     if !character.is_control() {
-                        let font =
-                            font_group
-                                .write()
-                                .find_by_codepoint(font_context, character, None);
+                        let font = font_group.write().find_by_codepoint(
+                            font_context,
+                            character,
+                            None,
+                            None,
+                        );
 
                         let bidi_level = match bidi_levels {
                             Some(levels) => levels[*paragraph_bytes_processed],

components/layout_2020/flow/inline/text_run.rs

@@ -430,17 +430,29 @@ impl TextRun {
                 continue;
             }
 
-            let Some(font) =
-                font_group
-                    .write()
-                    .find_by_codepoint(font_context, character, next_character)
-            else {
+            // If the script and BiDi level do not change, use the current font as the first fallback. This
+            // can potentially speed up fallback on long font lists or with uncommon scripts which might be
+            // at the bottom of the list.
+            let script = Script::from(character);
+            let bidi_level = bidi_info.levels[current_byte_index];
+            let current_font = current.as_ref().and_then(|(text_run_segment, font)| {
+                if text_run_segment.bidi_level == bidi_level && text_run_segment.script == script {
+                    Some(font.clone())
+                } else {
+                    None
+                }
+            });
+
+            let Some(font) = font_group.write().find_by_codepoint(
+                font_context,
+                character,
+                next_character,
+                current_font,
+            ) else {
                 continue;
             };
 
             // If the existing segment is compatible with the character, keep going.
-            let script = Script::from(character);
-            let bidi_level = bidi_info.levels[current_byte_index];
             if let Some(current) = current.as_mut() {
                 if current.0.update_if_compatible(
                     &font,

components/layout_thread_2020/lib.rs

@@ -1162,7 +1162,7 @@ impl FontMetricsProvider for LayoutFontMetricsProvider {
             .or_else(|| {
                 font_group
                     .write()
-                    .find_by_codepoint(font_context, '0', None)?
+                    .find_by_codepoint(font_context, '0', None, None)?
                     .metrics
                     .zero_horizontal_advance
             })
@@ -1173,7 +1173,7 @@ impl FontMetricsProvider for LayoutFontMetricsProvider {
             .or_else(|| {
                 font_group
                     .write()
-                    .find_by_codepoint(font_context, '\u{6C34}', None)?
+                    .find_by_codepoint(font_context, '\u{6C34}', None, None)?
                     .metrics
                     .ic_horizontal_advance
             })

tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-001.xht.ini

@@ -0,0 +1,2 @@
+[bidi-breaking-001.xht]
+  expected: FAIL

tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-002.xht.ini

@@ -0,0 +1,2 @@
+[bidi-breaking-002.xht]
+  expected: FAIL

tests/wpt/meta/css/css-content/quotes-016.html.ini

@@ -1,2 +0,0 @@
-[quotes-016.html]
-  expected: FAIL

tests/wpt/meta/css/css-content/quotes-026.html.ini

@@ -1,2 +0,0 @@
-[quotes-026.html]
-  expected: FAIL

tests/wpt/meta/css/css-text/word-break/word-break-keep-all-005.html.ini

@@ -1,2 +0,0 @@
-[word-break-keep-all-005.html]
-  expected: FAIL

tests/wpt/meta/css/css-text/word-break/word-break-keep-all-007.html.ini

@@ -1,2 +0,0 @@
-[word-break-keep-all-007.html]
-  expected: FAIL

tests/wpt/meta/css/css-text/word-break/word-break-keep-all-008.html.ini

@@ -1,2 +0,0 @@
-[word-break-keep-all-008.html]
-  expected: FAIL