~ireas/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH merge-rs] Calculate line spacing in paragraphs and remove left bearing on first character of lines

Details
Message ID
<20210620091339.26704-1-ap4sc4@gmail.com>
DKIM signature
missing
Download raw message
Patch: +145 -34
---
 src/elements.rs | 33 ++++++++++++++++-------
 src/fonts.rs    | 71 +++++++++++++++++++++++++++++++++++++++++++++----
 src/render.rs   | 59 ++++++++++++++++++++++++++--------------
 src/style.rs    | 16 +++++++++++
 4 files changed, 145 insertions(+), 34 deletions(-)

diff --git a/src/elements.rs b/src/elements.rs
index 1ff2dea..b7205eb 100644
--- a/src/elements.rs
+++ b/src/elements.rs
@@ -48,6 +48,7 @@ use std::iter;
use std::mem;

use crate::error::{Error, ErrorKind};
use crate::fonts;
use crate::render;
use crate::style::{Style, StyledString};
use crate::wrap;
@@ -197,9 +198,7 @@ impl Element for Text {
/// strings to this paragraph.  Besides the styling of the text (see [`Style`][]), you can also set
/// an [`Alignment`][] for the paragraph.
///
/// Note that the line height and spacing is currently calculated based on the style of the entire
/// paragraph.  If the font family or font size is changed in the [`Style`][] settings for a
/// string, the line height and spacing might be incorrect.
/// The line height and spacing are calculated based on the style of each string.
///
/// # Examples
///
@@ -315,14 +314,28 @@ impl Element for Paragraph {
            self.words = wrap::Words::new(mem::take(&mut self.text)).collect();
        }

        let height = style.line_height(&context.font_cache);
        let p_metrics = style.metrics(&context.font_cache);
        let words = self.words.iter().map(Into::into);
        let mut rendered_len = 0;
        for (line, delta) in wrap::Wrapper::new(words, context, area.size().width) {
            let width = line.iter().map(|s| s.width(&context.font_cache)).sum();
            let position = Position::new(self.get_offset(width, area.size().width), 0);
            // TODO: calculate the maximum line height
            if let Some(mut section) = area.text_section(&context.font_cache, position, style) {
            // Calculate the maximum line height
            let mut max_line_height = Mm::from(0.0);
            for s in &line {
                let line_height = s.style.metrics(&context.font_cache).line_height();
                if line_height > max_line_height {
                    max_line_height = line_height;
                }
            }
            let line_metrics = fonts::Metrics::new(max_line_height, p_metrics.glyph_height());
            let position = Position::new(
                self.get_offset(width, area.size().width),
                line_metrics.line_height(),
            );

            if let Some(mut section) =
                area.text_section(&context.font_cache, position, line_metrics)
            {
                for s in line {
                    section.print_str(&s.s, s.style)?;
                    rendered_len += s.s.len();
@@ -332,8 +345,10 @@ impl Element for Paragraph {
                result.has_more = true;
                break;
            }
            result.size = result.size.stack_vertical(Size::new(width, height));
            area.add_offset(Position::new(0, height));
            result.size = result
                .size
                .stack_vertical(Size::new(width, line_metrics.line_height()));
            area.add_offset(Position::new(0, line_metrics.line_height()));
        }

        // Remove the rendered data from self.words so that we don’t render it again on the next
diff --git a/src/fonts.rs b/src/fonts.rs
index efcbb6d..7378c7d 100644
--- a/src/fonts.rs
+++ b/src/fonts.rs
@@ -377,15 +377,31 @@ impl Font {
    ///
    /// [`FontCache`]: struct.FontCache.html
    pub fn char_width(&self, font_cache: &FontCache, c: char, font_size: u8) -> Mm {
        let advance_width = font_cache
        let advance_width = self.char_h_metrics(font_cache, c).advance_width;
        Mm::from(printpdf::Pt(f64::from(
            advance_width * f32::from(font_size),
        )))
    }

    /// Returns the width of the empty space between the origin of the glyph bounding
    /// box and the leftmost edge of the character, for a given font and font size.
    ///
    /// The given [`FontCache`][] must be the font cache that loaded this font.
    ///
    /// [`FontCache`]: struct.FontCache.html
    pub fn char_left_side_bearing(&self, font_cache: &FontCache, c: char, font_size: u8) -> Mm {
        let left_side_bearing = self.char_h_metrics(font_cache, c).left_side_bearing;
        Mm::from(printpdf::Pt(f64::from(
            left_side_bearing * f32::from(font_size),
        )))
    }

    fn char_h_metrics(&self, font_cache: &FontCache, c: char) -> rusttype::HMetrics {
        font_cache
            .get_rt_font(*self)
            .glyph(c)
            .scaled(self.scale)
            .h_metrics()
            .advance_width;
        Mm::from(printpdf::Pt(f64::from(
            advance_width * f32::from(font_size),
        )))
    }

    /// Returns the width of a string with this font and the given font size.
@@ -449,6 +465,14 @@ impl Font {
            .map(|g| g.id().0 as u16)
            .collect()
    }

    /// Calculate the metrics of a given font size for this font.
    pub fn metrics(&self, font_size: u8) -> Metrics {
        Metrics::new(
            self.line_height * f64::from(font_size),
            self.glyph_height * f64::from(font_size),
        )
    }
}

fn from_file(
@@ -488,3 +512,40 @@ pub fn from_files(
        bold_italic: from_file(dir, name, FontStyle::BoldItalic, builtin)?,
    })
}

/// The metrics of a font: glyph height and line height.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct Metrics {
    line_height: Mm,
    glyph_height: Mm,
}

impl Metrics {
    /// Create a new metrics instance with the given heights.
    pub fn new(line_height: Mm, glyph_height: Mm) -> Metrics {
        Metrics {
            line_height,
            glyph_height,
        }
    }

    /// Line height setter.
    pub fn set_line_height(&mut self, line_height: Mm) {
        self.line_height = line_height;
    }

    /// Glyph height setter.
    pub fn set_glyph_height(&mut self, glyph_height: Mm) {
        self.glyph_height = glyph_height;
    }

    /// Line height getter.
    pub fn line_height(&self) -> Mm {
        self.line_height
    }

    /// Glyph height getter.
    pub fn glyph_height(&self) -> Mm {
        self.glyph_height
    }
}
diff --git a/src/render.rs b/src/render.rs
index 5ecce1e..19f717c 100644
--- a/src/render.rs
+++ b/src/render.rs
@@ -370,7 +370,9 @@ impl<'a> Area<'a> {
        style: Style,
        s: S,
    ) -> Result<bool, Error> {
        if let Some(mut section) = self.text_section(font_cache, position, style) {
        if let Some(mut section) =
            self.text_section(font_cache, position, style.metrics(font_cache))
        {
            section.print_str(s, style)?;
            Ok(true)
        } else {
@@ -387,9 +389,9 @@ impl<'a> Area<'a> {
        &self,
        font_cache: &'f fonts::FontCache,
        position: Position,
        style: Style,
        metrics: fonts::Metrics,
    ) -> Option<TextSection<'_, 'f, 'a>> {
        TextSection::new(font_cache, self, position, style)
        TextSection::new(font_cache, self, position, metrics)
    }

    /// Transforms the given position that is relative to the upper left corner of the area to a
@@ -408,9 +410,10 @@ impl<'a> Area<'a> {
pub struct TextSection<'a, 'f, 'l> {
    font_cache: &'f fonts::FontCache,
    area: &'a Area<'l>,
    line_height: Mm,
    cursor: Position,
    position: Position,
    fill_color: Option<Color>,
    is_first: bool,
    metrics: fonts::Metrics,
}

impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
@@ -418,40 +421,45 @@ impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
        font_cache: &'f fonts::FontCache,
        area: &'a Area<'l>,
        position: Position,
        style: Style,
        metrics: fonts::Metrics,
    ) -> Option<TextSection<'a, 'f, 'l>> {
        let height = style.font(font_cache).glyph_height(style.font_size());

        if position.y + height > area.size.height {
        if position.y + metrics.glyph_height() > area.size.height {
            return None;
        }

        let line_height = style.line_height(font_cache);
        let section = TextSection {
            font_cache,
            area,
            line_height,
            cursor: position,
            position,
            fill_color: None,
            is_first: true,
            metrics,
        };

        section.layer().begin_text_section();
        section.layer().set_line_height(line_height.into());
        let cursor = area.transform_position(position);
        section
            .layer()
            .set_text_cursor(cursor.x.into(), (cursor.y - height).into());
            .set_line_height(metrics.line_height().into());
        Some(section)
    }

    fn set_text_cursor(&mut self) {
        let cursor_t = self.area.transform_position(self.position);
        self.layer().set_text_cursor(
            cursor_t.x.into(),
            (cursor_t.y - self.metrics.glyph_height()).into(),
        );
    }

    /// Tries to add a new line and returns `true` if the area was large enough to fit the new
    /// line.
    #[must_use]
    pub fn add_newline(&mut self) -> bool {
        if self.cursor.y + self.line_height > self.area.size.height {
        if self.position.y + self.metrics.line_height() > self.area.size.height {
            false
        } else {
            self.layer().add_line_break();
            self.cursor.y += self.line_height;
            self.position.y += self.metrics.line_height();
            true
        }
    }
@@ -461,18 +469,29 @@ impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
    /// The font cache for this text section must contain the PDF font for the given style.
    pub fn print_str(&mut self, s: impl AsRef<str>, style: Style) -> Result<(), Error> {
        let font = style.font(self.font_cache);
        let s_ref = s.as_ref();

        // Adjust cursor to remove left bearing of the first character of the first string
        if self.is_first {
            if let Some(first_c) = s_ref.chars().nth(0) {
                let l_bearing = style.char_left_side_bearing(self.font_cache, first_c);
                self.position.x -= l_bearing;
            }
            self.set_text_cursor();
        }
        self.is_first = false;

        let positions = font
            .kerning(self.font_cache, s.as_ref().chars())
            .kerning(self.font_cache, s_ref.chars())
            .into_iter()
            // Kerning is measured in 1/1000 em
            .map(|pos| pos * -1000.0)
            .map(|pos| pos as i64);
        let codepoints = if font.is_builtin() {
            // Built-in fonts always use the Windows-1252 encoding
            encode_win1252(s.as_ref())?
            encode_win1252(s_ref)?
        } else {
            font.glyph_ids(&self.font_cache, s.as_ref().chars())
            font.glyph_ids(&self.font_cache, s_ref.chars())
        };

        let font = self
diff --git a/src/style.rs b/src/style.rs
index e77258a..adb0973 100644
--- a/src/style.rs
+++ b/src/style.rs
@@ -254,6 +254,17 @@ impl Style {
            .char_width(font_cache, c, self.font_size())
    }

    /// Returns the width of the empty space between the origin of the glyph bounding
    /// box and the leftmost edge of the character, for this style using the given font cache.
    ///
    /// If the font family is set, it must have been created by the given [`FontCache`][].
    ///
    /// [`FontCache`]: ../fonts/struct.FontCache.html
    pub fn char_left_side_bearing(&self, font_cache: &fonts::FontCache, c: char) -> Mm {
        self.font(font_cache)
            .char_left_side_bearing(font_cache, c, self.font_size())
    }

    /// Calculates the width of the given string with this style using the data in the given font
    /// cache.
    ///
@@ -294,6 +305,11 @@ impl Style {
    pub fn line_height(&self, font_cache: &fonts::FontCache) -> Mm {
        self.font(font_cache).get_line_height(self.font_size()) * self.line_spacing()
    }

    /// Calculate the metrics of the font for this style.
    pub fn metrics(&self, font_cache: &fonts::FontCache) -> fonts::Metrics {
        self.font(font_cache).metrics(self.font_size())
    }
}

impl From<Color> for Style {
-- 
2.28.0

[merge-rs/patches] build failed

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CC8BXPJ6UEY6.1Y274MJN0P3WE@cirno>
In-Reply-To
<20210620091339.26704-1-ap4sc4@gmail.com> (view parent)
DKIM signature
missing
Download raw message
merge-rs/patches: FAILED in 59s

[Calculate line spacing in paragraphs and remove left bearing on first character of lines][0] from [ap4sc][1]

[0]: https://lists.sr.ht/~ireas/public-inbox/patches/23401
[1]: ap4sc4@gmail.com

✗ #529228 FAILED merge-rs/patches/archlinux.yml      https://builds.sr.ht/~ireas/job/529228
✗ #529227 FAILED merge-rs/patches/archlinux-msrv.yml https://builds.sr.ht/~ireas/job/529227
Details
Message ID
<0F3C0467-92B9-4808-A74D-ABF889D941E9@gmail.com>
In-Reply-To
<20210620091339.26704-1-ap4sc4@gmail.com> (view parent)
DKIM signature
missing
Download raw message
I’m not sure why this failed. Do I need to merge into the latest state of your master branch first before patching?

> On Jun 20, 2021, at 2:13 AM, ap4sc <ap4sc4@gmail.com> wrote:
> 
> ---
> src/elements.rs | 33 ++++++++++++++++-------
> src/fonts.rs    | 71 +++++++++++++++++++++++++++++++++++++++++++++----
> src/render.rs   | 59 ++++++++++++++++++++++++++--------------
> src/style.rs    | 16 +++++++++++
> 4 files changed, 145 insertions(+), 34 deletions(-)
> 
> diff --git a/src/elements.rs b/src/elements.rs
> index 1ff2dea..b7205eb 100644
> --- a/src/elements.rs
> +++ b/src/elements.rs
> @@ -48,6 +48,7 @@ use std::iter;
> use std::mem;
> 
> use crate::error::{Error, ErrorKind};
> +use crate::fonts;
> use crate::render;
> use crate::style::{Style, StyledString};
> use crate::wrap;
> @@ -197,9 +198,7 @@ impl Element for Text {
> /// strings to this paragraph.  Besides the styling of the text (see [`Style`][]), you can also set
> /// an [`Alignment`][] for the paragraph.
> ///
> -/// Note that the line height and spacing is currently calculated based on the style of the entire
> -/// paragraph.  If the font family or font size is changed in the [`Style`][] settings for a
> -/// string, the line height and spacing might be incorrect.
> +/// The line height and spacing are calculated based on the style of each string.
> ///
> /// # Examples
> ///
> @@ -315,14 +314,28 @@ impl Element for Paragraph {
>             self.words = wrap::Words::new(mem::take(&mut self.text)).collect();
>         }
> 
> -        let height = style.line_height(&context.font_cache);
> +        let p_metrics = style.metrics(&context.font_cache);
>         let words = self.words.iter().map(Into::into);
>         let mut rendered_len = 0;
>         for (line, delta) in wrap::Wrapper::new(words, context, area.size().width) {
>             let width = line.iter().map(|s| s.width(&context.font_cache)).sum();
> -            let position = Position::new(self.get_offset(width, area.size().width), 0);
> -            // TODO: calculate the maximum line height
> -            if let Some(mut section) = area.text_section(&context.font_cache, position, style) {
> +            // Calculate the maximum line height
> +            let mut max_line_height = Mm::from(0.0);
> +            for s in &line {
> +                let line_height = s.style.metrics(&context.font_cache).line_height();
> +                if line_height > max_line_height {
> +                    max_line_height = line_height;
> +                }
> +            }
> +            let line_metrics = fonts::Metrics::new(max_line_height, p_metrics.glyph_height());
> +            let position = Position::new(
> +                self.get_offset(width, area.size().width),
> +                line_metrics.line_height(),
> +            );
> +
> +            if let Some(mut section) =
> +                area.text_section(&context.font_cache, position, line_metrics)
> +            {
>                 for s in line {
>                     section.print_str(&s.s, s.style)?;
>                     rendered_len += s.s.len();
> @@ -332,8 +345,10 @@ impl Element for Paragraph {
>                 result.has_more = true;
>                 break;
>             }
> -            result.size = result.size.stack_vertical(Size::new(width, height));
> -            area.add_offset(Position::new(0, height));
> +            result.size = result
> +                .size
> +                .stack_vertical(Size::new(width, line_metrics.line_height()));
> +            area.add_offset(Position::new(0, line_metrics.line_height()));
>         }
> 
>         // Remove the rendered data from self.words so that we don’t render it again on the next
> diff --git a/src/fonts.rs b/src/fonts.rs
> index efcbb6d..7378c7d 100644
> --- a/src/fonts.rs
> +++ b/src/fonts.rs
> @@ -377,15 +377,31 @@ impl Font {
>     ///
>     /// [`FontCache`]: struct.FontCache.html
>     pub fn char_width(&self, font_cache: &FontCache, c: char, font_size: u8) -> Mm {
> -        let advance_width = font_cache
> +        let advance_width = self.char_h_metrics(font_cache, c).advance_width;
> +        Mm::from(printpdf::Pt(f64::from(
> +            advance_width * f32::from(font_size),
> +        )))
> +    }
> +
> +    /// Returns the width of the empty space between the origin of the glyph bounding
> +    /// box and the leftmost edge of the character, for a given font and font size.
> +    ///
> +    /// The given [`FontCache`][] must be the font cache that loaded this font.
> +    ///
> +    /// [`FontCache`]: struct.FontCache.html
> +    pub fn char_left_side_bearing(&self, font_cache: &FontCache, c: char, font_size: u8) -> Mm {
> +        let left_side_bearing = self.char_h_metrics(font_cache, c).left_side_bearing;
> +        Mm::from(printpdf::Pt(f64::from(
> +            left_side_bearing * f32::from(font_size),
> +        )))
> +    }
> +
> +    fn char_h_metrics(&self, font_cache: &FontCache, c: char) -> rusttype::HMetrics {
> +        font_cache
>             .get_rt_font(*self)
>             .glyph(c)
>             .scaled(self.scale)
>             .h_metrics()
> -            .advance_width;
> -        Mm::from(printpdf::Pt(f64::from(
> -            advance_width * f32::from(font_size),
> -        )))
>     }
> 
>     /// Returns the width of a string with this font and the given font size.
> @@ -449,6 +465,14 @@ impl Font {
>             .map(|g| g.id().0 as u16)
>             .collect()
>     }
> +
> +    /// Calculate the metrics of a given font size for this font.
> +    pub fn metrics(&self, font_size: u8) -> Metrics {
> +        Metrics::new(
> +            self.line_height * f64::from(font_size),
> +            self.glyph_height * f64::from(font_size),
> +        )
> +    }
> }
> 
> fn from_file(
> @@ -488,3 +512,40 @@ pub fn from_files(
>         bold_italic: from_file(dir, name, FontStyle::BoldItalic, builtin)?,
>     })
> }
> +
> +/// The metrics of a font: glyph height and line height.
> +#[derive(Clone, Copy, Debug, PartialEq)]
> +pub struct Metrics {
> +    line_height: Mm,
> +    glyph_height: Mm,
> +}
> +
> +impl Metrics {
> +    /// Create a new metrics instance with the given heights.
> +    pub fn new(line_height: Mm, glyph_height: Mm) -> Metrics {
> +        Metrics {
> +            line_height,
> +            glyph_height,
> +        }
> +    }
> +
> +    /// Line height setter.
> +    pub fn set_line_height(&mut self, line_height: Mm) {
> +        self.line_height = line_height;
> +    }
> +
> +    /// Glyph height setter.
> +    pub fn set_glyph_height(&mut self, glyph_height: Mm) {
> +        self.glyph_height = glyph_height;
> +    }
> +
> +    /// Line height getter.
> +    pub fn line_height(&self) -> Mm {
> +        self.line_height
> +    }
> +
> +    /// Glyph height getter.
> +    pub fn glyph_height(&self) -> Mm {
> +        self.glyph_height
> +    }
> +}
> diff --git a/src/render.rs b/src/render.rs
> index 5ecce1e..19f717c 100644
> --- a/src/render.rs
> +++ b/src/render.rs
> @@ -370,7 +370,9 @@ impl<'a> Area<'a> {
>         style: Style,
>         s: S,
>     ) -> Result<bool, Error> {
> -        if let Some(mut section) = self.text_section(font_cache, position, style) {
> +        if let Some(mut section) =
> +            self.text_section(font_cache, position, style.metrics(font_cache))
> +        {
>             section.print_str(s, style)?;
>             Ok(true)
>         } else {
> @@ -387,9 +389,9 @@ impl<'a> Area<'a> {
>         &self,
>         font_cache: &'f fonts::FontCache,
>         position: Position,
> -        style: Style,
> +        metrics: fonts::Metrics,
>     ) -> Option<TextSection<'_, 'f, 'a>> {
> -        TextSection::new(font_cache, self, position, style)
> +        TextSection::new(font_cache, self, position, metrics)
>     }
> 
>     /// Transforms the given position that is relative to the upper left corner of the area to a
> @@ -408,9 +410,10 @@ impl<'a> Area<'a> {
> pub struct TextSection<'a, 'f, 'l> {
>     font_cache: &'f fonts::FontCache,
>     area: &'a Area<'l>,
> -    line_height: Mm,
> -    cursor: Position,
> +    position: Position,
>     fill_color: Option<Color>,
> +    is_first: bool,
> +    metrics: fonts::Metrics,
> }
> 
> impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
> @@ -418,40 +421,45 @@ impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
>         font_cache: &'f fonts::FontCache,
>         area: &'a Area<'l>,
>         position: Position,
> -        style: Style,
> +        metrics: fonts::Metrics,
>     ) -> Option<TextSection<'a, 'f, 'l>> {
> -        let height = style.font(font_cache).glyph_height(style.font_size());
> -
> -        if position.y + height > area.size.height {
> +        if position.y + metrics.glyph_height() > area.size.height {
>             return None;
>         }
> 
> -        let line_height = style.line_height(font_cache);
>         let section = TextSection {
>             font_cache,
>             area,
> -            line_height,
> -            cursor: position,
> +            position,
>             fill_color: None,
> +            is_first: true,
> +            metrics,
>         };
> +
>         section.layer().begin_text_section();
> -        section.layer().set_line_height(line_height.into());
> -        let cursor = area.transform_position(position);
>         section
>             .layer()
> -            .set_text_cursor(cursor.x.into(), (cursor.y - height).into());
> +            .set_line_height(metrics.line_height().into());
>         Some(section)
>     }
> 
> +    fn set_text_cursor(&mut self) {
> +        let cursor_t = self.area.transform_position(self.position);
> +        self.layer().set_text_cursor(
> +            cursor_t.x.into(),
> +            (cursor_t.y - self.metrics.glyph_height()).into(),
> +        );
> +    }
> +
>     /// Tries to add a new line and returns `true` if the area was large enough to fit the new
>     /// line.
>     #[must_use]
>     pub fn add_newline(&mut self) -> bool {
> -        if self.cursor.y + self.line_height > self.area.size.height {
> +        if self.position.y + self.metrics.line_height() > self.area.size.height {
>             false
>         } else {
>             self.layer().add_line_break();
> -            self.cursor.y += self.line_height;
> +            self.position.y += self.metrics.line_height();
>             true
>         }
>     }
> @@ -461,18 +469,29 @@ impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
>     /// The font cache for this text section must contain the PDF font for the given style.
>     pub fn print_str(&mut self, s: impl AsRef<str>, style: Style) -> Result<(), Error> {
>         let font = style.font(self.font_cache);
> +        let s_ref = s.as_ref();
> +
> +        // Adjust cursor to remove left bearing of the first character of the first string
> +        if self.is_first {
> +            if let Some(first_c) = s_ref.chars().nth(0) {
> +                let l_bearing = style.char_left_side_bearing(self.font_cache, first_c);
> +                self.position.x -= l_bearing;
> +            }
> +            self.set_text_cursor();
> +        }
> +        self.is_first = false;
> 
>         let positions = font
> -            .kerning(self.font_cache, s.as_ref().chars())
> +            .kerning(self.font_cache, s_ref.chars())
>             .into_iter()
>             // Kerning is measured in 1/1000 em
>             .map(|pos| pos * -1000.0)
>             .map(|pos| pos as i64);
>         let codepoints = if font.is_builtin() {
>             // Built-in fonts always use the Windows-1252 encoding
> -            encode_win1252(s.as_ref())?
> +            encode_win1252(s_ref)?
>         } else {
> -            font.glyph_ids(&self.font_cache, s.as_ref().chars())
> +            font.glyph_ids(&self.font_cache, s_ref.chars())
>         };
> 
>         let font = self
> diff --git a/src/style.rs b/src/style.rs
> index e77258a..adb0973 100644
> --- a/src/style.rs
> +++ b/src/style.rs
> @@ -254,6 +254,17 @@ impl Style {
>             .char_width(font_cache, c, self.font_size())
>     }
> 
> +    /// Returns the width of the empty space between the origin of the glyph bounding
> +    /// box and the leftmost edge of the character, for this style using the given font cache.
> +    ///
> +    /// If the font family is set, it must have been created by the given [`FontCache`][].
> +    ///
> +    /// [`FontCache`]: ../fonts/struct.FontCache.html
> +    pub fn char_left_side_bearing(&self, font_cache: &fonts::FontCache, c: char) -> Mm {
> +        self.font(font_cache)
> +            .char_left_side_bearing(font_cache, c, self.font_size())
> +    }
> +
>     /// Calculates the width of the given string with this style using the data in the given font
>     /// cache.
>     ///
> @@ -294,6 +305,11 @@ impl Style {
>     pub fn line_height(&self, font_cache: &fonts::FontCache) -> Mm {
>         self.font(font_cache).get_line_height(self.font_size()) * self.line_spacing()
>     }
> +
> +    /// Calculate the metrics of the font for this style.
> +    pub fn metrics(&self, font_cache: &fonts::FontCache) -> fonts::Metrics {
> +        self.font(font_cache).metrics(self.font_size())
> +    }
> }
> 
> impl From<Color> for Style {
> -- 
> 2.28.0
> 
Details
Message ID
<20210620093252.GB1215@ireas.org>
In-Reply-To
<0F3C0467-92B9-4808-A74D-ABF889D941E9@gmail.com> (view parent)
DKIM signature
missing
Download raw message
On 2021-06-20 02:22:39, Ian C wrote:
> I’m not sure why this failed. Do I need to merge into the latest state
> of your master branch first before patching?

Not necessarily, only if there are conflicting changes (which is true in
this case).  But I think these are only minor conflicts, so I can
resolve them while reviewing.

/Robin
Details
Message ID
<20210620103024.GC1215@ireas.org>
In-Reply-To
<20210620091339.26704-1-ap4sc4@gmail.com> (view parent)
DKIM signature
missing
Download raw message
Thank you very much for this patch!  I’ve applied it to master with some
small changes:

On 2021-06-20 02:13:39, ap4sc wrote:
> @@ -315,14 +314,28 @@ impl Element for Paragraph {
>              self.words = wrap::Words::new(mem::take(&mut self.text)).collect();
>          }
>  
> -        let height = style.line_height(&context.font_cache);
> +        let p_metrics = style.metrics(&context.font_cache);
>          let words = self.words.iter().map(Into::into);
>          let mut rendered_len = 0;
>          for (line, delta) in wrap::Wrapper::new(words, context, area.size().width) {
>              let width = line.iter().map(|s| s.width(&context.font_cache)).sum();
> -            let position = Position::new(self.get_offset(width, area.size().width), 0);
> -            // TODO: calculate the maximum line height
> -            if let Some(mut section) = area.text_section(&context.font_cache, position, style) {
> +            // Calculate the maximum line height
> +            let mut max_line_height = Mm::from(0.0);
> +            for s in &line {
> +                let line_height = s.style.metrics(&context.font_cache).line_height();
> +                if line_height > max_line_height {
> +                    max_line_height = line_height;
> +                }
> +            }
> +            let line_metrics = fonts::Metrics::new(max_line_height, p_metrics.glyph_height());

We also need to calculate the maximum glyph height here.

> +            let position = Position::new(
> +                self.get_offset(width, area.size().width),
> +                line_metrics.line_height(),
> +            );

I think the height should still be zero here and not the line height.

> +/// The metrics of a font: glyph height and line height.
> +#[derive(Clone, Copy, Debug, PartialEq)]
> +pub struct Metrics {
> +    line_height: Mm,
> +    glyph_height: Mm,
> +}

For this small struct, I find it more convenient to have public fields
so that we don’t need the getters and setters.

> @@ -461,18 +469,29 @@ impl<'a, 'f, 'l> TextSection<'a, 'f, 'l> {
>      /// The font cache for this text section must contain the PDF font for the given style.
>      pub fn print_str(&mut self, s: impl AsRef<str>, style: Style) -> Result<(), Error> {
>          let font = style.font(self.font_cache);
> +        let s_ref = s.as_ref();

As we don’t lose any information, it is fine to just use
    let s = s.as_ref();
here.

> @@ -294,6 +305,11 @@ impl Style {
>      pub fn line_height(&self, font_cache: &fonts::FontCache) -> Mm {
>          self.font(font_cache).get_line_height(self.font_size()) * self.line_spacing()
>      }
> +
> +    /// Calculate the metrics of the font for this style.
> +    pub fn metrics(&self, font_cache: &fonts::FontCache) -> fonts::Metrics {
> +        self.font(font_cache).metrics(self.font_size())

At this point, we also have to take into account the line spacing.

Also, please try to add short entries to the changelog for changes that
affect library users.

/Robin
Reply to thread Export thread (mbox)