[HarfBuzz] collect_glyphs() API

[Behdad Esfahbod]

Hi Jonathan,

All should be fixed now.

Cheers,
behdad

On 12-12-31 11:33 AM, Jonathan Kew wrote:
> On 4/12/12 22:21, Behdad Esfahbod wrote:
>> Hi Werner and Jonathan,
>>
>> Just wanted to follow up and bring the new
>> hb_ot_layout_lookup_collect_glyphs() to your attention.
>>
>> The implementation of this API is complete as far as I'm concerned.  HOWEVER,
>> I've done absolutely zero testing.
> 
> I've finally gotten around to experimenting with this in Firefox, and run into
> a couple of issues that mean it doesn't quite work as-is:
> 
> (1) hb_ot_layout_collect_lookups(), called without a specific language tag,
> fails to take account of the DefaultLangSys in the script record(s), which
> means it misses some lookups -- in the (common) case where the script has ONLY
> a DefaultLangSys, it returns no lookups at all.
> 
> A possible fix is to explicitly collect lookups for the DefaultLangSys in
> addition to any specified languages.
> 
> (In _hb_ot_layout_collect_lookups_languages:)
> 
>     /* All languages... */
>     unsigned int count = hb_ot_layout_script_get_language_tags (face,
> table_tag, script_index, 0, NULL, NULL);
>     for (unsigned int language_index = 0; language_index < count;
> language_index++)
>       _hb_ot_layout_collect_lookups_features (face, table_tag, script_index,
> language_index, features, lookup_indexes);
>     /* ...including the DefaultLangSys, which is not in the array of
> LangSysRecords */
>     _hb_ot_layout_collect_lookups_features (face, table_tag, script_index,
> OT::Index::NOT_FOUND_INDEX, features, lookup_indexes);
> 
> (2) The API claims that you can pass NULL for any of the glyph sets.
> Internally, it handles this by using hb_set_get_empty() in place of any NULL
> parameters. However, this breaks because the low-level collect_glyphs()
> functions will then be trying to add glyphs to the (static, const) empty set.
> Ouch. For me, it dies with a protection error because that object is
> read-only, and const_casting it to a non-const hb_set* doesn't magically make
> it writable. But even if it didn't die, it would still be bad because it would
> make the "empty" object no longer empty!
> 
> This applies not only to any NULL glyph-set parameters that the user has
> passed, but also the use of hb_set_get_empty() in the
> hb_collect_glyphs_context_t::recurse() function to replace the user's
> before/input/after sets while recursing.
> 
> This could be fixed at various levels: the hb_set methods that modify a set
> could be made no-ops if the set is hb_set_get_empty(), though this adds an
> extra test to all those low-level operations; or -- better, I think -- the
> collect_glyphs() methods could check for (and skip) NULL sets, so that we
> don't need to use the const "empty" set for this purpose.
> 
> (To avoid the risk of errors like this, maybe hb_set_get_empty should be
> returning a const set?)
> 
>> That has to wait until I have time to do a
>> cmdline tool for all the introspection APIs.  As such, thought I give you a
>> heads up so you can go ahead and test, err, use it.
>>
>> Jonathan, I believe you know how to use the API.
>>
>> Werner, here's how you use the API:
>>
>>    1. Create a new set, call lookups (hb_set_create ()).
>>
>>    2. Collect the index of the lookups you want to process into the set.
>>
>>    3. Create sets for "backtrack", "input", "lookahead", and "output" glyph
>> sets.  They can point to the same object, or some can be NULL.
>>
>>    4. Iterate over the lookup indices, and call collect_glyphs() for each.
>>
>> Note that if you reuse a set, you are responsible for emptying it first.
>>
>> Here is how one iterates over the set:
>>
>>    hb_codepoint_t lookup_index;
>>    for (lookup_index = -1; hb_set_next (lookups, &lookup_index);)
>>        hb_ot_layout_lookup_collect_glyphs (face, HB_OT_TAG_GSUB, lookup_index,
>> ...);
>>
>> I probably should change the set API to use unsigned int instead of
>> hb_codepoint_t, but I'm not going to worry about it that much right now.
>>
>> As for step 2, collecting lookup indices, there are different things you can
>> do.  Based on what you described to me before, I think using
>> hb_ot_layout_collect_lookups() makes most sense for you.  Say, if you want the
>> 'scmp' feature for all scripts and languages, you call:
>>
>>    hb_tag_t features[] = {HB_TAG ('s','c','m','p'), HB_TAG_NONE};
>>    hb_ot_layout_collect_lookups (face, HB_OT_TAG_GSUB, NULL, NULL, features,
>> lookups);
>>
>> This will collect all the lookup indices for all scmp features in all scripts
>> / language systems into the lookups set.  You can of course limit this to
>> specific scripts, etc.
>>
>> Hope that helps,
>>
> 
> 

-- 
behdad
http://behdad.org/