[HarfBuzz] fixing graphite rtl integration

Martin Hosken mhosken at gmail.com
Mon Jun 15 23:45:59 PDT 2015


Dear Behdad,

The enclosed patch fixes rtl integration of graphite in hb-graphite2. It handles the cluster reversals, etc. and has received a modicum of testing. The only differences I see between running text through graphite and through graphite with hb is reordering of ltr text in an rtl run. But I think that difference is correct in that hb doesn't expect to do bidi reordering and assumes things are reordered first or called as sub runs with the correct direction.

Sorry it's a patch, but the outstanding PR sort of made a hiccup in terms of another PR. I can do the necessary jiggery pokery if you would prefer that.

In code terms the patch only affects the code following the cluster analysis, which is unchanged. The first hunk fixes a memory management problem which valgrind spotted. The second hunk is the core of the change.

TIA,
Yours,
Martin

diff --git a/src/hb-graphite2.cc b/src/hb-graphite2.cc
index 6214403..6c2ea24 100644
--- a/src/hb-graphite2.cc
+++ b/src/hb-graphite2.cc
@@ -359,6 +359,7 @@ _hb_graphite2_shape (hb_shape_plan_t    *shape_plan,
     return false;
   }
 
+  buffer->ensure (glyph_count);     // since sizeof(pos) > sizeof(cluster)+sizeof(codepoint)
   scratch = buffer->get_scratch_buffer (&scratch_size);
   while ((DIV_CEIL (sizeof (hb_graphite2_cluster_t) * buffer->len, sizeof (*scratch)) +
 	  DIV_CEIL (sizeof (hb_codepoint_t) * glyph_count, sizeof (*scratch))) > scratch_size)
@@ -433,31 +434,58 @@ _hb_graphite2_shape (hb_shape_plan_t    *shape_plan,
   buffer->len = glyph_count;
   //buffer->swap_buffers ();
 
-  if (HB_DIRECTION_IS_BACKWARD(buffer->props.direction))
-    curradvx = gr_seg_advance_X(seg);
-
-  hb_glyph_position_t *pPos;
-  for (pPos = hb_buffer_get_glyph_positions (buffer, NULL), is = gr_seg_first_slot (seg);
-       is; pPos++, is = gr_slot_next_in_segment (is))
+  if (!HB_DIRECTION_IS_BACKWARD(buffer->props.direction))
   {
-    pPos->x_offset = gr_slot_origin_X (is) - curradvx;
-    pPos->y_offset = gr_slot_origin_Y (is) - curradvy;
-    pPos->x_advance = gr_slot_advance_X (is, grface, grfont);
-    pPos->y_advance = gr_slot_advance_Y (is, grface, grfont);
-    if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction))
-      curradvx -= pPos->x_advance;
-    pPos->x_offset = gr_slot_origin_X (is) - curradvx;
-    if (!HB_DIRECTION_IS_BACKWARD (buffer->props.direction))
+    hb_glyph_position_t *pPos;
+    for (pPos = hb_buffer_get_glyph_positions (buffer, NULL), is = gr_seg_first_slot (seg);
+         is; pPos++, is = gr_slot_next_in_segment (is))
+    {
+      pPos->x_offset = gr_slot_origin_X (is) - curradvx;
+      pPos->y_offset = gr_slot_origin_Y (is) - curradvy;
+      pPos->x_advance = gr_slot_advance_X (is, grface, grfont);
+      pPos->y_advance = gr_slot_advance_Y (is, grface, grfont);
       curradvx += pPos->x_advance;
-    pPos->y_offset = gr_slot_origin_Y (is) - curradvy;
-    curradvy += pPos->y_advance;
-  }
-  if (!HB_DIRECTION_IS_BACKWARD (buffer->props.direction))
+      curradvy += pPos->y_advance;
+    }
     pPos[-1].x_advance += gr_seg_advance_X(seg) - curradvx;
-
-  if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction))
+  }
+  else
+  {
+    //curradvx = gr_seg_advance_X (seg);
+    //curradvy = gr_seg_advance_Y (seg);
+    hb_glyph_position_t *pPos = hb_buffer_get_glyph_positions (buffer, NULL) + buffer->len - 1;
+    const hb_glyph_info_t *info = buffer->info + buffer->len - 1;
+    const hb_glyph_info_t *tinfo;
+    const gr_slot *tis;
+    int currclus = -1;
+    float clusx = 0., clusy = 0.;
+    for (is = gr_seg_last_slot (seg); is; pPos--, info--, is = gr_slot_prev_in_segment (is))
+    {
+      if (info->cluster != currclus)
+      {
+        curradvx += clusx;
+        curradvy += clusy;
+        currclus = info->cluster;
+        clusx = 0.;
+        clusy = 0.;
+        for (tis = is, tinfo = info; tis && tinfo->cluster == currclus; tis = gr_slot_prev_in_segment (tis), tinfo--)
+        {
+          clusx += gr_slot_advance_X (tis, grface, grfont);
+          clusy += gr_slot_advance_Y (tis, grface, grfont);
+        }
+        curradvx += clusx;
+        curradvy += clusy;
+      }
+      pPos->x_advance = gr_slot_advance_X (is, grface, grfont);
+      pPos->y_advance = gr_slot_advance_Y (is, grface, grfont);
+      curradvx -= pPos->x_advance;
+      curradvy -= pPos->y_advance;
+      pPos->x_offset = gr_slot_origin_X (is) - curradvx;
+      pPos->y_offset = gr_slot_origin_Y (is) - curradvy;
+    }
     hb_buffer_reverse_clusters (buffer);
-
+  }
+ 
   if (feats) gr_featureval_destroy (feats);
   gr_seg_destroy (seg);
 


More information about the HarfBuzz mailing list