<div dir="ltr">2014/1/23 Behdad Esfahbod <span dir="ltr"><<a href="mailto:behdad@behdad.org" target="_blank">behdad@behdad.org</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On 14-01-23 03:54 PM, Konstantin Ritt wrote:<br>
> Hi guys,<br>
><br>
> There is yet another issue left out of the scope:<br>
> hb_position_t is a typedef of int32_t, you know, and to avoid float-to-integer<br>
> truncation the proposed solution was to specify scale factor shifted left.<br>
> For example to make all results in 26.6 float format, scale shall be shifted 6<br>
> bits left and then the result must be divided to 64.0 to get the original<br>
> float (well, almost original but 6 digit after the point is quite enough for<br>
> all use-cases).<br>
<br>
</div>So far so good.<br>
<div class="im"><br>
<br>
> However, the scale factor applied to results in OT shaper only,<br>
<br>
</div>The OT shaper doesn't know about the scale factor whatsoever. And it<br>
shouldn't need to.<br></blockquote><div><br></div><div>IIUC, OT shaper uses em_scale (int16_t v, int scale), which is `return (hb_position_t) (v * (int64_t) scale / face->get_upem ());`</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
> the other<br>
> shapers doesn't respect the scale factor and truncation may occur there (i.e.<br>
> double advance = ..; pos->x_advance = advance;). This also leads to unexpected<br>
> results since 1) pos->x_advance / 64.0 ~= 0 and 2)<br>
> CTFontCreateWithGraphicsFont (face_data->cg_font, font->y_scale, NULL, NULL)<br>
> falls back to pointSize=12 when y_scale is out of bounds. (BTW, what ppem is<br>
> for, then?)<br>
<br>
</div>Then this is a bug that should be fixed. We should just use font->y_scale /<br>
64. here, and scale back the results accordingly.<br></blockquote><div><br></div><div>But if you set scale to upem, font->y_scale / 64 won't give you a correct pixelSize.</div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
> I'd propose change hb_position_t to be in 26.6 float format everywhere and<br>
> explicitly mention that in the docs.<br>
<br>
</div>The 26.6 is NOT enforced and is NOT a limitation of the code. For example,<br>
you can do all layout in "font space" by setting scale to upem.<br></blockquote><div><br></div><div>Yes. But making it enforced breaks nothing, however solves the truncation issue once and for all.</div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
> This would guarantee an expected/unified<br>
> results for any shaper, w/o having to do some tricky scaling.<br>
<br>
</div>That tricky scaling shouldn't be needed. If it is, it's a bug we should fix.<br></blockquote><div><br></div><div>At least, I can not avoid using it w/o getting truncated results.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
> We could also want to introduce an API to fine-tune the hinting preference, so<br>
> that all metrics would be either in design units or in hinted advances (after<br>
> a brief review, it looks like it isn't possible to achieve that on client side<br>
> only).<br>
<br>
</div>It is. But you need to know what you are using first and what you have set up<br>
to return in your font callbacks. Right?<br></blockquote><div><br></div><div>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.</div>
<div><br></div><div>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).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
behdad<br>
<div class="im"><br>
<br>
> Correct me if I'm wrong.<br>
><br>
> Best regards,<br>
> Konstantin<br>
><br>
</div>> 2014/1/6 Konstantin Ritt <<a href="mailto:ritt.ks@gmail.com">ritt.ks@gmail.com</a> <mailto:<a href="mailto:ritt.ks@gmail.com">ritt.ks@gmail.com</a>>><br>
><br>
><br>
> 2014/1/6 Khaled Hosny <<a href="mailto:khaledhosny@eglug.org">khaledhosny@eglug.org</a> <mailto:<a href="mailto:khaledhosny@eglug.org">khaledhosny@eglug.org</a>>><br>
<div class=""><div class="h5">><br>
> Another thing that I’d like to see sorted before the API is finalised,<br>
> is reverse mapping of output glyphs to input characters with proper<br>
> handling of combing marks. I feel we should keep the current cluster<br>
> stuff since it might have its uses (I’m not sure though) and introduce a<br>
> separate way for that mapping. Either way, I think a more indicative<br>
> name would be better (it took me sometime to understand what those<br>
> clusters are for and they mislead me quit a bit when I tried to use<br>
> HarfBuzz for the first time).<br>
><br>
> Regards,<br>
> Khaled<br>
><br>
><br>
> Oh, I thought I'm the only one who felt into that trouble :)<br>
><br>
> Here is a result of my tries and fails:<br>
> [code]<br>
><br>
> ushort *log_clusters = ...;<br>
><br>
><br>
><br>
> const uint num_glyphs = hb_buffer_get_length(buffer);<br>
><br>
><br>
><br>
> hb_glyph_info_t *infos = hb_buffer_get_glyph_infos(buffer, 0);<br>
><br>
> for (uint i = 0; i < num_glyphs; ++i) {<br>
><br>
> log_clusters[i] = infos[i].cluster;<br>
><br>
><br>
><br>
> // ... glyphs[i] = infos[i].codepoint; and so on<br>
><br>
> }<br>
><br>
><br>
> // adjust clusters<br>
><br>
> uint glyph_pos = 0;<br>
><br>
> for (uint i = 0; i < num_characters; ++i) {<br>
><br>
> if (i != infos[glyph_pos].cluster) {<br>
><br>
> for (uint j = glyph_pos + 1; j < num_glyphs; ++j) {<br>
><br>
> if (i <= infos[j].cluster) {<br>
><br>
> if (i == infos[j].cluster)<br>
><br>
> glyph_pos = j;<br>
><br>
> break;<br>
><br>
> }<br>
><br>
> }<br>
><br>
> }<br>
><br>
> log_clusters[i] = glyph_pos;<br>
><br>
> }<br>
><br>
> [/code]<br>
><br>
> You could agree that the latter part is not obvious.<br>
> And except of bringing some inconvenience to the user, lack of reverse<br>
> mapping API also hits the performance a bit since we can not avoid this<br>
> loop with "if (num_glyphs != num_characters)".<br>
><br>
> Regards,<br>
> Konstantin<br>
><br>
><br>
<br>
</div></div><span class=""><font color="#888888">--<br>
behdad<br>
<a href="http://behdad.org/" target="_blank">http://behdad.org/</a><br>
</font></span></blockquote><br></div>