[HarfBuzz] To zero or not to zero

[Behdad Esfahbod]

Hi Jonathan,

In this commit:

commit b5a0f69e47ace468b06e21cf069a18ddcfcf6064
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Thu Oct 17 18:04:23 2013 +0200

    [indic] Pass zero-context=false to would_substitute for newer scripts

    For scripts without an old/new spec distinction, use zero-context=false.
    This changes behavior in Sinhala / Khmer, but doesn't seem to regress.
    This will be useful and used in Javanese.


The *intention* was to change zero-context from true to false for scripts that
don't have old-vs-new specs.  However, checking the code, looks like we
essentially change zero-context to always be true; ie. we only changed things
for old-spec, and we broke them.  That's what causes this bug:

  https://bugs.freedesktop.org/show_bug.cgi?id=76705

The root of the bug is here:

  /* Use zero-context would_substitute() matching for new-spec of the main
   * Indic scripts, but not for old-spec or scripts with one spec only. */
  bool zero_context = indic_plan->config->has_old_spec ||
!indic_plan->is_old_spec;

Note that is_old_spec itself is:

  indic_plan->is_old_spec = indic_plan->config->has_old_spec &&
((plan->map.chosen_script[0] & 0x000000FF) != '2');

It's easy to show that zero_context is now always true.  What we really meant was:

  bool zero_context = indic_plan->config->has_old_spec &&
!indic_plan->is_old_spec;

Ie, "&&" instead of "||".  We made this change supposedly to make Javanese
work.  But apparently we got it working regardless!  So I'm going to fix this
to only change the logic for old-spec and not touch other cases.  Please
check.  Would be great to see updated test runs.

   /* Use zero-context would_substitute() matching for new-spec of the main
-   * Indic scripts, but not for old-spec or scripts with one spec only. */
-  bool zero_context = indic_plan->config->has_old_spec ||
!indic_plan->is_old_spec;
+   * Indic scripts, and scripts with one spec only, but not for old-specs. */
+  bool zero_context = !indic_plan->is_old_spec;


Thanks,
-- 
behdad
http://behdad.org/