[Vertical] fix vertical gpos

Wow, it took me a few days to find the right fix!

We now set the advance for attached marks to zero, but we
do this in the _finish() state of gpos, so it shouldn't
regress with fonts like DejaVuSansMono that explicitly
decrease the mark advance width to set it to zero.

2 files changed, 103 insertions, 73 deletions

diff --git a/src/hb-ot-layout-gpos-private.hh b/src/hb-ot-layout-gpos-private.hh
index 87e25ff1..d51a2b08 100644
--- a/src/hb-ot-layout-gpos-private.hh
+++ b/src/hb-ot-layout-gpos-private.hh
@@ -873,38 +873,57 @@ struct CursivePosFormat1
     (this+this_record.exitAnchor).get_anchor (c->font, c->buffer->info[i].codepoint, &exit_x, &exit_y);
     (this+next_record.entryAnchor).get_anchor (c->font, c->buffer->info[j].codepoint, &entry_x, &entry_y);
 
-    /* Align the exit anchor of the left/top glyph with the entry anchor of the right/bottom glyph
-     * by adjusting advance of the left/top glyph. */
-    if (HB_DIRECTION_IS_BACKWARD (c->direction))
-    {
-      if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction)))
-	c->buffer->pos[j].x_advance = c->buffer->pos[j].x_offset + entry_x - exit_x;
-      else
-	c->buffer->pos[j].y_advance = c->buffer->pos[j].y_offset + entry_y - exit_y;
-    }
-    else
-    {
-      if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction)))
-	c->buffer->pos[i].x_advance = c->buffer->pos[i].x_offset + exit_x - entry_x;
-      else
-	c->buffer->pos[i].y_advance = c->buffer->pos[i].y_offset + exit_y - entry_y;
+    hb_glyph_position_t *pos = c->buffer->pos;
+
+    hb_position_t d;
+    /* Main-direction adjustment */
+    switch (c->direction) {
+      case HB_DIRECTION_LTR:
+	pos[i].x_advance  =  exit_x + pos[i].x_offset;
+
+	d = entry_x + pos[j].x_offset;
+	pos[j].x_advance -= d;
+	pos[j].x_offset  -= d;
+	break;
+      case HB_DIRECTION_RTL:
+	d = exit_x + pos[i].x_offset;
+	pos[i].x_advance -= d;
+	pos[i].x_offset  -= d;
+
+	pos[j].x_advance  =  entry_x + pos[j].x_offset;
+	break;
+      case HB_DIRECTION_TTB:
+	pos[i].y_advance  =  exit_y + pos[i].y_offset;
+
+	d = entry_y + pos[j].y_offset;
+	pos[j].y_advance -= d;
+	pos[j].y_offset  -= d;
+	break;
+      case HB_DIRECTION_BTT:
+	d = exit_y + pos[i].y_offset;
+	pos[i].y_advance -= d;
+	pos[i].y_offset  -= d;
+
+	pos[j].y_advance  =  entry_y;
+	break;
+      case HB_DIRECTION_INVALID:
+      default:
+	break;
     }
 
-    if  (c->lookup_props & LookupFlag::RightToLeft)
-    {
-      c->buffer->pos[i].cursive_chain() = j - i;
+    /* Cross-direction adjustment */
+    if  (c->lookup_props & LookupFlag::RightToLeft) {
+      pos[i].cursive_chain() = j - i;
       if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction)))
-	c->buffer->pos[i].y_offset = entry_y - exit_y;
+	pos[i].y_offset = entry_y - exit_y;
       else
-	c->buffer->pos[i].x_offset = entry_x - exit_x;
-    }
-    else
-    {
-      c->buffer->pos[j].cursive_chain() = i - j;
+	pos[i].x_offset = entry_x - exit_x;
+    } else {
+      pos[j].cursive_chain() = i - j;
       if (likely (HB_DIRECTION_IS_HORIZONTAL (c->direction)))
-	c->buffer->pos[j].y_offset = exit_y - entry_y;
+	pos[j].y_offset = exit_y - entry_y;
       else
-	c->buffer->pos[j].x_offset = exit_x - entry_x;
+	pos[j].x_offset = exit_x - entry_x;
     }
 
     c->buffer->i = j;
@@ -1500,61 +1519,69 @@ struct GPOS : GSUBGPOS
   DEFINE_SIZE_STATIC (10);
 };
 
