From fcf4beeac074005cddf110b88011cec3e86954e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Tue, 9 Sep 2025 00:29:33 +0200 Subject: [PATCH] Seperate freetype face and named instance indices (#39213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In servo, each `LocalFontIdentifier` has an `fn index() -> u32`, which returns the index of the font within the font file in case it is a font collection (`.ttc` instead of `.ttf`). The way this index is obtained is platform-dependent. On systems using `fontconfig`, we get the index by querying `FC_INDEX`: https://github.com/servo/servo/blob/d2c78db981a508fab67f01ecb1a4eb90061c2e94/components/fonts/platform/freetype/font_list.rs#L109-L112 There is a sneaky bug here: In addition to the aforementioned face index, the value for `FC_INDEX` contains the index of the named instance of the face in the upper 16 bits. This behaviour is completely undocumented, but FreeType uses the same format: https://freetype.org/freetype2/docs/reference/ft2-face_creation.html#ft_open_face. This wasn't a problem for the longest time, because the only consumer of the `index` value coming from fontconfig was FreeType. However, sometime after https://github.com/servo/servo/commit/29e618dcf779c424301c70c15270e8a7cda7aaa0, we started also passing it to`read-fonts` . `read-fonts` expects only the face index, causing it to return errors as seen in https://github.com/servo/servo/issues/39209. The fix is to seperate the two indices when storing them in a `LocalFontDescriptor` and pass only the lower 16 bits to `read-fonts`. I'm unsure whether we should continue passing the named instance index to FreeType since we don't actually support variable fonts, but I've opted to keep the current behaviour for now. Fixes: https://github.com/servo/servo/issues/39209 Signed-off-by: Simon Wülker --- .../platform/freetype/android/font_list.rs | 3 ++- components/fonts/platform/freetype/font.rs | 6 ++++- .../fonts/platform/freetype/font_list.rs | 3 ++- .../fonts/platform/freetype/ohos/font_list.rs | 3 ++- components/fonts/tests/font_context.rs | 3 ++- components/shared/fonts/font_identifier.rs | 27 ++++++++++++++++--- 6 files changed, 36 insertions(+), 9 deletions(-) diff --git a/components/fonts/platform/freetype/android/font_list.rs b/components/fonts/platform/freetype/android/font_list.rs index f8bf939eebd..2f1ecee14ff 100644 --- a/components/fonts/platform/freetype/android/font_list.rs +++ b/components/fonts/platform/freetype/android/font_list.rs @@ -432,7 +432,8 @@ where let mut produce_font = |font: &Font| { let local_font_identifier = LocalFontIdentifier { path: Atom::from(FontList::font_absolute_path(&font.filename)), - variation_index: 0, + face_index: 0, + named_instance_index: 0, }; let stretch = StyleFontStretch::NORMAL; let weight = font diff --git a/components/fonts/platform/freetype/font.rs b/components/fonts/platform/freetype/font.rs index 92998e8ae94..4dfca9cf7c3 100644 --- a/components/fonts/platform/freetype/font.rs +++ b/components/fonts/platform/freetype/font.rs @@ -96,7 +96,11 @@ impl PlatformFontMethods for PlatformFont { let library = FreeTypeLibraryHandle::get().lock(); let filename = CString::new(&*font_identifier.path).expect("filename contains NUL byte!"); - let face = FreeTypeFace::new_from_file(&library, &filename, font_identifier.index())?; + let face = FreeTypeFace::new_from_file( + &library, + &filename, + font_identifier.face_index_for_freetype(), + )?; let normalized_variations = face.set_variations_for_font(variations, &library)?; diff --git a/components/fonts/platform/freetype/font_list.rs b/components/fonts/platform/freetype/font_list.rs index 585daa84177..57759e20f91 100644 --- a/components/fonts/platform/freetype/font_list.rs +++ b/components/fonts/platform/freetype/font_list.rs @@ -123,7 +123,8 @@ where let local_font_identifier = LocalFontIdentifier { path: Atom::from(c_str_to_string(path as *const c_char)), - variation_index: index as i32, + face_index: (index & 0xFFFF) as u16, + named_instance_index: (index >> 16) as u16, }; let descriptor = FontTemplateDescriptor::new(weight, stretch, style); diff --git a/components/fonts/platform/freetype/ohos/font_list.rs b/components/fonts/platform/freetype/ohos/font_list.rs index f3776f951b7..345a2b6827c 100644 --- a/components/fonts/platform/freetype/ohos/font_list.rs +++ b/components/fonts/platform/freetype/ohos/font_list.rs @@ -444,7 +444,8 @@ where let mut produce_font = |font: &Font| { let local_font_identifier = LocalFontIdentifier { path: Atom::from(font.filepath.clone()), - variation_index: 0, + face_index: 0, + named_instance_index: 0, }; let stretch = font.width.into(); let weight = font diff --git a/components/fonts/tests/font_context.rs b/components/fonts/tests/font_context.rs index a9e3a3f9b25..8b78cf60df6 100644 --- a/components/fonts/tests/font_context.rs +++ b/components/fonts/tests/font_context.rs @@ -186,7 +186,8 @@ mod font_context { let local_font_identifier = LocalFontIdentifier { path: path.to_str().expect("Could not load test font").into(), - variation_index: 0, + face_index: 0, + named_instance_index: 0, }; let handle = PlatformFont::new_from_local_font_identifier( local_font_identifier.clone(), diff --git a/components/shared/fonts/font_identifier.rs b/components/shared/fonts/font_identifier.rs index 00050c968c1..8029252a20c 100644 --- a/components/shared/fonts/font_identifier.rs +++ b/components/shared/fonts/font_identifier.rs @@ -41,18 +41,26 @@ mod platform { /// The path to the font. pub path: Atom, /// The variation index within the font. - pub variation_index: i32, + pub face_index: u16, + /// The index of the named instance within the font. + /// + /// For non-variable fonts, this is ignored. + pub named_instance_index: u16, } impl LocalFontIdentifier { pub fn index(&self) -> u32 { - self.variation_index.try_into().unwrap() + self.face_index as u32 + } + + pub fn named_instance_index(&self) -> u32 { + self.named_instance_index as u32 } pub fn native_font_handle(&self) -> NativeFontHandle { NativeFontHandle { path: PathBuf::from(&*self.path), - index: self.variation_index as u32, + index: self.face_index as u32, } } @@ -64,9 +72,20 @@ mod platform { Some(FontDataAndIndex { data, - index: self.variation_index as u32, + index: self.face_index as u32, }) } + + /// Fontconfig and FreeType use a packed format to represent face and + /// named instance indexes in a single integer. The first 16 bits make + /// up the named instance index and the second 16 bits make up the + /// face index. + /// + /// See + /// for more information. + pub fn face_index_for_freetype(&self) -> u32 { + ((self.named_instance_index()) << 16) | self.index() + } } }