Providing a get_glyph_extents like API about ttf-parser HOT 20 CLOSED

I don't like complex methods. There are too many ways you can implement this and it will be too confusing for a user.

You can easily write a wrapper for this.

from ttf-parser.

I don't like complex methods.

Sure

There are too many ways you can implement this and it will be too confusing for a user.

Yes please, how can I achieve it without performance cost?

from ttf-parser.

Can you link the hb_font_get_glyph_extents impementation in the harfbuzz? I can't find it because of hb_font_funcs_t...

from ttf-parser.

Here it is https://github.com/harfbuzz/harfbuzz/blob/82545c5e2ba2067f2eb117c7358ed6d5b50ca942/src/hb-ot-font.cc#L178-L201

from ttf-parser.

Thanks. Found it. Qt Creator can't open hb properly, which annoys me a lot...

Looks like this method is very hb-specific. This is how it's implemented in rustybuzz:

hb_bool_t hb_ot_get_glyph_extents(hb_font_t *font, void *font_data, hb_codepoint_t glyph, hb_glyph_extents_t *extents)
{
    hb_glyph_bbox_t param;
    const auto ret = rb_ot_get_glyph_bbox(font->ttf_parser, glyph, font->x_ppem, &param);
    if (ret) {
        if (param.min_x >= param.max_x) {
            extents->width = 0;
            extents->x_bearing = 0;
        } else {
            extents->x_bearing = font->em_scalef_x(param.min_x);
            extents->width = font->em_scalef_x(param.max_x - param.min_x);
        }
        if (param.min_y >= param.max_y) {
            extents->height = 0;
            extents->y_bearing = 0;
        } else {
            extents->y_bearing = font->em_scalef_y(param.max_y);
            extents->height = font->em_scalef_y(param.min_y - param.max_y);
        }
    }

    // TODO Hook up side-bearings variations.
    return ret;
}

#[no_mangle]
pub extern "C" fn rb_ot_get_glyph_bbox(ttf_parser_data: *const ffi::rb_ttf_parser_t, glyph: u32, mut x_ppem: u32,
                                       extents: *mut ffi::hb_glyph_bbox_t) -> i32 {
    let font = ttf_parser_from_raw(ttf_parser_data);
    let glyph_id = GlyphId(u16::try_from(glyph).unwrap());

    if x_ppem == 0 {
        x_ppem = std::u16::MAX as u32;
    }

    if let Some(img) = font.glyph_raster_image(glyph_id, x_ppem as u16) {
        // HarfBuzz also supports only PNG.
        if img.format == ttf_parser::RasterImageFormat::PNG {
            let scale = font.units_per_em().unwrap() as f32 / img.pixels_per_em as f32;
            unsafe {
                *extents = ffi::hb_glyph_bbox_t {
                    x_min: (img.x as f32 * scale).round() as i16,
                    y_min: (img.y as f32 * scale).round() as i16,
                    x_max: ((img.x as f32 + img.width as f32) * scale).round() as i16,
                    y_max: ((img.y as f32 + img.height as f32) * scale).round() as i16,
                };
            }

            return 1;
        }
    }

    match font.glyph_bounding_box(glyph_id) {
        Some(bbox) => unsafe {
            *extents = ffi::hb_glyph_bbox_t {
                x_min: bbox.x_min,
                y_min: bbox.y_min,
                x_max: bbox.x_max,
                y_max: bbox.y_max,
            };

            1
        }
        _ => 0,
    }
}

So in short, I guess you can use ttfp_get_glyph_bbox for benchmarks. It not exactly the same, since ttf-parser doesn't care about font size, but still.

from ttf-parser.

Oh sorry for not founding it, thanks!

from ttf-parser.

Ok, something quickly revealed on numbers harfbuzz/harfbuzz#2510 ttfp_get_glyph_bbox apparently doesn't consider for variation, can that be fixed you think?

from ttf-parser.

It does. You have to use ttfp_set_variation beforehand.

from ttf-parser.

Hmm, I am applying https://github.com/harfbuzz/harfbuzz/blob/2e49be7977aee689aab1cc458e8b20be9df36dd1/perf/perf-extents.hh#L47 but unlike https://github.com/harfbuzz/harfbuzz/blob/2e49be7977aee689aab1cc458e8b20be9df36dd1/perf/perf-draw.hh#L117 I don't see the difference somehow :/

from ttf-parser.

Are you testing gvar or CFF2? Because for gvar, ttfp_get_glyph_bbox ignores variation for some reason. Try using ttfp_outline_glyph instead. It also returns a bbox.

from ttf-parser.

Looks like a ttf-parser bug. And what is more interesting, is that it doesn't affect hb tests at all.

from ttf-parser.

Are you testing gvar or CFF2? Because for gvar, ttfp_get_glyph_bbox ignores variation for some reason. Try using ttfp_outline_glyph instead. It also returns a bbox.

Thanks, will use in the meanwhile

Looks like a ttf-parser bug. And what is more interesting, is that it doesn't affect hb tests at all.

Correct, maybe not lots of var tests, a fix would be very nice. Thanks

from ttf-parser.

Fixed.

from ttf-parser.

Fix is confirmed, thank you very much!

In case you want to test also,

meson build -Dexperimental_api=true -Dbenchmark=true -Dexperimental_api=true -Doptimization=2 && ninja -Cbuild && build/perf/perf

on top of harfbuzz/harfbuzz#2510

glyf numbers are a bit worrying, maybe this shouldn't be applied until a coordinate is given anyway.

extents/glyf - ot - Comfortaa                        23618 ns      23595 ns      28663
extents/glyf - ft - Comfortaa                       659447 ns     658750 ns       1054
extents/glyf - tp - Comfortaa                      1353911 ns    1352579 ns        511

from ttf-parser.

Does Comfortaa a variable font?

from ttf-parser.

Yes, but coordinates weren't given on that case.

from ttf-parser.

1. ttf-parser doesn't check for default var coordinates right now.
2. ttf-parser doesn't support phantom points, like hb. Therefore it always outlines the whole glyph.

So this is an expected result. I will create corresponding issues.

from ttf-parser.

Also there is HVAR/VVAR harfbuzz/harfbuzz#1389 (comment) not all the fonts have it but would be nice if implemented here before going the glyphs tables.

from ttf-parser.

Implemented where? In ttfp_get_glyph_bbox? There are separate methods for that. I don't want adding to much logic on top of TrueType tables.

from ttf-parser.

oh, that's ok

from ttf-parser.