+
+static void
+fix_cursive_minor_offset (hb_glyph_position_t *pos, unsigned int i, hb_direction_t direction)
+{
+    unsigned int j = pos[i].cursive_chain();
+    if (likely (!j))
+      return;
+
+    j += i;
+
+    pos[i].cursive_chain() = 0;
+
+    fix_cursive_minor_offset (pos, j, direction);
+
+    if (HB_DIRECTION_IS_HORIZONTAL (direction))
+      pos[i].y_offset += pos[j].y_offset;
+    else
+      pos[i].x_offset += pos[j].x_offset;
+}
+
+static void
+fix_mark_attachment (hb_glyph_position_t *pos, unsigned int i, hb_direction_t direction)
+{
+  if (likely (!(pos[i].attach_lookback())))
+    return;
+
+  unsigned int j = i - pos[i].attach_lookback();
+
+  pos[i].x_advance = 0;
+  pos[i].y_advance = 0;
+  pos[i].x_offset += pos[j].x_offset;
+  pos[i].y_offset += pos[j].y_offset;
+
+  if (HB_DIRECTION_IS_FORWARD (direction))
+    for (unsigned int k = j; k < i; j++) {
+      pos[i].x_offset -= pos[k].x_advance;
+      pos[i].y_offset -= pos[k].y_advance;
+    }
+  else
+    for (unsigned int k = j + 1; k < i + 1; j++) {
+      pos[i].x_offset += pos[k].x_advance;
+      pos[i].y_offset += pos[k].y_advance;
+    }
+}
+
 void
 GPOS::position_finish (hb_buffer_t *buffer)
 {
-  unsigned int i, j;
   unsigned int len;
   hb_glyph_position_t *pos = hb_buffer_get_glyph_positions (buffer, &len);
   hb_direction_t direction = buffer->props.direction;
 
-  /* Handle cursive connections:
-   * First handle all chain-back connections, then handle all chain-forward connections. */
-  if (likely (HB_DIRECTION_IS_HORIZONTAL (direction)))
+  /* Handle cursive connections */
+  for (unsigned int i = 0; i < len; i++)
   {
-    for (j = 0; j < len; j++) {
-      if (pos[j].cursive_chain() < 0)
-	pos[j].y_offset += pos[j + pos[j].cursive_chain()].y_offset;
-    }
-    for (i = len; i > 0; i--) {
-      j = i - 1;
-      if (pos[j].cursive_chain() > 0)
-	pos[j].y_offset += pos[j + pos[j].cursive_chain()].y_offset;
-    }
+    fix_cursive_minor_offset (pos, i, direction);
   }
-  else
-  {
-    for (j = 0; j < len; j++) {
-      if (pos[j].cursive_chain() < 0)
-	pos[j].x_offset += pos[j + pos[j].cursive_chain()].x_offset;
-    }
-    for (i = len; i > 0; i--) {
-      j = i - 1;
-      if (pos[j].cursive_chain() > 0)
-	pos[j].x_offset += pos[j + pos[j].cursive_chain()].x_offset;
-    }
-  }
-
 
   /* Handle attachments */
-  for (i = 0; i < len; i++)
-    if (pos[i].attach_lookback())
-    {
-      unsigned int back = i - pos[i].attach_lookback();
-      pos[i].x_offset += pos[back].x_offset;
-      pos[i].y_offset += pos[back].y_offset;
-
-      if (HB_DIRECTION_IS_BACKWARD (buffer->props.direction))
-	for (j = back + 1; j < i + 1; j++) {
-	  pos[i].x_offset += pos[j].x_advance;
-	  pos[i].y_offset += pos[j].y_advance;
-	}
-      else
-	for (j = back; j < i; j++) {
-	  pos[i].x_offset -= pos[j].x_advance;
-	  pos[i].y_offset -= pos[j].y_advance;
-	}
-    }
+  for (unsigned int i = 0; i < len; i++)
+  {
+    fix_mark_attachment (pos, i, direction);
+  }
 }
 
 
diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc
index d8970a18..f78c29b2 100644
--- a/src/hb-ot-shape.cc
+++ b/src/hb-ot-shape.cc
@@ -184,9 +184,12 @@ hb_ensure_native_direction (hb_ot_shape_context_t *c)
 {
   hb_direction_t direction = c->buffer->props.direction;
 
-  /* XXX vertical */
-  if (HB_DIRECTION_IS_HORIZONTAL (direction) &&
-      direction != hb_script_get_horizontal_direction (c->buffer->props.script))
+  /* TODO vertical:
+   * The only BTT vertical script is Ogham, but it's not clear to me whether OpenType
+   * Ogham fonts are supposed to be implemented BTT or not.  Need to research that
+   * first. */
+  if ((HB_DIRECTION_IS_HORIZONTAL (direction) && direction != hb_script_get_horizontal_direction (c->buffer->props.script)) ||
+      (HB_DIRECTION_IS_VERTICAL   (direction) && direction != HB_DIRECTION_TTB))
   {
     hb_buffer_reverse_clusters (c->buffer);
     c->buffer->props.direction = HB_DIRECTION_REVERSE (c->buffer->props.direction);