From f949d2adc877b2be60f3e579d61bf4738cad0cf9 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 28 Feb 2025 15:33:21 +0100 Subject: [PATCH] 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 --- components/canvas/canvas_data.rs | 2 +- components/fonts/font.rs | 13 ++++------ components/fonts/tests/font_context.rs | 10 +++---- components/layout/text.rs | 12 +++++---- .../layout_2020/flow/inline/text_run.rs | 26 ++++++++++++++----- components/layout_thread_2020/lib.rs | 4 +-- .../CSS2/bidi-text/bidi-breaking-001.xht.ini | 2 ++ .../CSS2/bidi-text/bidi-breaking-002.xht.ini | 2 ++ .../meta/css/css-content/quotes-016.html.ini | 2 -- .../meta/css/css-content/quotes-026.html.ini | 2 -- .../word-break-keep-all-005.html.ini | 2 -- .../word-break-keep-all-007.html.ini | 2 -- .../word-break-keep-all-008.html.ini | 2 -- 13 files changed, 43 insertions(+), 38 deletions(-) create mode 100644 tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-001.xht.ini create mode 100644 tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-002.xht.ini delete mode 100644 tests/wpt/meta/css/css-content/quotes-016.html.ini delete mode 100644 tests/wpt/meta/css/css-content/quotes-026.html.ini delete mode 100644 tests/wpt/meta/css/css-text/word-break/word-break-keep-all-005.html.ini delete mode 100644 tests/wpt/meta/css/css-text/word-break/word-break-keep-all-007.html.ini delete mode 100644 tests/wpt/meta/css/css-text/word-break/word-break-keep-all-008.html.ini diff --git a/components/canvas/canvas_data.rs b/components/canvas/canvas_data.rs index a31fc1d53a2..1e2c88d5f53 100644 --- a/components/canvas/canvas_data.rs +++ b/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( diff --git a/components/fonts/font.rs b/components/fonts/font.rs index 7cedbd4d0f5..e0d6cd065d8 100644 --- a/components/fonts/font.rs +++ b/components/fonts/font.rs @@ -568,8 +568,6 @@ pub type FontRef = Arc; pub struct FontGroup { descriptor: FontDescriptor, families: SmallVec<[FontGroupFamily; 8]>, - #[ignore_malloc_size_of = "This measured in the FontContext font cache."] - last_matching_fallback: Option, } 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, + first_fallback: Option, ) -> Option { // 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); } diff --git a/components/fonts/tests/font_context.rs b/components/fonts/tests/font_context.rs index cf6268970d2..e6afab2e725 100644 --- a/components/fonts/tests/font_context.rs +++ b/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()), diff --git a/components/layout/text.rs b/components/layout/text.rs index ea53bd3a98b..c8bde51f635 100644 --- a/components/layout/text.rs +++ b/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], diff --git a/components/layout_2020/flow/inline/text_run.rs b/components/layout_2020/flow/inline/text_run.rs index 78160337396..a10664ee672 100644 --- a/components/layout_2020/flow/inline/text_run.rs +++ b/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, diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index a591226b946..80d98ebb7d6 100644 --- a/components/layout_thread_2020/lib.rs +++ b/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 }) diff --git a/tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-001.xht.ini b/tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-001.xht.ini new file mode 100644 index 00000000000..8fd11e91d82 --- /dev/null +++ b/tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-001.xht.ini @@ -0,0 +1,2 @@ +[bidi-breaking-001.xht] + expected: FAIL diff --git a/tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-002.xht.ini b/tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-002.xht.ini new file mode 100644 index 00000000000..b6445461a9c --- /dev/null +++ b/tests/wpt/meta/css/CSS2/bidi-text/bidi-breaking-002.xht.ini @@ -0,0 +1,2 @@ +[bidi-breaking-002.xht] + expected: FAIL diff --git a/tests/wpt/meta/css/css-content/quotes-016.html.ini b/tests/wpt/meta/css/css-content/quotes-016.html.ini deleted file mode 100644 index 90393afab7f..00000000000 --- a/tests/wpt/meta/css/css-content/quotes-016.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[quotes-016.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-content/quotes-026.html.ini b/tests/wpt/meta/css/css-content/quotes-026.html.ini deleted file mode 100644 index 3b442679653..00000000000 --- a/tests/wpt/meta/css/css-content/quotes-026.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[quotes-026.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-005.html.ini b/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-005.html.ini deleted file mode 100644 index feddd4f48c1..00000000000 --- a/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-005.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[word-break-keep-all-005.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-007.html.ini b/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-007.html.ini deleted file mode 100644 index 0610498c013..00000000000 --- a/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-007.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[word-break-keep-all-007.html] - expected: FAIL diff --git a/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-008.html.ini b/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-008.html.ini deleted file mode 100644 index 0ae96b6eb3c..00000000000 --- a/tests/wpt/meta/css/css-text/word-break/word-break-keep-all-008.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[word-break-keep-all-008.html] - expected: FAIL