From a8791ddcbcdfd0e8e0f0f87cf5920b6e580ed717 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 15 Mar 2024 13:14:36 +0100 Subject: [PATCH] clippy: Fix remaining warnings in `gfx` for MacOS (#31669) --- components/gfx/font.rs | 2 +- components/gfx/font_context.rs | 2 +- components/gfx/font_template.rs | 11 ++-- components/gfx/platform/freetype/font.rs | 14 ++--- components/gfx/platform/macos/font.rs | 59 ++++++++++---------- components/gfx/platform/macos/font_list.rs | 2 +- components/gfx/platform/windows/font.rs | 25 ++++----- components/gfx/platform/windows/font_list.rs | 5 +- components/gfx/rendering_context.rs | 15 ++--- 9 files changed, 66 insertions(+), 69 deletions(-) diff --git a/components/gfx/font.rs b/components/gfx/font.rs index fe5de070421..6916b2e5261 100644 --- a/components/gfx/font.rs +++ b/components/gfx/font.rs @@ -59,7 +59,7 @@ pub trait FontHandleMethods: Sized { fctx: &FontContextHandle, template: Arc, pt_size: Option, - ) -> Result; + ) -> Result; fn template(&self) -> Arc; fn family_name(&self) -> Option; diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs index bd4a80d82f3..2febda51d78 100644 --- a/components/gfx/font_context.rs +++ b/components/gfx/font_context.rs @@ -207,7 +207,7 @@ impl FontContext { info: FontTemplateInfo, descriptor: FontDescriptor, synthesized_small_caps: Option, - ) -> Result { + ) -> Result { let handle = FontHandle::new_from_template( &self.platform_handle, info.font_template, diff --git a/components/gfx/font_template.rs b/components/gfx/font_template.rs index 0cb7ca80828..00d95159632 100644 --- a/components/gfx/font_template.rs +++ b/components/gfx/font_template.rs @@ -181,16 +181,15 @@ impl FontTemplate { }) } - fn instantiate(&mut self, font_context: &FontContextHandle) -> Result<(), ()> { + fn instantiate(&mut self, font_context: &FontContextHandle) -> Result<(), &'static str> { if !self.is_valid { - return Err(()); + return Err("Invalid font template"); } - let data = self.data().map_err(|_| ())?; - let handle: Result = - FontHandleMethods::new_from_template(font_context, data, None); + let data = self.data().map_err(|_| "Could not get FontTemplate data")?; + let handle = FontHandleMethods::new_from_template(font_context, data, None); self.is_valid = handle.is_ok(); - let handle = handle?; + let handle: FontHandle = handle?; self.descriptor = Some(FontTemplateDescriptor::new( handle.boldness(), handle.stretchiness(), diff --git a/components/gfx/platform/freetype/font.rs b/components/gfx/platform/freetype/font.rs index 1fd429aaefa..024b3c563e1 100644 --- a/components/gfx/platform/freetype/font.rs +++ b/components/gfx/platform/freetype/font.rs @@ -97,7 +97,7 @@ fn create_face( lib: FT_Library, template: &FontTemplateData, pt_size: Option, -) -> Result { +) -> Result { unsafe { let mut face: FT_Face = ptr::null_mut(); let face_index = 0 as FT_Long; @@ -125,11 +125,11 @@ fn create_face( }; if !succeeded(result) || face.is_null() { - return Err(()); + return Err("Could not create FreeType face"); } if let Some(s) = pt_size { - FontHandle::set_char_size(face, s).or(Err(()))? + FontHandle::set_char_size(face, s)? } Ok(face) @@ -141,10 +141,10 @@ impl FontHandleMethods for FontHandle { fctx: &FontContextHandle, template: Arc, pt_size: Option, - ) -> Result { + ) -> Result { let ft_ctx: FT_Library = fctx.ctx.ctx; if ft_ctx.is_null() { - return Err(()); + return Err("Null FT_Library"); } let face = create_face(ft_ctx, &template, pt_size)?; @@ -370,7 +370,7 @@ impl FontHandleMethods for FontHandle { } impl<'a> FontHandle { - fn set_char_size(face: FT_Face, pt_size: Au) -> Result<(), ()> { + fn set_char_size(face: FT_Face, pt_size: Au) -> Result<(), &'static str> { let char_size = pt_size.to_f64_px() * 64.0 + 0.5; unsafe { @@ -378,7 +378,7 @@ impl<'a> FontHandle { if succeeded(result) { Ok(()) } else { - Err(()) + Err("FT_Set_Char_Size failed") } } } diff --git a/components/gfx/platform/macos/font.rs b/components/gfx/platform/macos/font.rs index b9681e246fd..764519f8632 100644 --- a/components/gfx/platform/macos/font.rs +++ b/components/gfx/platform/macos/font.rs @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use std::cmp::Ordering; use std::ops::Range; use std::sync::Arc; use std::{fmt, ptr}; @@ -52,7 +53,7 @@ fn au_from_pt(pt: f64) -> Au { impl FontTable { pub fn wrap(data: CFData) -> FontTable { - FontTable { data: data } + FontTable { data } } } @@ -76,7 +77,7 @@ impl FontHandle { fn find_h_kern_subtable(&self) -> Option { let font_table = self.table_for_tag(KERN)?; let mut result = CachedKernTable { - font_table: font_table, + font_table, pair_data_range: 0..0, px_per_font_unit: 0.0, }; @@ -101,7 +102,7 @@ impl FontHandle { let end = start + len; if cov == KERN_COVERAGE_HORIZONTAL_FORMAT_0 { // Found a matching subtable. - if result.pair_data_range.len() > 0 { + if !result.pair_data_range.is_empty() { debug!("Found multiple horizontal kern tables. Disable fast path."); return None; } @@ -117,13 +118,13 @@ impl FontHandle { } let pt_per_font_unit = - self.ctfont.pt_size() as f64 / self.ctfont.units_per_em() as f64; + self.ctfont.pt_size() / self.ctfont.units_per_em() as f64; result.px_per_font_unit = pt_to_px(pt_per_font_unit); } start = end; } } - if result.pair_data_range.len() > 0 { + if !result.pair_data_range.is_empty() { Some(result) } else { None @@ -147,12 +148,12 @@ impl CachedKernTable { while start < end { let i = (start + end) / 2; let key = BigEndian::read_u32(&pairs[i * KERN_PAIR_LEN..]); - if key > query { - end = i; - } else if key < query { - start = i + 1; - } else { - return Some(BigEndian::read_i16(&pairs[i * KERN_PAIR_LEN + 4..])); + match key.cmp(&query) { + Ordering::Less => start = i + 1, + Ordering::Equal => { + return Some(BigEndian::read_i16(&pairs[i * KERN_PAIR_LEN + 4..])) + }, + Ordering::Greater => end = i, } } None @@ -170,7 +171,7 @@ impl FontHandleMethods for FontHandle { _fctx: &FontContextHandle, template: Arc, pt_size: Option, - ) -> Result { + ) -> Result { let size = match pt_size { Some(s) => s.to_f64_px(), None => 0.0, @@ -190,7 +191,7 @@ impl FontHandleMethods for FontHandle { handle.table_for_tag(GSUB).is_none(); Ok(handle) }, - None => Err(()), + None => Err("Could not generate CTFont for FontTemplateData"), } } @@ -247,7 +248,7 @@ impl FontHandleMethods for FontHandle { return None; } - return Some(glyphs[0] as GlyphId); + Some(glyphs[0] as GlyphId) } fn glyph_h_kerning(&self, first_glyph: GlyphId, second_glyph: GlyphId) -> FractionalPixel { @@ -278,43 +279,43 @@ impl FontHandleMethods for FontHandle { fn metrics(&self) -> FontMetrics { let bounding_rect: CGRect = self.ctfont.bounding_box(); - let ascent = self.ctfont.ascent() as f64; - let descent = self.ctfont.descent() as f64; - let em_size = Au::from_f64_px(self.ctfont.pt_size() as f64); - let leading = self.ctfont.leading() as f64; + let ascent = self.ctfont.ascent(); + let descent = self.ctfont.descent(); + let em_size = Au::from_f64_px(self.ctfont.pt_size()); + let leading = self.ctfont.leading(); - let scale = px_to_pt(self.ctfont.pt_size() as f64) / (ascent + descent); + let scale = px_to_pt(self.ctfont.pt_size()) / (ascent + descent); let line_gap = (ascent + descent + leading + 0.5).floor(); - let max_advance_width = au_from_pt(bounding_rect.size.width as f64); + let max_advance = au_from_pt(bounding_rect.size.width); let average_advance = self .glyph_index('0') .and_then(|idx| self.glyph_h_advance(idx)) .map(Au::from_f64_px) - .unwrap_or(max_advance_width); + .unwrap_or(max_advance); let metrics = FontMetrics { - underline_size: au_from_pt(self.ctfont.underline_thickness() as f64), + underline_size: au_from_pt(self.ctfont.underline_thickness()), // TODO(Issue #201): underline metrics are not reliable. Have to pull out of font table // directly. // // see also: https://bugs.webkit.org/show_bug.cgi?id=16768 // see also: https://bugreports.qt-project.org/browse/QTBUG-13364 - underline_offset: au_from_pt(self.ctfont.underline_position() as f64), + underline_offset: au_from_pt(self.ctfont.underline_position()), strikeout_size: Au(0), // FIXME(Issue #942) strikeout_offset: Au(0), // FIXME(Issue #942) leading: au_from_pt(leading), - x_height: au_from_pt((self.ctfont.x_height() as f64) * scale), - em_size: em_size, + x_height: au_from_pt(self.ctfont.x_height() * scale), + em_size, ascent: au_from_pt(ascent * scale), descent: au_from_pt(descent * scale), - max_advance: max_advance_width, - average_advance: average_advance, + max_advance, + average_advance, line_gap: Au::from_f64_px(line_gap), }; debug!( "Font metrics (@{} pt): {:?}", - self.ctfont.pt_size() as f64, + self.ctfont.pt_size(), metrics ); metrics @@ -322,7 +323,7 @@ impl FontHandleMethods for FontHandle { fn table_for_tag(&self, tag: FontTableTag) -> Option { let result: Option = self.ctfont.get_font_table(tag); - result.and_then(|data| Some(FontTable::wrap(data))) + result.map(FontTable::wrap) } fn identifier(&self) -> &FontIdentifier { diff --git a/components/gfx/platform/macos/font_list.rs b/components/gfx/platform/macos/font_list.rs index 490f5e44028..14f9f1fa34a 100644 --- a/components/gfx/platform/macos/font_list.rs +++ b/components/gfx/platform/macos/font_list.rs @@ -213,4 +213,4 @@ pub fn fallback_font_families(codepoint: Option) -> Vec<&'static str> { families } -pub static SANS_SERIF_FONT_FAMILY: &'static str = "Helvetica"; +pub static SANS_SERIF_FONT_FAMILY: &str = "Helvetica"; diff --git a/components/gfx/platform/windows/font.rs b/components/gfx/platform/windows/font.rs index 8b05eb3cf07..73d6687ffc8 100644 --- a/components/gfx/platform/windows/font.rs +++ b/components/gfx/platform/windows/font.rs @@ -63,8 +63,6 @@ fn make_tag(tag_bytes: &[u8]) -> FontTableTag { unsafe { *(tag_bytes.as_ptr() as *const FontTableTag) } } -macro_rules! try_lossy(($result:expr) => ($result.map_err(|_| (()))?)); - // We need the font (DWriteFont) in order to be able to query things like // the family name, face name, weight, etc. On Windows 10, the // DWriteFontFace3 interface provides this on the FontFace, but that's only @@ -83,7 +81,7 @@ struct FontInfo { } impl FontInfo { - fn new_from_face(face: &FontFace) -> Result { + fn new_from_face(face: &FontFace) -> Result { use std::cmp::{max, min}; use std::collections::HashMap; use std::io::Cursor; @@ -95,11 +93,11 @@ impl FontInfo { let names_bytes = face.get_font_table(make_tag(b"name")); let windows_metrics_bytes = face.get_font_table(make_tag(b"OS/2")); if names_bytes.is_none() || windows_metrics_bytes.is_none() { - return Err(()); + return Err("No 'name' or 'OS/2' tables"); } let mut cursor = Cursor::new(names_bytes.as_ref().unwrap()); - let table = try_lossy!(Names::read(&mut cursor)); + let table = Names::read(&mut cursor).map_err(|_| "Could not read 'name' table")?; let language_tags = table.language_tags().collect::>(); let mut names = table .iter() @@ -113,15 +111,15 @@ impl FontInfo { .collect::>(); let family = match names.remove(&NameID::FontFamilyName) { Some(family) => family, - _ => return Err(()), + _ => return Err("Could not find family"), }; let face = match names.remove(&NameID::FontSubfamilyName) { Some(face) => face, - _ => return Err(()), + _ => return Err("Could not find subfamily"), }; let mut cursor = Cursor::new(windows_metrics_bytes.as_ref().unwrap()); - let table = try_lossy!(WindowsMetrics::read(&mut cursor)); + let table = WindowsMetrics::read(&mut cursor).map_err(|_| "Could not read OS/2 table")?; let (weight_val, width_val, italic_bool) = match table { WindowsMetrics::Version0(ref m) => { (m.weight_class, m.width_class, m.selection_flags.0 & 1 == 1) @@ -151,7 +149,7 @@ impl FontInfo { 7 => FontStretchKeyword::Expanded, 8 => FontStretchKeyword::ExtraExpanded, 9 => FontStretchKeyword::UltraExpanded, - _ => return Err(()), + _ => return Err("Unknown stretch size"), } .compute(); @@ -170,7 +168,7 @@ impl FontInfo { }) } - fn new_from_font(font: &Font) -> Result { + fn new_from_font(font: &Font) -> Result { let style = match font.style() { FontStyle::Normal => StyleFontStyle::NORMAL, FontStyle::Oblique => StyleFontStyle::OBLIQUE, @@ -233,15 +231,16 @@ impl FontHandleMethods for FontHandle { _: &FontContextHandle, template: Arc, pt_size: Option, - ) -> Result { + ) -> Result { let (face, info) = match template.get_font() { Some(font) => (font.create_font_face(), FontInfo::new_from_font(&font)?), None => { let bytes = template.bytes(); - let font_file = FontFile::new_from_data(bytes).ok_or(())?; + let font_file = + FontFile::new_from_data(bytes).ok_or_else(|| "Could not create FontFile")?; let face = font_file .create_face(0, dwrote::DWRITE_FONT_SIMULATIONS_NONE) - .map_err(|_| ())?; + .map_err(|_| "Could not create FontFace")?; let info = FontInfo::new_from_face(&face)?; (face, info) }, diff --git a/components/gfx/platform/windows/font_list.rs b/components/gfx/platform/windows/font_list.rs index 6da723cdc38..bc5e0030972 100644 --- a/components/gfx/platform/windows/font_list.rs +++ b/components/gfx/platform/windows/font_list.rs @@ -2,14 +2,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -use std::collections::HashMap; use std::hash::Hash; -use std::sync::{Arc, Mutex}; +use std::sync::Arc; use dwrote::{FontCollection, FontDescriptor}; -use lazy_static::lazy_static; use serde::{Deserialize, Serialize}; -use servo_atoms::Atom; use ucd::{Codepoint, UnicodeBlock}; use crate::text::util::unicode_plane; diff --git a/components/gfx/rendering_context.rs b/components/gfx/rendering_context.rs index 9f3f733efd3..3ea7cbe6ebc 100644 --- a/components/gfx/rendering_context.rs +++ b/components/gfx/rendering_context.rs @@ -90,22 +90,23 @@ impl RenderingContext { Ok(RenderingContext(Rc::new(data))) } - pub fn create_surface_texture( - &self, - surface: Surface, - ) -> Result { + pub fn create_surface_texture(&self, surface: Surface) -> Result { let device = &self.0.device.borrow(); let context = &mut self.0.context.borrow_mut(); - device.create_surface_texture(context, surface) + device + .create_surface_texture(context, surface) + .map_err(|(error, _)| error) } pub fn destroy_surface_texture( &self, surface_texture: SurfaceTexture, - ) -> Result { + ) -> Result { let device = &self.0.device.borrow(); let context = &mut self.0.context.borrow_mut(); - device.destroy_surface_texture(context, surface_texture) + device + .destroy_surface_texture(context, surface_texture) + .map_err(|(error, _)| error) } pub fn make_gl_context_current(&self) -> Result<(), Error> {