GlyphBrush::pixel_bounds seems to return incorrect bounds

[hecrj]

GlyphBrush::pixel_bounds seems to return an incorrect result. I found about this when working on basic GUI support for Coffee. You can find more details in hecrj/coffee#43.

Here is an SSCCE using gfx_glyph (full code):

let section = Section {
    text: "Pixel bounds test",
    screen_position: (50.0, 50.0),
    scale: Scale::uniform(30.0),
    color: [1.0, 1.0, 1.0, 1.0],
    ..Section::default()
};

let bounds = glyph_brush.pixel_bounds(section).unwrap();

glyph_brush.queue(Section {
    bounds: (bounds.width() as f32, bounds.height() as f32),
    ..section
});

Which produces:

Is this a bug, or am I wrong when I assume the returned pixel_bounds should be compatible with Section::bounds? I will investigate a bit and try to find the issue.

[alexheretic]

pixel_bounds are indeed not compatible with layout bounds.

pixel_bounds describes the minimum box that can hold the rendered result.

The box is a conservative whole number pixel rectangle that can contain the section.

screen_position & bounds however relate to the layout, none of the built-in layouts have an origin the same as the top-left pixel (I guess unless the first glyph is a square matching the ascent).

This is why it has the 'pixel_' prefix, and relates closely to rusttype's pixel_bounding_box.

[alexheretic]

It might be cool to have something like a layout_bounds method, that would return a box in terms of glyph advance-width & descent from the baseline. You'd be able to use this for drawing box backgrounds around dynamic text.

Right now you can access this info using the glyphs iterator.

[alexheretic]

layout_bounds would be useful actually, not that I've been misusing pixel_bounds myself as a lazy way to calculate tooltip backgrounds, who said that?!

I'm testing out this for layout-bound calculation:

use glyph_brush::{rusttype::*, *};
use std::borrow::Cow;

pub trait ExtGlyphCruncher {
    fn layout_bounds<'a, S>(&mut self, section: S) -> Option<Rect<f32>>
    where
        S: Into<Cow<'a, VariedSection<'a>>>;
}

impl<'font, T> ExtGlyphCruncher for T
where
    T: GlyphCruncher<'font>,
{
    #[inline]
    fn layout_bounds<'a, S>(&mut self, section: S) -> Option<Rect<f32>>
    where
        S: Into<Cow<'a, VariedSection<'a>>>,
    {
        self.glyphs(section).fold(None, |b, glyph| {
            if let Some(font) = glyph.font() {
                let hm = glyph.unpositioned().h_metrics();
                let vm = font.v_metrics(glyph.scale());
                let pos = glyph.position();
                let lbound = Rect {
                    min: point(pos.x - hm.left_side_bearing, pos.y - vm.ascent),
                    max: point(pos.x + hm.advance_width, pos.y - vm.descent),
                };

                b.map(|b| {
                    let min_x = b.min.x.min(lbound.min.x);
                    let max_x = b.max.x.max(lbound.max.x);
                    let min_y = b.min.y.min(lbound.min.y);
                    let max_y = b.max.y.max(lbound.max.y);
                    Rect { min: point(min_x, min_y), max: point(max_x, max_y) }
                })
                .or_else(|| Some(lbound))
            } else {
                b
            }
        })
    }
}

[hecrj]

pixel_bounds are indeed not compatible with layout bounds.

I see! My bad.

It might be cool to have something like a layout_bounds method, that would return a box in terms of glyph advance-width & descent from the baseline.

Right, I need this to measure text nodes when computing UI layout. I will give it a shot! It looks like we will need to use PositionedGlyph::font and PositionedGlyph::id, then use Font::glyph, and then finally Glyph::get_data? Is there an easier way to obtain the advance-width... Sees new comment

Oh, you just commented! 😅 🎉 I don't know much about rusttype and font layouting, but it looks reasonable to me.

[alexheretic]

Yeah I think it should work ok. Should work with any layout as it's glyph based. I'll have to add some benches to see how well it performs. It also needs decent docs, as the distinction could be confusing.

I'll add this method when I get a chance, probably later on today.

[hecrj]

I'll add this method when I get a chance, probably later on today.

No hurries! Let me know if I can help. And thank you! :)

[alexheretic]

Ok I've added this with 7c0eb23. I ended up calling it glyph_bounds, as "layout bounds" is already used to refer to the specified section bounds (ie the maximum bounds of a section).

[hecrj]

Amazing! Thank you again for your insanely quick support!

[alexheretic]

No worries, I was kind of "in the area" anyway, just avoiding the issue. I'll make a new release tomorrow.

[hecrj]

I just wanted to say that I've implemented the fix on my end and it's working like a charm. Thank you!