`Layout::height` seems too short

[ennis]

The following example, adapted from swash_render, fails on my machine (Windows 11)

Code

```rust //! A simple example that lays out some text using Parley, rasterises the glyph using Swash //! and then renders it into a PNG using the `image` crate. use image::codecs::png::PngEncoder; use image::{self, Pixel, Rgba, RgbaImage}; use parley::layout::{Alignment, Glyph, GlyphRun, Layout, PositionedLayoutItem}; use parley::style::{FontStack, FontWeight, StyleProperty, TextStyle}; use parley::{FontContext, InlineBox, LayoutContext}; use peniko::Color; use std::fs::File; use swash::scale::image::Content; use swash::scale::{Render, ScaleContext, Scaler, Source, StrikeWith}; use swash::zeno; use swash::FontRef; use zeno::{Format, Vector}; fn main() { // The text we are going to style and lay out let text = String::from("Sample Text"); // The display scale for HiDPI rendering let display_scale = 1.0; // The width for line wrapping let max_advance = Some(400.0 * display_scale); // Colours for rendering let text_color = Color::rgb8(0, 0, 0); let bg_color = Rgba([255, 255, 255, 255]); // Padding around the output image let padding = 1; // Create a FontContext, LayoutContext and ScaleContext // // These are all intended to be constructed rarely (perhaps even once per app (or once per thread)) // and provide caches and scratch space to avoid allocations let mut font_cx = FontContext::default(); let mut layout_cx = LayoutContext::new(); let mut scale_cx = ScaleContext::new(); // Setup some Parley text styles let font_stack = FontStack::Source("Inter"); let mut layout = { // TREE BUILDER // ============ let root_style = TextStyle { brush: text_color, font_stack, line_height: 1.0, font_size: 40.0, ..Default::default() }; let mut builder = layout_cx.tree_builder(&mut font_cx, display_scale, &root_style); builder.push_text(&text); // Build the builder into a Layout // let mut layout: Layout = builder.build(&text); let (layout, _text): (Layout, String) = builder.build(); layout }; // Perform layout (including bidi resolution and shaping) with start alignment layout.break_all_lines(max_advance); layout.align(max_advance, Alignment::Start); // Create image to render into let width = layout.width().ceil() as u32 + (padding * 2); let height = layout.height().ceil() as u32 + (padding * 2); let mut img = RgbaImage::from_pixel(width, height, bg_color); // Iterate over laid out lines for line in layout.lines() { // Iterate over GlyphRun's within each line for item in line.items() { match item { PositionedLayoutItem::GlyphRun(glyph_run) => { render_glyph_run(&mut scale_cx, &glyph_run, &mut img, padding); } PositionedLayoutItem::InlineBox(inline_box) => { for x_off in 0..(inline_box.width.floor() as u32) { for y_off in 0..(inline_box.height.floor() as u32) { let x = inline_box.x as u32 + x_off + padding; let y = inline_box.y as u32 + y_off + padding; img.put_pixel(x, y, Rgba([0, 0, 0, 255])); } } } }; } } // Write image to PNG file in examples/_output dir let output_path = { let path = std::path::PathBuf::from(file!()); let mut path = std::fs::canonicalize(path).unwrap(); path.pop(); path.pop(); path.pop(); path.push("_output"); let _ = std::fs::create_dir(path.clone()); path.push("swash_render.png"); path }; let output_file = File::create(output_path).unwrap(); let png_encoder = PngEncoder::new(output_file); img.write_with_encoder(png_encoder).unwrap(); } fn render_glyph_run( context: &mut ScaleContext, glyph_run: &GlyphRun, img: &mut RgbaImage, padding: u32, ) { // Resolve properties of the GlyphRun let mut run_x = glyph_run.offset(); let run_y = glyph_run.baseline(); let style = glyph_run.style(); let color = style.brush; // Get the "Run" from the "GlyphRun" let run = glyph_run.run(); // Resolve properties of the Run let font = run.font(); let font_size = run.font_size(); let normalized_coords = run.normalized_coords(); // Convert from parley::Font to swash::FontRef let font_ref = FontRef::from_index(font.data.as_ref(), font.index as usize).unwrap(); // Build a scaler. As the font properties are constant across an entire run of glyphs // we can build one scaler for the run and reuse it for each glyph. let mut scaler = context .builder(font_ref) .size(font_size) .hint(true) .normalized_coords(normalized_coords) .build(); // Iterates over the glyphs in the GlyphRun for glyph in glyph_run.glyphs() { let glyph_x = run_x + glyph.x + (padding as f32); let glyph_y = run_y - glyph.y + (padding as f32); run_x += glyph.advance; render_glyph(img, &mut scaler, color, glyph, glyph_x, glyph_y); } } fn render_glyph( img: &mut RgbaImage, scaler: &mut Scaler, color: Color, glyph: Glyph, glyph_x: f32, glyph_y: f32, ) { // Compute the fractional offset // You'll likely want to quantize this in a real renderer let offset = Vector::new(glyph_x.fract(), glyph_y.fract()); // Render the glyph using swash let rendered_glyph = Render::new( // Select our source order &[ Source::ColorOutline(0), Source::ColorBitmap(StrikeWith::BestFit), Source::Outline, ], ) // Select the simple alpha (non-subpixel) format .format(Format::Alpha) // Apply the fractional offset .offset(offset) // Render the image .render(scaler, glyph.id) .unwrap(); let glyph_width = rendered_glyph.placement.width; let glyph_height = rendered_glyph.placement.height; let glyph_x = (glyph_x.floor() as i32 + rendered_glyph.placement.left) as u32; let glyph_y = (glyph_y.floor() as i32 - rendered_glyph.placement.top) as u32; match rendered_glyph.content { Content::Mask => { let mut i = 0; for pixel_y in 0..glyph_height { for pixel_x in 0..glyph_width { let x = glyph_x + pixel_x; let y = glyph_y + pixel_y; let alpha = rendered_glyph.data[i]; let color = Rgba([color.r, color.g, color.b, alpha]); img.get_pixel_mut(x, y).blend(&color); i += 1; } } } Content::SubpixelMask => unimplemented!(), Content::Color => { let row_size = glyph_width as usize * 4; for (pixel_y, row) in rendered_glyph.data.chunks_exact(row_size).enumerate() { for (pixel_x, pixel) in row.chunks_exact(4).enumerate() { let x = glyph_x + pixel_x as u32; let y = glyph_y + pixel_y as u32; let color = Rgba(pixel.try_into().expect("Not RGBA")); img.get_pixel_mut(x, y).blend(&color); } } } }; } ```

with thread 'main' panicked at examples\swash_render\src/main.rs:197:25: Image index (86, 42) out of bounds (235, 42). It seems that the allocated image isn't tall enough. It does work correctly if I change the text to Samle Text (without the p). It's as if the height calculation didn't take into account the descender of the p.

[nicoburns]

So the current implementation of computed height computes the height of each line as line_height * font_size (where these are the input styles). This matches how the web / CSS computes line height, and is necessary if you want to match how browsers layout text. (see https://github.com/linebender/parley/pull/84)

However as you have discovered, it does not accurately measure how much space is actually needed for a visual rendering of the text. We do have this information, so we could definitely expose this in addition to the "layout line height". We could also consider making the line height computation (for the purposes of layout) configurable.

In the meantime you might want to consider increasing the line height you are using. 1.2 is the default used by web browsers, and will generally leave enough room for descenders. And anything from 1.3 to 1.5 is often recommended by typographic experts for aesthetic / readability purposes.

[ennis]

I see, this makes sense; in my case I am interested in the pixel bounds of the rendered glyph runs, and not so much about the typographic height, so that I can allocate an image large enough for rendering. But extra padding / line height will do in the meantime.