[HarfBuzz] HarfBuzz 1.0 API; the message you were hoping would never come

Thu Jan 23 15:29:16 PST 2014

2014/1/23 Behdad Esfahbod <behdad at behdad.org>

> On 14-01-23 03:54 PM, Konstantin Ritt wrote:
> > Hi guys,
> >
> > There is yet another issue left out of the scope:
> > hb_position_t is a typedef of int32_t, you know, and to avoid
> float-to-integer
> > truncation the proposed solution was to specify scale factor shifted
> left.
> > For example to make all results in 26.6 float format, scale shall be
> shifted 6
> > bits left and then the result must be divided to 64.0 to get the original
> > float (well, almost original but 6 digit after the point is quite enough
> for
> > all use-cases).
>
> So far so good.
>
>
> > However, the scale factor applied to results in OT shaper only,
>
> The OT shaper doesn't know about the scale factor whatsoever.  And it
> shouldn't need to.
>

IIUC, OT shaper uses em_scale (int16_t v, int scale), which is `return
(hb_position_t) (v * (int64_t) scale / face->get_upem ());`

>
>
> > the other
> > shapers doesn't respect the scale factor and truncation may occur there
> (i.e.
> > double advance = ..; pos->x_advance = advance;). This also leads to
> unexpected
> > results since 1) pos->x_advance / 64.0 ~= 0 and 2)
> > CTFontCreateWithGraphicsFont (face_data->cg_font, font->y_scale, NULL,
> NULL)
> > falls back to pointSize=12 when y_scale is out of bounds. (BTW, what
> ppem is
> > for, then?)
>
> Then this is a bug that should be fixed.  We should just use font->y_scale
> /
> 64. here, and scale back the results accordingly.
>

But if you set scale to upem, font->y_scale / 64 won't give you a correct
pixelSize.
That's the dilemma -- if scale is aimed to mean "pixelSize", then the
behavior looks unified but em_scale() above and all other shapers would
return truncated result (note: not rounded); if it could be any value, like
`pixelSize * 64` in my case, then the behavior certainly differers from
shaper to shaper.

>
>
> > I'd propose change hb_position_t to be in 26.6 float format everywhere
> and
> > explicitly mention that in the docs.
>
> The 26.6 is NOT enforced and is NOT a limitation of the code.  For example,
> you can do all layout in "font space" by setting scale to upem.
>

Yes. But making it enforced breaks nothing, however solves the truncation
issue once and for all.
I mean it is still possible to do all layout in "font space" by setting
scale to upem, the only difference is that that output values are 26.6
floats rather than ints. 26 bits for integrals won't cause overflows for
calculations in design metrics.

>
>
> > This would guarantee an expected/unified
> > results for any shaper, w/o having to do some tricky scaling.
>
> That tricky scaling shouldn't be needed.  If it is, it's a bug we should
> fix.
>

At least, I can not avoid using it w/o getting truncated results.

>
>
> > We could also want to introduce an API to fine-tune the hinting
> preference, so
> > that all metrics would be either in design units or in hinted advances
> (after
> > a brief review, it looks like it isn't possible to achieve that on
> client side
> > only).
>
> It is.  But you need to know what you are using first and what you have
> set up
> to return in your font callbacks.  Right?
>

If I understand the code correctly, Graphite2 backend always returns pixel
advances based on hinted glyph advances in the font (gr_slot_advance_X()
docs say so). I don't know if setting scale to upem would force it do all
layout in design metrics.

IMO, HB needs to support all 3 cases in a unified way and let the user an
ability to force metrics somehow: 1. shaper works in design metrics; 2.
shaper works in scaled/hinted metrics (default for i.e. Uniscribe and for
Graphite2 in current backend implementation); 3. shaper works in
scaled/unhinted metrics (default for i.e. CoreText). Currently, case 3
isn't supported due to truncation issue and it seems like case 2 isn't
supported because truncation isn't rounding and the advance value should be
rounded before it affects other metrics (what HB-old did in the absence of
the DesignMetrics flag).

>
> behdad
>
>
> > Correct me if I'm wrong.
> >
> > Best regards,
> > Konstantin
> >
> > 2014/1/6 Konstantin Ritt <ritt.ks at gmail.com <mailto:ritt.ks at gmail.com>>
> >
> >
> >     2014/1/6 Khaled Hosny <khaledhosny at eglug.org <mailto:
> khaledhosny at eglug.org>>
> >
> >         Another thing that I’d like to see sorted before the API is
> finalised,
> >         is reverse mapping of output glyphs to input characters with
> proper
> >         handling of combing marks. I feel we should keep the current
> cluster
> >         stuff since it might have its uses (I’m not sure though) and
> introduce a
> >         separate way for that mapping. Either way, I think a more
> indicative
> >         name would be better (it took me sometime to understand what
> those
> >         clusters are for and they mislead me quit a bit when I tried to
> use
> >         HarfBuzz for the first time).
> >
> >         Regards,
> >         Khaled
> >
> >
> >     Oh, I thought I'm the only one who felt into that trouble :)
> >
> >     Here is a result of my tries and fails:
> >     [code]
> >
> >             ushort *log_clusters = ...;
> >
> >
> >
> >             const uint num_glyphs = hb_buffer_get_length(buffer);
> >
> >
> >
> >             hb_glyph_info_t *infos = hb_buffer_get_glyph_infos(buffer,
> 0);
> >
> >             for (uint i = 0; i < num_glyphs; ++i) {
> >
> >                 log_clusters[i] = infos[i].cluster;
> >
> >
> >
> >                 // ... glyphs[i] = infos[i].codepoint; and so on
> >
> >             }
> >
> >
> >             // adjust clusters
> >
> >             uint glyph_pos = 0;
> >
> >             for (uint i = 0; i < num_characters; ++i) {
> >
> >                 if (i != infos[glyph_pos].cluster) {
> >
> >                     for (uint j = glyph_pos + 1; j < num_glyphs; ++j) {
> >
> >                         if (i <= infos[j].cluster) {
> >
> >                             if (i == infos[j].cluster)
> >
> >                                 glyph_pos = j;
> >
> >                             break;
> >
> >                         }
> >
> >                     }
> >
> >                 }
> >
> >                 log_clusters[i] = glyph_pos;
> >
> >             }
> >
> >     [/code]
> >
> >     You could agree that the latter part is not obvious.
> >     And except of bringing some inconvenience to the user, lack of
> reverse
> >     mapping API also hits the performance a bit since we can not avoid
> this
> >     loop with "if (num_glyphs != num_characters)".
> >
> >     Regards,
> >     Konstantin
> >
> >
>
> --
> behdad
> http://behdad.org/
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/harfbuzz/attachments/20140124/ba20ed3d/attachment-0001.html>