From 88033bd65435ff502b847ecc783616c4f2ff74bd Mon Sep 17 00:00:00 2001 From: eri Date: Fri, 8 Mar 2024 08:10:15 +0100 Subject: [PATCH] clippy: fix warnings in components/gfx (#31560) * clippy: fix warnings in components/gfx * refactor: switched the order of impl so that its intent is clearer * fix: add font context default in other platforms --- components/gfx/font.rs | 32 ++++----- components/gfx/font_cache_thread.rs | 34 ++++------ components/gfx/font_context.rs | 53 +++++++-------- components/gfx/font_template.rs | 16 ++--- components/gfx/platform/freetype/font.rs | 29 ++++---- .../gfx/platform/freetype/font_context.rs | 4 +- components/gfx/platform/freetype/font_list.rs | 10 +-- .../gfx/platform/freetype/font_template.rs | 5 +- components/gfx/platform/macos/font_context.rs | 10 +-- .../gfx/platform/windows/font_context.rs | 9 +-- components/gfx/rendering_context.rs | 60 ++++++++--------- components/gfx/tests/font_context.rs | 8 +-- components/gfx/tests/font_template.rs | 2 +- components/gfx/text/glyph.rs | 58 ++++++++-------- components/gfx/text/shaping/harfbuzz.rs | 66 +++++++++---------- components/gfx/text/text_run.rs | 26 ++++---- components/gfx/text/util.rs | 7 +- 17 files changed, 195 insertions(+), 234 deletions(-) diff --git a/components/gfx/font.rs b/components/gfx/font.rs index 9fd25b839d0..885a89db25a 100644 --- a/components/gfx/font.rs +++ b/components/gfx/font.rs @@ -96,7 +96,7 @@ impl FontTableTagConversions for FontTableTag { (self >> 24) as u8, (self >> 16) as u8, (self >> 8) as u8, - (self >> 0) as u8, + *self as u8, ]; str::from_utf8(&bytes).unwrap().to_owned() } @@ -190,7 +190,7 @@ impl Font { let metrics = handle.metrics(); Font { - handle: handle, + handle, shaper: None, descriptor, metrics, @@ -403,7 +403,7 @@ impl FontGroup { .font_family .families .iter() - .map(|family| FontGroupFamily::new(descriptor.clone(), &family)) + .map(|family| FontGroupFamily::new(descriptor.clone(), family)) .collect(); FontGroup { @@ -419,7 +419,7 @@ impl FontGroup { /// found, returns None. pub fn find_by_codepoint( &mut self, - mut font_context: &mut FontContext, + font_context: &mut FontContext, codepoint: char, ) -> Option { let should_look_for_small_caps = self.descriptor.variant == font_variant_caps::T::SmallCaps && @@ -436,43 +436,40 @@ impl FontGroup { let has_glyph = |font: &FontRef| font.borrow().has_glyph_for(codepoint); - if let Some(font) = self.find(&mut font_context, |font| has_glyph(font)) { + if let Some(font) = self.find(font_context, has_glyph) { return font_or_synthesized_small_caps(font); } if let Some(ref last_matching_fallback) = self.last_matching_fallback { - if has_glyph(&last_matching_fallback) { + if has_glyph(last_matching_fallback) { return font_or_synthesized_small_caps(last_matching_fallback.clone()); } } - if let Some(font) = self.find_fallback(&mut font_context, Some(codepoint), has_glyph) { + if let Some(font) = self.find_fallback(font_context, Some(codepoint), has_glyph) { self.last_matching_fallback = Some(font.clone()); return font_or_synthesized_small_caps(font); } - self.first(&mut font_context) + self.first(font_context) } /// Find the first available font in the group, or the first available fallback font. - pub fn first( - &mut self, - mut font_context: &mut FontContext, - ) -> Option { - self.find(&mut font_context, |_| true) - .or_else(|| self.find_fallback(&mut font_context, None, |_| true)) + pub fn first(&mut self, font_context: &mut FontContext) -> Option { + self.find(font_context, |_| true) + .or_else(|| self.find_fallback(font_context, None, |_| true)) } /// Find a font which returns true for `predicate`. This method mutates because we may need to /// load new font data in the process of finding a suitable font. - fn find(&mut self, mut font_context: &mut FontContext, predicate: P) -> Option + fn find(&mut self, font_context: &mut FontContext, predicate: P) -> Option where S: FontSource, P: FnMut(&FontRef) -> bool, { self.families .iter_mut() - .filter_map(|family| family.font(&mut font_context)) + .filter_map(|family| family.font(font_context)) .find(predicate) } @@ -567,8 +564,7 @@ impl RunMetrics { } pub fn get_and_reset_text_shaping_performance_counter() -> usize { - let value = TEXT_SHAPING_PERFORMANCE_COUNTER.swap(0, Ordering::SeqCst); - value + TEXT_SHAPING_PERFORMANCE_COUNTER.swap(0, Ordering::SeqCst) } /// The scope within which we will look for a font. diff --git a/components/gfx/font_cache_thread.rs b/components/gfx/font_cache_thread.rs index bc8e7680417..5b954a33346 100644 --- a/components/gfx/font_cache_thread.rs +++ b/components/gfx/font_cache_thread.rs @@ -31,6 +31,7 @@ use crate::platform::font_list::{ use crate::platform::font_template::FontTemplateData; /// A list of font templates that make up a given font family. +#[derive(Default)] pub struct FontTemplates { templates: Vec, } @@ -61,10 +62,6 @@ impl SerializedFontTemplate { } impl FontTemplates { - pub fn new() -> FontTemplates { - FontTemplates { templates: vec![] } - } - /// Find a font in this family that matches a given descriptor. pub fn find_font_for_style( &mut self, @@ -214,7 +211,7 @@ impl FontCache { font_key: font_template_info.font_key, }, ))); - let _ = bytes_sender.send(&*font_template_info.font_template.bytes()); + let _ = bytes_sender.send(&font_template_info.font_template.bytes()); }, }; }, @@ -262,7 +259,7 @@ impl FontCache { }; if !self.web_families.contains_key(&family_name) { - let templates = FontTemplates::new(); + let templates = FontTemplates::default(); self.web_families.insert(family_name.clone(), templates); } @@ -298,7 +295,7 @@ impl FontCache { FetchResponseMsg::ProcessResponseChunk(new_bytes) => { trace!("@font-face {} chunk={:?}", family_name, new_bytes); if *response_valid.lock().unwrap() { - bytes.lock().unwrap().extend(new_bytes.into_iter()) + bytes.lock().unwrap().extend(new_bytes) } }, FetchResponseMsg::ProcessResponseEOF(response) => { @@ -312,7 +309,7 @@ impl FontCache { channel_to_self.send(msg).unwrap(); return; } - let bytes = mem::replace(&mut *bytes.lock().unwrap(), vec![]); + let bytes = mem::take(&mut *bytes.lock().unwrap()); trace!("@font-face {} data={:?}", family_name, bytes); let bytes = match fontsan::process(&bytes) { Ok(san) => san, @@ -365,10 +362,7 @@ impl FontCache { self.local_families.clear(); for_each_available_family(|family_name| { let family_name = LowercaseString::new(&family_name); - if !self.local_families.contains_key(&family_name) { - let templates = FontTemplates::new(); - self.local_families.insert(family_name, templates); - } + self.local_families.entry(family_name).or_default(); }); } @@ -443,7 +437,7 @@ impl FontCache { FontTemplateInfo { font_template: template, - font_key: font_key, + font_key, } } @@ -454,13 +448,13 @@ impl FontCache { ) -> Option { match family_descriptor.scope { FontSearchScope::Any => self - .find_font_in_web_family(&template_descriptor, &family_descriptor.name) + .find_font_in_web_family(template_descriptor, &family_descriptor.name) .or_else(|| { - self.find_font_in_local_family(&template_descriptor, &family_descriptor.name) + self.find_font_in_local_family(template_descriptor, &family_descriptor.name) }), FontSearchScope::Local => { - self.find_font_in_local_family(&template_descriptor, &family_descriptor.name) + self.find_font_in_local_family(template_descriptor, &family_descriptor.name) }, } .map(|t| self.get_font_template_info(t)) @@ -489,12 +483,12 @@ impl FontCacheThread { let generic_fonts = populate_generic_fonts(); let mut cache = FontCache { - port: port, + port, channel_to_self, generic_fonts, local_families: HashMap::new(), web_families: HashMap::new(), - font_context: FontContextHandle::new(), + font_context: FontContextHandle::default(), core_resource_thread, webrender_api, webrender_fonts: HashMap::new(), @@ -506,7 +500,7 @@ impl FontCacheThread { }) .expect("Thread spawning failed"); - FontCacheThread { chan: chan } + FontCacheThread { chan } } pub fn add_web_font( @@ -621,7 +615,7 @@ impl Deref for LowercaseString { #[inline] fn deref(&self) -> &str { - &*self.inner + &self.inner } } diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs index 3a577a1fbd1..bd4a80d82f3 100644 --- a/components/gfx/font_context.rs +++ b/components/gfx/font_context.rs @@ -65,7 +65,7 @@ pub struct FontContext { impl FontContext { pub fn new(font_source: S) -> FontContext { - let handle = FontContextHandle::new(); + let handle = FontContextHandle::default(); FontContext { platform_handle: handle, font_source, @@ -99,8 +99,8 @@ impl FontContext { style, }; - if let Some(ref font_group) = self.font_group_cache.get(&cache_key) { - return (*font_group).clone(); + if let Some(font_group) = self.font_group_cache.get(&cache_key) { + return font_group.clone(); } let font_group = Rc::new(RefCell::new(FontGroup::new(&cache_key.style))); @@ -150,30 +150,27 @@ impl FontContext { family_descriptor: family_descriptor.clone(), }; - self.font_cache - .get(&cache_key) - .map(|v| v.clone()) - .unwrap_or_else(|| { - debug!( - "FontContext::font cache miss for font_descriptor={:?} family_descriptor={:?}", - font_descriptor, family_descriptor - ); + self.font_cache.get(&cache_key).cloned().unwrap_or_else(|| { + debug!( + "FontContext::font cache miss for font_descriptor={:?} family_descriptor={:?}", + font_descriptor, family_descriptor + ); - let font = self - .font_template(&font_descriptor.template_descriptor, family_descriptor) - .and_then(|template_info| { - self.create_font( - template_info, - font_descriptor.to_owned(), - synthesized_small_caps_font, - ) - .ok() - }) - .map(|font| Rc::new(RefCell::new(font))); + let font = self + .font_template(&font_descriptor.template_descriptor, family_descriptor) + .and_then(|template_info| { + self.create_font( + template_info, + font_descriptor.to_owned(), + synthesized_small_caps_font, + ) + .ok() + }) + .map(|font| Rc::new(RefCell::new(font))); - self.font_cache.insert(cache_key, font.clone()); - font - }) + self.font_cache.insert(cache_key, font.clone()); + font + }) } fn font_template( @@ -182,11 +179,11 @@ impl FontContext { family_descriptor: &FontFamilyDescriptor, ) -> Option { let cache_key = FontTemplateCacheKey { - template_descriptor: template_descriptor.clone(), + template_descriptor: *template_descriptor, family_descriptor: family_descriptor.clone(), }; - self.font_template_cache.get(&cache_key).map(|v| v.clone()).unwrap_or_else(|| { + self.font_template_cache.get(&cache_key).cloned().unwrap_or_else(|| { debug!( "FontContext::font_template cache miss for template_descriptor={:?} family_descriptor={:?}", template_descriptor, @@ -194,7 +191,7 @@ impl FontContext { ); let template_info = self.font_source.font_template( - template_descriptor.clone(), + *template_descriptor, family_descriptor.clone(), ); diff --git a/components/gfx/font_template.rs b/components/gfx/font_template.rs index 3ac7026c29a..ff4e1d49824 100644 --- a/components/gfx/font_template.rs +++ b/components/gfx/font_template.rs @@ -109,18 +109,12 @@ impl FontTemplate { None => None, }; - let maybe_strong_ref = match maybe_data { - Some(data) => Some(Arc::new(data)), - None => None, - }; + let maybe_strong_ref = maybe_data.map(Arc::new); - let maybe_weak_ref = match maybe_strong_ref { - Some(ref strong_ref) => Some(Arc::downgrade(strong_ref)), - None => None, - }; + let maybe_weak_ref = maybe_strong_ref.as_ref().map(Arc::downgrade); Ok(FontTemplate { - identifier: identifier, + identifier, descriptor: None, weak_ref: maybe_weak_ref, strong_ref: maybe_strong_ref, @@ -161,7 +155,7 @@ impl FontTemplate { fctx: &FontContextHandle, requested_desc: &FontTemplateDescriptor, ) -> Option> { - self.descriptor(&fctx).and_then(|descriptor| { + self.descriptor(fctx).and_then(|descriptor| { if *requested_desc == descriptor { self.data().ok() } else { @@ -177,7 +171,7 @@ impl FontTemplate { font_context: &FontContextHandle, requested_descriptor: &FontTemplateDescriptor, ) -> Option<(Arc, f32)> { - self.descriptor(&font_context).and_then(|descriptor| { + self.descriptor(font_context).and_then(|descriptor| { self.data() .ok() .map(|data| (data, descriptor.distance_from(requested_descriptor))) diff --git a/components/gfx/platform/freetype/font.rs b/components/gfx/platform/freetype/font.rs index a0de645a544..39cd4958d5f 100644 --- a/components/gfx/platform/freetype/font.rs +++ b/components/gfx/platform/freetype/font.rs @@ -147,7 +147,7 @@ impl FontHandleMethods for FontHandle { let face = create_face(ft_ctx, &template, pt_size)?; let mut handle = FontHandle { - face: face, + face, font_data: template, context_handle: fctx.clone(), can_do_fast_shaping: false, @@ -264,7 +264,7 @@ impl FontHandleMethods for FontHandle { let res = FT_Load_Glyph(self.face, glyph as FT_UInt, GLYPH_LOAD_FLAGS); if succeeded(res) { let void_glyph = (*self.face).glyph; - let slot: FT_GlyphSlot = mem::transmute(void_glyph); + let slot: FT_GlyphSlot = void_glyph; assert!(!slot.is_null()); let advance = (*slot).metrics.horiAdvance; debug!("h_advance for {} is {}", glyph, advance); @@ -313,17 +313,17 @@ impl FontHandleMethods for FontHandle { .map_or(max_advance, |advance| self.font_units_to_au(advance)); let metrics = FontMetrics { - underline_size: underline_size, - underline_offset: underline_offset, - strikeout_size: strikeout_size, - strikeout_offset: strikeout_offset, - leading: leading, - x_height: x_height, - em_size: em_size, - ascent: ascent, + underline_size, + underline_offset, + strikeout_size, + strikeout_offset, + leading, + x_height, + em_size, + ascent, descent: -descent, // linux font's seem to use the opposite sign from mac - max_advance: max_advance, - average_advance: average_advance, + max_advance, + average_advance, line_gap: height, }; @@ -392,6 +392,7 @@ impl<'a> FontHandle { } } + #[allow(clippy::mut_from_ref)] // Intended for this function fn face_rec_mut(&'a self) -> &'a mut FT_FaceRec { unsafe { &mut (*self.face) } } @@ -402,10 +403,10 @@ impl<'a> FontHandle { // face.size is a *c_void in the bindings, presumably to avoid // recursive structural types let size: &FT_SizeRec = unsafe { mem::transmute(&(*face.size)) }; - let metrics: &FT_Size_Metrics = &(*size).metrics; + let metrics: &FT_Size_Metrics = &(size).metrics; let em_size = face.units_per_EM as f64; - let x_scale = (metrics.x_ppem as f64) / em_size as f64; + let x_scale = (metrics.x_ppem as f64) / em_size; // If this isn't true then we're scaling one of the axes wrong assert_eq!(metrics.x_ppem, metrics.y_ppem); diff --git a/components/gfx/platform/freetype/font_context.rs b/components/gfx/platform/freetype/font_context.rs index c5df0598563..b08ff552261 100644 --- a/components/gfx/platform/freetype/font_context.rs +++ b/components/gfx/platform/freetype/font_context.rs @@ -109,8 +109,8 @@ impl MallocSizeOf for FontContextHandle { } } -impl FontContextHandle { - pub fn new() -> FontContextHandle { +impl Default for FontContextHandle { + fn default() -> Self { let user = Box::into_raw(Box::new(User { size: 0 })); let mem = Box::into_raw(Box::new(FT_MemoryRec_ { user: user as *mut c_void, diff --git a/components/gfx/platform/freetype/font_list.rs b/components/gfx/platform/freetype/font_list.rs index a5363fc4848..e9e9ad7a531 100644 --- a/components/gfx/platform/freetype/font_list.rs +++ b/components/gfx/platform/freetype/font_list.rs @@ -17,10 +17,10 @@ use log::debug; use super::c_str_to_string; use crate::text::util::is_cjk; -static FC_FAMILY: &'static [u8] = b"family\0"; -static FC_FILE: &'static [u8] = b"file\0"; -static FC_INDEX: &'static [u8] = b"index\0"; -static FC_FONTFORMAT: &'static [u8] = b"fontformat\0"; +static FC_FAMILY: &[u8] = b"family\0"; +static FC_FILE: &[u8] = b"file\0"; +static FC_INDEX: &[u8] = b"index\0"; +static FC_FONTFORMAT: &[u8] = b"fontformat\0"; pub fn for_each_available_family(mut callback: F) where @@ -150,7 +150,7 @@ pub fn system_default_family(generic_name: &str) -> Option { } } -pub static SANS_SERIF_FONT_FAMILY: &'static str = "DejaVu Sans"; +pub static SANS_SERIF_FONT_FAMILY: &str = "DejaVu Sans"; // Based on gfxPlatformGtk::GetCommonFallbackFonts() in Gecko pub fn fallback_font_families(codepoint: Option) -> Vec<&'static str> { diff --git a/components/gfx/platform/freetype/font_template.rs b/components/gfx/platform/freetype/font_template.rs index cb3a1ef3857..b65bb8f4b04 100644 --- a/components/gfx/platform/freetype/font_template.rs +++ b/components/gfx/platform/freetype/font_template.rs @@ -36,10 +36,7 @@ impl fmt::Debug for FontTemplateData { impl FontTemplateData { pub fn new(identifier: Atom, bytes: Option>) -> Result { - Ok(FontTemplateData { - bytes: bytes, - identifier: identifier, - }) + Ok(FontTemplateData { bytes, identifier }) } /// Returns a clone of the data in this font. This may be a hugely expensive diff --git a/components/gfx/platform/macos/font_context.rs b/components/gfx/platform/macos/font_context.rs index 753cb97a572..b296ce6738b 100644 --- a/components/gfx/platform/macos/font_context.rs +++ b/components/gfx/platform/macos/font_context.rs @@ -4,16 +4,10 @@ use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct FontContextHandle { - _ctx: (), -} - -impl FontContextHandle { // this is a placeholder until NSFontManager or whatever is bound in here. - pub fn new() -> FontContextHandle { - FontContextHandle { _ctx: () } - } + _ctx: (), } impl MallocSizeOf for FontContextHandle { diff --git a/components/gfx/platform/windows/font_context.rs b/components/gfx/platform/windows/font_context.rs index d5e0015dd56..a67a366fc11 100644 --- a/components/gfx/platform/windows/font_context.rs +++ b/components/gfx/platform/windows/font_context.rs @@ -4,14 +4,7 @@ use malloc_size_of::malloc_size_of_is_0; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct FontContextHandle; -impl FontContextHandle { - // *shrug* - pub fn new() -> FontContextHandle { - FontContextHandle {} - } -} - malloc_size_of_is_0!(FontContextHandle); diff --git a/components/gfx/rendering_context.rs b/components/gfx/rendering_context.rs index 4e78ba9117c..9f3f733efd3 100644 --- a/components/gfx/rendering_context.rs +++ b/components/gfx/rendering_context.rs @@ -30,8 +30,8 @@ struct RenderingContextData { impl Drop for RenderingContextData { fn drop(&mut self) { - let ref mut device = self.device.borrow_mut(); - let ref mut context = self.context.borrow_mut(); + let device = &mut self.device.borrow_mut(); + let context = &mut self.context.borrow_mut(); if let Some(ref swap_chain) = self.swap_chain { let _ = swap_chain.destroy(device, context); } @@ -45,7 +45,7 @@ impl RenderingContext { adapter: &Adapter, surface_type: SurfaceType, ) -> Result { - let mut device = connection.create_device(&adapter)?; + let mut device = connection.create_device(adapter)?; let flags = ContextAttributeFlags::ALPHA | ContextAttributeFlags::DEPTH | ContextAttributeFlags::STENCIL; @@ -94,8 +94,8 @@ impl RenderingContext { &self, surface: Surface, ) -> Result { - let ref device = self.0.device.borrow(); - let ref mut context = self.0.context.borrow_mut(); + let device = &self.0.device.borrow(); + let context = &mut self.0.context.borrow_mut(); device.create_surface_texture(context, surface) } @@ -103,14 +103,14 @@ impl RenderingContext { &self, surface_texture: SurfaceTexture, ) -> Result { - let ref device = self.0.device.borrow(); - let ref mut context = self.0.context.borrow_mut(); + let device = &self.0.device.borrow(); + let context = &mut self.0.context.borrow_mut(); device.destroy_surface_texture(context, surface_texture) } pub fn make_gl_context_current(&self) -> Result<(), Error> { - let ref device = self.0.device.borrow(); - let ref context = self.0.context.borrow(); + let device = &self.0.device.borrow(); + let context = &self.0.context.borrow(); device.make_context_current(context) } @@ -119,8 +119,8 @@ impl RenderingContext { } pub fn resize(&self, size: Size2D) -> Result<(), Error> { - let ref mut device = self.0.device.borrow_mut(); - let ref mut context = self.0.context.borrow_mut(); + let device = &mut self.0.device.borrow_mut(); + let context = &mut self.0.context.borrow_mut(); if let Some(swap_chain) = self.0.swap_chain.as_ref() { return swap_chain.resize(device, context, size); } @@ -135,8 +135,8 @@ impl RenderingContext { } pub fn present(&self) -> Result<(), Error> { - let ref mut device = self.0.device.borrow_mut(); - let ref mut context = self.0.context.borrow_mut(); + let device = &mut self.0.device.borrow_mut(); + let context = &mut self.0.context.borrow_mut(); if let Some(ref swap_chain) = self.0.swap_chain { return swap_chain.swap_buffers(device, context, PreserveBuffer::No); } @@ -153,8 +153,8 @@ impl RenderingContext { /// Invoke a closure with the surface associated with the current front buffer. /// This can be used to create a surfman::SurfaceTexture to blit elsewhere. pub fn with_front_buffer Surface>(&self, mut f: F) { - let ref mut device = self.0.device.borrow_mut(); - let ref mut context = self.0.context.borrow_mut(); + let device = &mut self.0.device.borrow_mut(); + let context = &mut self.0.context.borrow_mut(); let surface = device .unbind_surface_from_context(context) .unwrap() @@ -168,52 +168,52 @@ impl RenderingContext { } pub fn connection(&self) -> Connection { - let ref device = self.0.device.borrow(); + let device = &self.0.device.borrow(); device.connection() } pub fn adapter(&self) -> Adapter { - let ref device = self.0.device.borrow(); + let device = &self.0.device.borrow(); device.adapter() } pub fn native_context(&self) -> NativeContext { - let ref device = self.0.device.borrow(); - let ref context = self.0.context.borrow(); + let device = &self.0.device.borrow(); + let context = &self.0.context.borrow(); device.native_context(context) } pub fn native_device(&self) -> NativeDevice { - let ref device = self.0.device.borrow(); + let device = &self.0.device.borrow(); device.native_device() } pub fn context_attributes(&self) -> ContextAttributes { - let ref device = self.0.device.borrow(); - let ref context = self.0.context.borrow(); - let ref descriptor = device.context_descriptor(context); + let device = &self.0.device.borrow(); + let context = &self.0.context.borrow(); + let descriptor = &device.context_descriptor(context); device.context_descriptor_attributes(descriptor) } pub fn context_surface_info(&self) -> Result, Error> { - let ref device = self.0.device.borrow(); - let ref context = self.0.context.borrow(); + let device = &self.0.device.borrow(); + let context = &self.0.context.borrow(); device.context_surface_info(context) } pub fn surface_info(&self, surface: &Surface) -> SurfaceInfo { - let ref device = self.0.device.borrow(); + let device = &self.0.device.borrow(); device.surface_info(surface) } pub fn surface_texture_object(&self, surface: &SurfaceTexture) -> u32 { - let ref device = self.0.device.borrow(); + let device = &self.0.device.borrow(); device.surface_texture_object(surface) } pub fn get_proc_address(&self, name: &str) -> *const c_void { - let ref device = self.0.device.borrow(); - let ref context = self.0.context.borrow(); + let device = &self.0.device.borrow(); + let context = &self.0.context.borrow(); device.get_proc_address(context, name) } @@ -221,7 +221,7 @@ impl RenderingContext { let device = self.0.device.borrow_mut(); let mut context = self.0.context.borrow_mut(); let mut surface = device.unbind_surface_from_context(&mut context)?.unwrap(); - let _ = device.destroy_surface(&mut context, &mut surface)?; + device.destroy_surface(&mut context, &mut surface)?; Ok(()) } diff --git a/components/gfx/tests/font_context.rs b/components/gfx/tests/font_context.rs index 6ea58172c27..10a193272ec 100644 --- a/components/gfx/tests/font_context.rs +++ b/components/gfx/tests/font_context.rs @@ -34,13 +34,13 @@ struct TestFontSource { impl TestFontSource { fn new() -> TestFontSource { - let mut csstest_ascii = FontTemplates::new(); + let mut csstest_ascii = FontTemplates::default(); Self::add_face(&mut csstest_ascii, "csstest-ascii", None); - let mut csstest_basic = FontTemplates::new(); + let mut csstest_basic = FontTemplates::default(); Self::add_face(&mut csstest_basic, "csstest-basic-regular", None); - let mut fallback = FontTemplates::new(); + let mut fallback = FontTemplates::default(); Self::add_face(&mut fallback, "csstest-basic-regular", Some("fallback")); let mut families = HashMap::new(); @@ -49,7 +49,7 @@ impl TestFontSource { families.insert(fallback_font_families(None)[0].to_owned(), fallback); TestFontSource { - handle: FontContextHandle::new(), + handle: FontContextHandle::default(), families, find_font_count: Rc::new(Cell::new(0)), } diff --git a/components/gfx/tests/font_template.rs b/components/gfx/tests/font_template.rs index eecaacbd695..d3958ce4a2e 100644 --- a/components/gfx/tests/font_template.rs +++ b/components/gfx/tests/font_template.rs @@ -35,7 +35,7 @@ fn test_font_template_descriptor() { ) .unwrap(); - let context = FontContextHandle::new(); + let context = FontContextHandle::default(); template.descriptor(&context).unwrap() } diff --git a/components/gfx/text/glyph.rs b/components/gfx/text/glyph.rs index 98dc39bc0e4..976fe012336 100644 --- a/components/gfx/text/glyph.rs +++ b/components/gfx/text/glyph.rs @@ -28,7 +28,7 @@ pub struct GlyphEntry { impl GlyphEntry { fn new(value: u32) -> GlyphEntry { - GlyphEntry { value: value } + GlyphEntry { value } } fn initial() -> GlyphEntry { @@ -40,7 +40,7 @@ impl GlyphEntry { assert!(is_simple_glyph_id(id)); assert!(is_simple_advance(advance)); - let id_mask = id as u32; + let id_mask = id; let Au(advance) = advance; let advance_mask = (advance as u32) << GLYPH_ADVANCE_SHIFT; @@ -89,7 +89,7 @@ const GLYPH_ID_MASK: u32 = 0x0000FFFF; const GLYPH_COUNT_MASK: u32 = 0x0000FFFF; fn is_simple_glyph_id(id: GlyphId) -> bool { - ((id as u32) & GLYPH_ID_MASK) == id + (id & GLYPH_ID_MASK) == id } fn is_simple_advance(advance: Au) -> bool { @@ -157,9 +157,9 @@ struct DetailedGlyph { impl DetailedGlyph { fn new(id: GlyphId, advance: Au, offset: Point2D) -> DetailedGlyph { DetailedGlyph { - id: id, - advance: advance, - offset: offset, + id, + advance, + offset, } } } @@ -172,18 +172,18 @@ struct DetailedGlyphRecord { detail_offset: usize, } -impl PartialOrd for DetailedGlyphRecord { - fn partial_cmp(&self, other: &DetailedGlyphRecord) -> Option { - self.entry_offset.partial_cmp(&other.entry_offset) - } -} - impl Ord for DetailedGlyphRecord { fn cmp(&self, other: &DetailedGlyphRecord) -> Ordering { self.entry_offset.cmp(&other.entry_offset) } } +impl PartialOrd for DetailedGlyphRecord { + fn partial_cmp(&self, other: &DetailedGlyphRecord) -> Option { + Some(self.cmp(other)) + } +} + // Manages the lookup table for detailed glyphs. Sorting is deferred // until a lookup is actually performed; this matches the expected // usage pattern of setting/appending all the detailed glyphs, and @@ -210,7 +210,7 @@ impl<'a> DetailedGlyphStore { fn add_detailed_glyphs_for_entry(&mut self, entry_offset: ByteIndex, glyphs: &[DetailedGlyph]) { let entry = DetailedGlyphRecord { - entry_offset: entry_offset, + entry_offset, detail_offset: self.detail_buffer.len(), }; @@ -245,7 +245,7 @@ impl<'a> DetailedGlyphStore { assert!(self.lookup_is_sorted); let key = DetailedGlyphRecord { - entry_offset: entry_offset, + entry_offset, detail_offset: 0, // unused }; @@ -268,7 +268,7 @@ impl<'a> DetailedGlyphStore { assert!(self.lookup_is_sorted); let key = DetailedGlyphRecord { - entry_offset: entry_offset, + entry_offset, detail_offset: 0, // unused }; @@ -329,11 +329,11 @@ impl GlyphData { ligature_start: bool, ) -> GlyphData { GlyphData { - id: id, - advance: advance, + id, + advance, offset: offset.unwrap_or(Point2D::zero()), - cluster_start: cluster_start, - ligature_start: ligature_start, + cluster_start, + ligature_start, } } } @@ -455,8 +455,8 @@ impl<'a> GlyphStore { total_advance: Au(0), total_word_separators: 0, has_detailed_glyphs: false, - is_whitespace: is_whitespace, - is_rtl: is_rtl, + is_whitespace, + is_rtl, } } @@ -490,7 +490,7 @@ impl<'a> GlyphStore { let mut total_advance = Au(0); let mut total_word_separators = 0; for glyph in self.iter_glyphs_for_byte_range(&Range::new(ByteIndex(0), self.len())) { - total_advance = total_advance + glyph.advance(); + total_advance += glyph.advance(); if glyph.char_is_word_separator() { total_word_separators += 1; } @@ -539,7 +539,7 @@ impl<'a> GlyphStore { pub fn add_glyphs_for_byte_index(&mut self, i: ByteIndex, data_for_glyphs: &[GlyphData]) { assert!(i < self.len()); - assert!(data_for_glyphs.len() > 0); + assert!(!data_for_glyphs.is_empty()); let glyph_count = data_for_glyphs.len(); @@ -623,8 +623,6 @@ impl<'a> GlyphStore { pub fn advance_for_byte_range(&self, range: &Range, extra_word_spacing: Au) -> Au { if range.begin() == ByteIndex(0) && range.end() == self.len() { self.total_advance + extra_word_spacing * (self.total_word_separators as i32) - } else if !self.has_detailed_glyphs { - self.advance_for_byte_range_simple_glyphs(range, extra_word_spacing) } else { self.advance_for_byte_range_simple_glyphs(range, extra_word_spacing) } @@ -664,13 +662,13 @@ impl<'a> GlyphStore { impl fmt::Debug for GlyphStore { fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "GlyphStore:\n")?; + writeln!(formatter, "GlyphStore:")?; let mut detailed_buffer = self.detail_store.detail_buffer.iter(); for entry in self.entry_buffer.iter() { if entry.is_simple() { - write!( + writeln!( formatter, - " simple id={:?} advance={:?}\n", + " simple id={:?} advance={:?}", entry.id(), entry.advance() )?; @@ -683,9 +681,9 @@ impl fmt::Debug for GlyphStore { if detailed_buffer.next().is_none() { continue; } - write!( + writeln!( formatter, - " detailed id={:?} advance={:?}\n", + " detailed id={:?} advance={:?}", entry.id(), entry.advance() )?; diff --git a/components/gfx/text/shaping/harfbuzz.rs b/components/gfx/text/shaping/harfbuzz.rs index 389a44b5457..96d5ccc3251 100644 --- a/components/gfx/text/shaping/harfbuzz.rs +++ b/components/gfx/text/shaping/harfbuzz.rs @@ -47,21 +47,19 @@ pub struct ShapedGlyphEntry { } impl ShapedGlyphData { - pub fn new(buffer: *mut hb_buffer_t) -> ShapedGlyphData { - unsafe { - let mut glyph_count = 0; - let glyph_infos = hb_buffer_get_glyph_infos(buffer, &mut glyph_count); - assert!(!glyph_infos.is_null()); - let mut pos_count = 0; - let pos_infos = hb_buffer_get_glyph_positions(buffer, &mut pos_count); - assert!(!pos_infos.is_null()); - assert_eq!(glyph_count, pos_count); + pub unsafe fn new(buffer: *mut hb_buffer_t) -> ShapedGlyphData { + let mut glyph_count = 0; + let glyph_infos = hb_buffer_get_glyph_infos(buffer, &mut glyph_count); + assert!(!glyph_infos.is_null()); + let mut pos_count = 0; + let pos_infos = hb_buffer_get_glyph_positions(buffer, &mut pos_count); + assert!(!pos_infos.is_null()); + assert_eq!(glyph_count, pos_count); - ShapedGlyphData { - count: glyph_count as usize, - glyph_infos: glyph_infos, - pos_infos: pos_infos, - } + ShapedGlyphData { + count: glyph_count as usize, + glyph_infos, + pos_infos, } } @@ -70,7 +68,7 @@ impl ShapedGlyphData { assert!(i < self.count); unsafe { - let glyph_info_i = self.glyph_infos.offset(i as isize); + let glyph_info_i = self.glyph_infos.add(i); (*glyph_info_i).cluster } } @@ -79,13 +77,17 @@ impl ShapedGlyphData { self.count } + pub fn is_empty(&self) -> bool { + self.count == 0 + } + /// Returns shaped glyph data for one glyph, and updates the y-position of the pen. pub fn entry_for_glyph(&self, i: usize, y_pos: &mut Au) -> ShapedGlyphEntry { assert!(i < self.count); unsafe { - let glyph_info_i = self.glyph_infos.offset(i as isize); - let pos_info_i = self.pos_infos.offset(i as isize); + let glyph_info_i = self.glyph_infos.add(i); + let pos_info_i = self.pos_infos.add(i); let x_offset = Shaper::fixed_to_float((*pos_info_i).x_offset); let y_offset = Shaper::fixed_to_float((*pos_info_i).y_offset); let x_advance = Shaper::fixed_to_float((*pos_info_i).x_advance); @@ -101,7 +103,7 @@ impl ShapedGlyphData { } else { // adjust the pen.. if y_advance > Au(0) { - *y_pos = *y_pos - y_advance; + *y_pos -= y_advance; } Some(Point2D::new(x_offset, *y_pos - y_offset)) @@ -110,7 +112,7 @@ impl ShapedGlyphData { ShapedGlyphEntry { codepoint: (*glyph_info_i).codepoint as GlyphId, advance: x_advance, - offset: offset, + offset, } } } @@ -136,6 +138,7 @@ impl Drop for Shaper { } impl Shaper { + #[allow(clippy::not_unsafe_ptr_arg_deref)] // Has an unsafe block inside pub fn new(font: *const Font) -> Shaper { unsafe { let hb_face: *mut hb_face_t = hb_face_create_for_tables( @@ -165,9 +168,9 @@ impl Shaper { ); Shaper { - hb_face: hb_face, - hb_font: hb_font, - font: font, + hb_face, + hb_font, + font, } } } @@ -415,7 +418,7 @@ impl Shaper { glyphs: &mut GlyphStore, buffer: *mut hb_buffer_t, ) { - let glyph_data = ShapedGlyphData::new(buffer); + let glyph_data = unsafe { ShapedGlyphData::new(buffer) }; let glyph_count = glyph_data.len(); let byte_max = text.len(); @@ -485,7 +488,7 @@ impl Shaper { } // if no glyphs were found yet, extend the char byte range more. - if glyph_span.len() == 0 { + if glyph_span.is_empty() { continue; } @@ -508,8 +511,8 @@ impl Shaper { // span or reach the end of the text. } - assert!(byte_range.len() > 0); - assert!(glyph_span.len() > 0); + assert!(!byte_range.is_empty()); + assert!(!glyph_span.is_empty()); // Now byte_range is the ligature clump formed by the glyphs in glyph_span. // We will save these glyphs to the glyph store at the index of the first byte. @@ -580,7 +583,7 @@ impl Shaper { options: &ShapingOptions, ) -> Au { if let Some(letter_spacing) = options.letter_spacing { - advance = advance + letter_spacing; + advance += letter_spacing; }; // CSS 2.1 ยง 16.4 states that "word spacing affects each space (U+0020) and non-breaking @@ -654,13 +657,10 @@ extern "C" fn glyph_h_advance_func( fn glyph_space_advance(font: *const Font) -> (hb_codepoint_t, f64) { let space_unicode = ' '; - let space_glyph: hb_codepoint_t; - match unsafe { (*font).glyph_index(space_unicode) } { - Some(g) => { - space_glyph = g as hb_codepoint_t; - }, + let space_glyph: hb_codepoint_t = match unsafe { (*font).glyph_index(space_unicode) } { + Some(g) => g as hb_codepoint_t, None => panic!("No space info"), - } + }; let space_advance = unsafe { (*font).glyph_h_advance(space_glyph as GlyphId) }; (space_glyph, space_advance) } diff --git a/components/gfx/text/text_run.rs b/components/gfx/text/text_run.rs index 011ba784107..21c239257fb 100644 --- a/components/gfx/text/text_run.rs +++ b/components/gfx/text/text_run.rs @@ -129,7 +129,7 @@ impl<'a> Iterator for NaturalWordSliceIterator<'a> { if !byte_range.is_empty() { Some(TextRunSlice { - glyphs: &*slice_glyphs.glyph_store, + glyphs: &slice_glyphs.glyph_store, offset: slice_range_begin, range: byte_range, }) @@ -172,7 +172,7 @@ impl<'a> Iterator for CharacterSliceIterator<'a> { let index_within_glyph_run = byte_start - glyph_run.range.begin(); Some(TextRunSlice { - glyphs: &*glyph_run.glyph_store, + glyphs: &glyph_run.glyph_store, offset: glyph_run.range.begin(), range: Range::new(index_within_glyph_run, byte_len), }) @@ -197,7 +197,7 @@ impl<'a> TextRun { font_key: font.font_key, pt_size: font.descriptor.pt_size, glyphs: Arc::new(glyphs), - bidi_level: bidi_level, + bidi_level, extra_word_spacing: Au(0), }, break_at_zero, @@ -217,7 +217,7 @@ impl<'a> TextRun { let mut break_at_zero = false; if breaker.is_none() { - if text.len() == 0 { + if text.is_empty() { return (glyphs, true); } *breaker = Some(LineBreakLeafIter::new(text, 0)); @@ -253,7 +253,7 @@ impl<'a> TextRun { // keep-all, try increasing the slice. continue; } - if slice.len() > 0 { + if !slice.is_empty() { glyphs.push(GlyphRun { glyph_store: font.shape_text(&text[slice.clone()], options), range: Range::new( @@ -262,8 +262,8 @@ impl<'a> TextRun { ), }); } - if whitespace.len() > 0 { - let mut options = options.clone(); + if !whitespace.is_empty() { + let mut options = *options; options .flags .insert(ShapingFlags::IS_WHITESPACE_SHAPING_FLAG); @@ -348,7 +348,9 @@ impl<'a> TextRun { } } - if let Ok(result) = (&**self.glyphs).binary_search_by(|current| current.compare(&index)) + if let Ok(result) = self + .glyphs + .binary_search_by(|current| current.compare(&index)) { index_of_first_glyph_run_cache.set(Some((self_ptr, index, result))); Some(result) @@ -396,7 +398,7 @@ impl<'a> TextRun { }; NaturalWordSliceIterator { glyphs: &self.glyphs[..], - index: index, + index, range: *range, reverse: false, } @@ -424,9 +426,9 @@ impl<'a> TextRun { }; NaturalWordSliceIterator { glyphs: &self.glyphs[..], - index: index, + index, range: *range, - reverse: reverse, + reverse, } } @@ -445,7 +447,7 @@ impl<'a> TextRun { CharacterSliceIterator { text: &self.text, glyph_run: first_glyph_run, - glyph_run_iter: glyph_run_iter, + glyph_run_iter, range: *range, } } diff --git a/components/gfx/text/util.rs b/components/gfx/text/util.rs index a2644b8b302..be9ba144dca 100644 --- a/components/gfx/text/util.rs +++ b/components/gfx/text/util.rs @@ -113,12 +113,7 @@ pub fn fixed_to_float(before: usize, f: i32) -> f64 { } pub fn is_bidi_control(c: char) -> bool { - match c { - '\u{202A}'..='\u{202E}' => true, - '\u{2066}'..='\u{2069}' => true, - '\u{200E}' | '\u{200F}' | '\u{061C}' => true, - _ => false, - } + matches!(c, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}' | '\u{200E}' | '\u{200F}' | '\u{061C}') } pub fn unicode_plane(codepoint: char) -> u32 {