Bug 76705

Created attachment 96479 [details]
Desired display as per Uniscribe

Created attachment 96480 [details]
Unexpected rendering by Harfbuzz 0.9.27

Fixed.  Please test.

src/hb-ot-shape-complex-indic.cc |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

New commits:
commit 828e109c7aac3389cc3b89ea1f13388aefb63804
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Fri Apr 18 16:53:34 2014 -0700

    [indic] Fix-up zero-context matching
    
    commit b5a0f69e47ace468b06e21cf069a18ddcfcf6064
    Author: Behdad Esfahbod <behdad@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.

diff --git a/src/hb-ot-shape-complex-indic.cc b/src/hb-ot-shape-complex-indic.cc
index 942e7b9..ee63164 100644
--- a/src/hb-ot-shape-complex-indic.cc
+++ b/src/hb-ot-shape-complex-indic.cc
@@ -557,8 +557,8 @@ data_create_indic (const hb_ot_shape_plan_t *plan)
   indic_plan->virama_glyph = (hb_codepoint_t) -1;
 
   /* 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;
   indic_plan->rphf.init (&plan->map, HB_TAG('r','p','h','f'), zero_context);
   indic_plan->pref.init (&plan->map, HB_TAG('p','r','e','f'), zero_context);
   indic_plan->blwf.init (&plan->map, HB_TAG('b','l','w','f'), zero_context);

Verified OK with various related conjuncts. Examples: ല്ല ബ്ല മ്പ്ലോ.  Did not test for any other regressions.