From 51e69d7040c943aad8453bc03031c75b4cb302bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ram=C3=B3n=20Jim=C3=A9nez?= Date: Fri, 1 Sep 2023 04:04:15 +0200 Subject: [PATCH] Replace `MaybeUninit` with `Option` in `paragraph` --- graphics/src/text/paragraph.rs | 72 +++++++++++++--------------------- 1 file changed, 28 insertions(+), 44 deletions(-) diff --git a/graphics/src/text/paragraph.rs b/graphics/src/text/paragraph.rs index c6921005b3..8e8907f98a 100644 --- a/graphics/src/text/paragraph.rs +++ b/graphics/src/text/paragraph.rs @@ -5,10 +5,10 @@ use crate::core::{Font, Pixels, Point, Size}; use crate::text::{self, FontSystem}; use std::fmt; -use std::mem::MaybeUninit; use std::sync::{self, Arc}; -pub struct Paragraph(MaybeUninit>); +#[derive(Clone, PartialEq, Default)] +pub struct Paragraph(Option>); struct Internal { buffer: cosmic_text::Buffer, @@ -21,12 +21,6 @@ struct Internal { min_bounds: Size, } -impl Default for Paragraph { - fn default() -> Self { - Self(MaybeUninit::new(Arc::new(Internal::default()))) - } -} - impl Paragraph { pub fn new() -> Self { Self::default() @@ -58,7 +52,7 @@ impl Paragraph { let min_bounds = text::measure(&buffer); - Self(MaybeUninit::new(Arc::new(Internal { + Self(Some(Arc::new(Internal { buffer, content: text.content.to_owned(), font: text.font, @@ -71,13 +65,11 @@ impl Paragraph { } pub fn buffer(&self) -> &cosmic_text::Buffer { - #[allow(unsafe_code)] - &unsafe { self.0.assume_init_ref() }.buffer + &self.internal().buffer } pub fn downgrade(&self) -> Weak { - #[allow(unsafe_code)] - let paragraph = unsafe { self.0.assume_init_ref() }; + let paragraph = self.internal(); Weak { raw: Arc::downgrade(paragraph), @@ -88,13 +80,10 @@ impl Paragraph { } pub fn resize(&mut self, new_bounds: Size, font_system: &FontSystem) { - // Place uninit for now, we always write to `self.0` in the end - let paragraph = std::mem::replace(&mut self.0, MaybeUninit::uninit()); - - // Mutable self guarantees unique access and `uninit` call only happens - // in this method. - #[allow(unsafe_code)] - let paragraph = unsafe { paragraph.assume_init() }; + let paragraph = self + .0 + .take() + .expect("paragraph should always be initialized"); match Arc::try_unwrap(paragraph) { Ok(mut internal) => { @@ -107,14 +96,14 @@ impl Paragraph { internal.bounds = new_bounds; internal.min_bounds = text::measure(&internal.buffer); - let _ = self.0.write(Arc::new(internal)); + self.0 = Some(Arc::new(internal)); } Err(internal) => { let metrics = internal.buffer.metrics(); // If there is a strong reference somewhere, we recompute the // buffer from scratch - let new_paragraph = Self::with_text( + *self = Self::with_text( Text { content: &internal.content, bounds: internal.bounds, @@ -129,19 +118,14 @@ impl Paragraph { }, font_system, ); - - // New paragraph should always be initialized - #[allow(unsafe_code)] - let _ = self.0.write(unsafe { new_paragraph.0.assume_init() }); } } } - fn internal_ref(&self) -> &Internal { - #[allow(unsafe_code)] - unsafe { - self.0.assume_init_ref() - } + fn internal(&self) -> &Arc { + self.0 + .as_ref() + .expect("paragraph should always be initialized") } } @@ -149,51 +133,51 @@ impl core::text::Paragraph for Paragraph { type Font = Font; fn content(&self) -> &str { - &self.internal_ref().content + &self.internal().content } fn text_size(&self) -> Pixels { - Pixels(self.internal_ref().buffer.metrics().font_size) + Pixels(self.internal().buffer.metrics().font_size) } fn line_height(&self) -> LineHeight { LineHeight::Absolute(Pixels( - self.internal_ref().buffer.metrics().line_height, + self.internal().buffer.metrics().line_height, )) } fn font(&self) -> Font { - self.internal_ref().font + self.internal().font } fn shaping(&self) -> Shaping { - self.internal_ref().shaping + self.internal().shaping } fn horizontal_alignment(&self) -> alignment::Horizontal { - self.internal_ref().horizontal_alignment + self.internal().horizontal_alignment } fn vertical_alignment(&self) -> alignment::Vertical { - self.internal_ref().vertical_alignment + self.internal().vertical_alignment } fn bounds(&self) -> Size { - self.internal_ref().bounds + self.internal().bounds } fn min_bounds(&self) -> Size { - self.internal_ref().min_bounds + self.internal().min_bounds } fn hit_test(&self, point: Point) -> Option { - let cursor = self.internal_ref().buffer.hit(point.x, point.y)?; + let cursor = self.internal().buffer.hit(point.x, point.y)?; Some(Hit::CharOffset(cursor.index)) } fn grapheme_position(&self, line: usize, index: usize) -> Option { - let run = self.internal_ref().buffer.layout_runs().nth(line)?; + let run = self.internal().buffer.layout_runs().nth(line)?; // TODO: Index represents a grapheme, not a glyph let glyph = run.glyphs.get(index).or_else(|| run.glyphs.last())?; @@ -244,7 +228,7 @@ impl Default for Internal { impl fmt::Debug for Paragraph { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let paragraph = self.internal_ref(); + let paragraph = self.internal(); f.debug_struct("Paragraph") .field("content", ¶graph.content) @@ -268,7 +252,7 @@ pub struct Weak { impl Weak { pub fn upgrade(&self) -> Option { - self.raw.upgrade().map(MaybeUninit::new).map(Paragraph) + self.raw.upgrade().map(Some).map(Paragraph) } }