Fix bugs related to text and padding.

CHANGELOG.md

@@ -25,26 +25,33 @@ Please only add new entries below the [Unreleased](#unreleased---releasedate) he
 
 ## [@Unreleased] - @ReleaseDate
 
+### Features
+
+- **core**: Added `Measure` to enable support for percentage values for position, and `Anchor` now supports percentage values. (#672 @M-Adoo)
+- **core**: Add APIs `AppCtx::once_next_frame`, `Window::once_next_frame`, `Window::once_frame_finished` and `Window::once_before_layout`. (#672 @M-Adoo)
+- **painter**: Typography now supports baselines (middle and alphabetic). (#674 @M-Adoo)
+
 ### Fixed
 
-- **core**: fix set opacity zero no work to it's children. (#671 @wjian23)
+- **core**: Fix set opacity zero no work to it's children. (#671 @wjian23)
 - **core**: Fix TextStyle cause providers mismatched (#671 @wjian23)
+- **core**: Running an animation that is already in progress does not trigger a smooth transition. (#672 @M-Adoo)
+- **core**: The framework incorrectly clamps the layout result of the render widget. (#672 @M-Adoo)
+- **core**: Padding will not change the child's size; otherwise, child elements like `Icon` may not work correctly. (#674 @M-Adoo)
+- **painter**: Fixed text line height does not work correctly. (#674 @M-Adoo)
+- **painter**: Fixed issue with text not being drawn at the middle baseline by default. (#674 @M-Adoo)
 
 ## [0.4.0-alpha.19] - 2024-12-18
 
 ### Features
 
 - **core**: Added `grab_pointer` to grabs all the pointer input. (#669 @wjian23)
-- **core**: Added `Measure` to enable support for percentage values for position, and `Anchor` now supports percentage values. (#672 @M-Adoo)
-- **core**: Add APIs `AppCtx::once_next_frame`, `Window::once_next_frame`, `Window::once_frame_finished` and `Window::once_before_layout`. (#672 @M-Adoo)
 - **widgets**: Added the widget of Slider (#669 @wjian23)
 
 ### Fixed
 
 - **core**: Fix mismatch of providers. (#669 @wjian23)
 - **core**: Added DeclarerWithSubscription to let Widget `Expanded` accept pipe value. (#669 @wjian23)
-- **core**: Running an animation that is already in progress does not trigger a smooth transition. (#672 @M-Adoo)
-- **core**: The framework incorrectly clamps the layout result of the render widget. (#672 @M-Adoo)
 
 ## [0.4.0-alpha.18] - 2024-12-11
 

core/src/builtin_widgets/margin.rs

@@ -8,7 +8,28 @@ pub struct EdgeInsets {
   pub top: f32,
 }
 
-/// A widget that create space around its child.
+/// The widget utilizes empty space to surround the child widget.
+///
+/// Both `Padding` and `Margin` utilize empty space to surround the child
+/// widget. The difference lies in the fact that when used as a built-in widget,
+/// padding makes the empty space appear as a part of the child widget, whereas
+/// margin does not. For example:
+///
+/// ```
+/// use ribir::prelude::*;
+///
+/// let _padding = text! {
+///   text: "Background includes the empty space",
+///   padding: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+///
+/// let _margin = text! {
+///   text: "Background does not include the empty space",
+///   margin: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+/// ```
 #[derive(SingleChild, Default, Clone, PartialEq)]
 pub struct Margin {
   pub margin: EdgeInsets,
@@ -22,24 +43,8 @@ impl Declare for Margin {
 
 impl Render for Margin {
   fn perform_layout(&self, clamp: BoxClamp, ctx: &mut LayoutCtx) -> Size {
-    let thickness = self.margin.thickness();
-    let zero = Size::zero();
-    let min = (clamp.min - thickness).max(zero);
-    let max = (clamp.max - thickness).max(zero);
-    let child_clamp = BoxClamp { min, max };
-
-    let child = ctx.assert_single_child();
-    let size = ctx.perform_child_layout(child, child_clamp);
-    ctx.update_position(child, Point::new(self.margin.left, self.margin.top));
-
-    size + thickness
+    space_around_layout(&self.margin, &clamp, ctx)
   }
-
-  #[inline]
-  fn only_sized_by_parent(&self) -> bool { false }
-
-  #[inline]
-  fn paint(&self, _: &mut PaintingCtx) {}
 }
 
 impl Margin {
@@ -112,6 +117,34 @@ impl std::ops::AddAssign for EdgeInsets {
   }
 }
 
+pub(crate) fn space_around_layout(
+  edges: &EdgeInsets, clamp: &BoxClamp, ctx: &mut LayoutCtx,
+) -> Size {
+  let child = match ctx.single_child() {
+    Some(c) => c,
+    None => return Size::zero(),
+  };
+
+  // Reset children position before layout
+  ctx.update_position(child, Point::zero());
+  ctx.force_child_relayout(child);
+
+  let thickness = edges.thickness();
+  let zero = Size::zero();
+  let min = (clamp.min - thickness).max(zero);
+  let max = (clamp.max - thickness).max(zero);
+
+  // Shrink the clamp of child.
+  let child_clamp = BoxClamp { min, max };
+  let size = ctx.assert_perform_single_child_layout(child_clamp);
+  let pos = ctx.position(child).unwrap();
+  let pos = pos + Vector::new(edges.left, edges.top);
+  ctx.update_position(child, pos);
+
+  // Expand the size, so the child have padding.
+  clamp.clamp(size + thickness)
+}
+
 #[cfg(test)]
 mod tests {
   use ribir_dev_helper::*;

core/src/builtin_widgets/padding.rs

@@ -1,7 +1,28 @@
-use crate::{prelude::*, wrap_render::*};
+use crate::prelude::*;
 
-/// A widget that insets its child by the given padding.
-#[derive(Default)]
+/// The widget utilizes empty space to surround the child widget.
+///
+/// Both `Padding` and `Margin` utilize empty space to surround the child
+/// widget. The difference lies in the fact that when used as a built-in widget,
+/// padding makes the empty space appear as a part of the child widget, whereas
+/// margin does not. For example:
+///
+/// ```
+/// use ribir::prelude::*;
+///
+/// let _padding = text! {
+///   text: "Background includes the empty space",
+///   padding: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+///
+/// let _margin = text! {
+///   text: "Background does not include the empty space",
+///   margin: EdgeInsets::all(10.),
+///   background: Color::GREEN,
+/// };
+/// ```
+#[derive(Default, SingleChild)]
 pub struct Padding {
   pub padding: EdgeInsets,
 }
@@ -12,37 +33,9 @@ impl Declare for Padding {
   fn declarer() -> Self::Builder { FatObj::new(()) }
 }
 
-impl_compose_child_for_wrap_render!(Padding);
-
-impl WrapRender for Padding {
-  fn perform_layout(&self, clamp: BoxClamp, host: &dyn Render, ctx: &mut LayoutCtx) -> Size {
-    let thickness = self.padding.thickness();
-    let zero = Size::zero();
-    // Reset children position before layout
-    let (ctx, children) = ctx.split_children();
-    for c in children {
-      ctx.update_position(c, Point::zero());
-    }
-
-    let min = (clamp.min - thickness).max(zero);
-    let max = (clamp.max - thickness).max(zero);
-    // Shrink the clamp of child.
-    let child_clamp = BoxClamp { min, max };
-    let mut size = host.perform_layout(child_clamp, ctx);
-
-    size = clamp.clamp(size + thickness);
-
-    let (ctx, children) = ctx.split_children();
-    // Update the children's positions to ensure they are correctly positioned after
-    // expansion with padding.
-    for c in children {
-      if let Some(pos) = ctx.widget_box_pos(c) {
-        let pos = pos + Vector::new(self.padding.left, self.padding.top);
-        ctx.update_position(c, pos);
-      }
-    }
-
-    size
+impl Render for Padding {
+  fn perform_layout(&self, clamp: BoxClamp, ctx: &mut LayoutCtx) -> Size {
+    space_around_layout(&self.padding, &clamp, ctx)
   }
 }
 
@@ -68,11 +61,32 @@ mod tests {
         }
       }
     }),
-    // padding widget
+    // Padding widget
     LayoutCase::default().with_size(Size::new(101., 100.)),
-    // MockBox
+    // MockMulti widget
     LayoutCase::new(&[0, 0])
       .with_size(Size::new(100., 100.))
-      .with_x(1.)
+      .with_x(1.),
+    // MockBox
+    LayoutCase::new(&[0, 0, 0])
+      .with_size(Size::new(100., 100.))
+      .with_x(0.)
   );
+
+  #[test]
+  #[cfg(not(target_arch = "wasm32"))]
+  fn fix_padding_draw() {
+    crate::reset_test_env!();
+
+    assert_widget_eq_image!(
+      WidgetTester::new(text! {
+        padding: EdgeInsets::all(10.),
+        background: Color::GREEN,
+        text: "Hello, Ribir!"
+      })
+      .with_wnd_size(Size::new(128., 48.))
+      .with_comparison(0.000025),
+      "padding_draw"
+    );
+  }
 }

core/src/builtin_widgets/text.rs

@@ -1,5 +1,6 @@
 use std::cell::{Ref, RefCell};
 
+use font_db::GlyphBaseline;
 use typography::PlaceLineDirection;
 
 use crate::prelude::*;
@@ -25,6 +26,7 @@ impl Render for Text {
         style,
         clamp.max,
         self.text_align,
+        GlyphBaseline::Middle,
         PlaceLineDirection::TopToBottom,
       );
 
@@ -131,4 +133,25 @@ mod tests {
     .with_wnd_size(WND_SIZE)
     .with_comparison(0.000025)
   );
+
+  widget_image_tests!(
+    middle_baseline,
+    WidgetTester::new(self::column! {
+      item_gap: 8.,
+      @Text {
+        text: "Baseline check!",
+        font_size: 20.,
+        text_line_height: 20.,
+        background: Color::RED,
+      }
+      @Text {
+        text: "Text line height check!",
+        font_size: 20.,
+        text_line_height: 40.,
+        background: Color::GREEN,
+      }
+    })
+    .with_wnd_size(WND_SIZE)
+    .with_comparison(0.000025)
+  );
 }

painter/src/painter.rs

@@ -98,16 +98,6 @@ pub enum SpreadMethod {
   Repeat,
 }
 
-impl From<usvg::SpreadMethod> for SpreadMethod {
-  fn from(value: usvg::SpreadMethod) -> Self {
-    match value {
-      usvg::SpreadMethod::Pad => SpreadMethod::Pad,
-      usvg::SpreadMethod::Reflect => SpreadMethod::Reflect,
-      usvg::SpreadMethod::Repeat => SpreadMethod::Repeat,
-    }
-  }
-}
-
 /// A path and its geometry information are friendly to paint and cache.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct PathCommand {
@@ -637,12 +627,11 @@ impl Painter {
     let Some(face) = font_db.try_get_face_data(g.face_id) else { return self };
 
     let unit = face.units_per_em() as f32;
-    let scale = font_size / unit;
-
     let matrix = *self.transform();
 
     let bounds = g.bounds();
     if let Some(path) = face.outline_glyph(g.glyph_id) {
+      let scale = font_size / unit;
       self
         .translate(bounds.min_x(), bounds.min_y())
         .scale(scale, -scale)
@@ -688,12 +677,16 @@ impl Painter {
     let Some(paint_rect) = self.intersection_paint_bounds(&box_rect) else {
       return self;
     };
+    let Some(glyphs) = visual_glyphs.glyphs_in_bounds(&paint_rect) else {
+      return self;
+    };
+
     if !paint_rect.contains_rect(&visual_rect) {
       self.clip(Path::rect(&paint_rect).into());
     }
     self.translate(visual_rect.origin.x, visual_rect.origin.y);
 
-    for g in visual_glyphs.glyphs_in_bounds(&paint_rect) {
+    for g in glyphs {
       self.draw_glyph(&g, visual_glyphs.font_size(), font_db);
     }
 
@@ -899,6 +892,17 @@ impl CommandBrush {
     self
   }
 }
+
+impl From<usvg::SpreadMethod> for SpreadMethod {
+  fn from(value: usvg::SpreadMethod) -> Self {
+    match value {
+      usvg::SpreadMethod::Pad => SpreadMethod::Pad,
+      usvg::SpreadMethod::Reflect => SpreadMethod::Reflect,
+      usvg::SpreadMethod::Repeat => SpreadMethod::Repeat,
+    }
+  }
+}
+
 // bounds that has a limited location and size
 fn locatable_bounds(bounds: &Rect) -> bool {
   bounds.origin.is_finite() && !bounds.width().is_nan() && !bounds.height().is_nan()

painter/src/text.rs

@@ -191,6 +191,8 @@ impl GlyphUnit {
   pub const MAX: Self = Self(i32::MAX);
   pub const STANDARD_EM: Self = Self(Self::UNITS_PER_EM as i32);
 
+  pub fn new(pos: i32) -> Self { Self(pos) }
+
   pub fn from_pixel(pos: f32) -> Self { Self(f32::ceil(pos * Self::UNITS_PER_PIXEL as f32) as i32) }
 
   pub fn max(&self, other: Self) -> Self { Self(self.0.max(other.0)) }