Part of Bug 355782 – Misaligned extents in pango

2006-09-13  Behdad Esfahbod  <behdad@gnome.org>

        Part of Bug 355782 – Misaligned extents in pango

        * pango/pango-layout.c (cluster_width): Rewrite based on
        is_cluster_start instead of accessing log_clusters directly.  This is
        more robust as next_cluster uses that, so the two match now.

        * pango/pango-layout.c (update_cluster): Remove the
        cluster_start_index parameter and use iter->cluster_start instead.

        * pango/pango-layout.c (pango_layout_iter_copy): Fix typo in copying
        iter->cluster_width.

        * pango/pango-layout.c (pango_layout_iter_get_cluster_extents): Add
        an assert to make sure iter->cluster_width is correctly set.

        * pango/pango-layout.c (pango_layout_iter_get_char_extents): Remove
        assert that was readded above to check more cases.

8 files changed, 93 insertions, 31 deletions

diff --git a/ChangeLog b/ChangeLog
index 17e27050..c48d1cd9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2006-09-13  Behdad Esfahbod  <behdad@gnome.org>
+
+	Part of Bug 355782 – Misaligned extents in pango
+
+	* pango/pango-layout.c (cluster_width): Rewrite based on
+	is_cluster_start instead of accessing log_clusters directly.  This is
+	more robust as next_cluster uses that, so the two match now.
+	
+	* pango/pango-layout.c (update_cluster): Remove the
+	cluster_start_index parameter and use iter->cluster_start instead.
+
+	* pango/pango-layout.c (pango_layout_iter_copy): Fix typo in copying
+	iter->cluster_width.
+	
+	* pango/pango-layout.c (pango_layout_iter_get_cluster_extents): Add
+	an assert to make sure iter->cluster_width is correctly set.
+
+	* pango/pango-layout.c (pango_layout_iter_get_char_extents): Remove
+	assert that was readded above to check more cases.
+
 2006-09-12  Behdad Esfahbod  <behdad@gnome.org>
 
 	Bug 355605 – hexbox with 0000 in it
diff --git a/configure.in b/configure.in
index 39132fec..e6610f6e 100644
--- a/configure.in
+++ b/configure.in
@@ -394,7 +394,7 @@ fi
 #
 # Modules to build
 #
-arabic_modules="arabic-fc"
+arabic_modules="arabic-fc,arabic-lang"
 basic_modules="basic-fc,basic-win32,basic-x,basic-atsui"
 hangul_modules="hangul-fc"
 hebrew_modules="hebrew-fc"
@@ -472,6 +472,7 @@ done
 IFS="$pango_save_ifs"
 
 AM_CONDITIONAL(INCLUDE_ARABIC_FC,	echo $included_modules | egrep '(^|,)arabic-fc($|,)' 	> /dev/null)
+AM_CONDITIONAL(INCLUDE_ARABIC_LANG,	echo $included_modules | egrep '(^|,)arabic-lang($|,)' 	> /dev/null)
 AM_CONDITIONAL(INCLUDE_BASIC_FC,	echo $included_modules | egrep '(^|,)basic-fc($|,)' 	> /dev/null)
 AM_CONDITIONAL(INCLUDE_BASIC_WIN32,	echo $included_modules | egrep '(^|,)basic-win32($|,)' 	> /dev/null)
 AM_CONDITIONAL(INCLUDE_BASIC_X,		echo $included_modules | egrep '(^|,)basic-x($|,)' 	> /dev/null)
@@ -485,6 +486,7 @@ AM_CONDITIONAL(INCLUDE_THAI_FC,		echo $included_modules | egrep '(^|,)thai-fc($|
 AM_CONDITIONAL(INCLUDE_TIBETAN_FC,	echo $included_modules | egrep '(^|,)tibetan-fc($|,)' 	> /dev/null)
 
 AM_CONDITIONAL(DYNAMIC_ARABIC_FC,	echo $dynamic_modules | egrep '(^|,)arabic-fc($|,)' 	> /dev/null)
+AM_CONDITIONAL(DYNAMIC_ARABIC_LANG,	echo $dynamic_modules | egrep '(^|,)arabic-lang($|,)' 	> /dev/null)
 AM_CONDITIONAL(DYNAMIC_BASIC_FC,	echo $dynamic_modules | egrep '(^|,)basic-fc($|,)' 	> /dev/null)
 AM_CONDITIONAL(DYNAMIC_BASIC_WIN32,	echo $dynamic_modules | egrep '(^|,)basic-win32($|,)' 	> /dev/null)
 AM_CONDITIONAL(DYNAMIC_BASIC_X,		echo $dynamic_modules | egrep '(^|,)basic-x($|,)' 	> /dev/null)
diff --git a/modules/arabic/Makefile.am b/modules/arabic/Makefile.am
index ca129b8d..26b232eb 100644
--- a/modules/arabic/Makefile.am
+++ b/modules/arabic/Makefile.am
@@ -21,3 +21,21 @@ pango_arabic_fc_la_LIBADD = $(pangoft2libs)
 pango_arabic_fc_la_SOURCES = $(fc_sources)
 libpango_arabic_fc_la_SOURCES = $(fc_sources)
 libpango_arabic_fc_la_CFLAGS = -DPANGO_MODULE_PREFIX=_pango_arabic_fc
+
+
+if INCLUDE_ARABIC_LANG
+noinst_LTLIBRARIES += libpango-arabic-lang.la
+else
+if DYNAMIC_ARABIC_LANG
+module_LTLIBRARIES += pango-arabic-lang.la
+endif
+endif
+
+lang_sources = \
+	arabic-lang.c
+
+pango_arabic_lang_la_LDFLAGS = -module $(MODULE_LIBTOOL_OPTIONS)
+pango_arabic_lang_la_LIBADD = $(pangolibs)
+pango_arabic_lang_la_SOURCES = $(lang_sources)
+libpango_arabic_lang_la_SOURCES = $(lang_sources)
+libpango_arabic_lang_la_CFLAGS = -DPANGO_MODULE_PREFIX=_pango_arabic_lang
diff --git a/pango/break.c b/pango/break.c
index 7a23ac19..fc9defb7 100644
--- a/pango/break.c
+++ b/pango/break.c
@@ -1488,11 +1488,17 @@ pango_break (const gchar   *text,
   g_return_if_fail (analysis != NULL);
   g_return_if_fail (attrs != NULL);
   
+  g_message ("break");
+
   if (analysis->lang_engine &&
       PANGO_ENGINE_LANG_GET_CLASS (analysis->lang_engine)->script_break)
     PANGO_ENGINE_LANG_GET_CLASS (analysis->lang_engine)->script_break (analysis->lang_engine, text, length, analysis, attrs, attrs_len);
   else
     pango_default_break (text, length, analysis, attrs, attrs_len);
+
+  int i;
+  for (i = 0; i < attrs_len; i++)
+    g_message ("%d %d", attrs[i].is_cursor_position, attrs[i].backspace_deletes_character);
 }
 
 /**
@@ -1629,6 +1635,7 @@ pango_get_log_attrs (const char    *text,
   static guint engine_type_id = 0;
   static guint render_type_id = 0;
   PangoAnalysis analysis = { 0 };
+  PangoScriptIter *iter;
 
   analysis.level = level;
 
@@ -1647,8 +1654,18 @@ pango_get_log_attrs (const char    *text,
       render_type_id = g_quark_from_static_string (PANGO_RENDER_TYPE_NONE);
     }
 
+  g_message ("logattrs %d >>>>>", length);
   lang_map = pango_find_map (language, engine_type_id, render_type_id);
 
+
+  iter = pango_script_iter_new (text, length);
+  do
+    {
+      
+    }
+  while (pango_script_iter_next (iter));
+  pango_script_iter_free (iter);
+
   range_start = text;
   script = pango_script_for_unichar (g_utf8_get_char (text));
   range_engine = (PangoEngineLang*) pango_map_get_engine (lang_map, script);
@@ -1666,14 +1683,14 @@ pango_get_log_attrs (const char    *text,
       g_assert (end - pos < length);
 
       script = pango_script_for_unichar (g_utf8_get_char (pos));
-      analysis.lang_engine =
-        (PangoEngineLang*) pango_map_get_engine (lang_map, script);
+      range_engine = (PangoEngineLang*) pango_map_get_engine (lang_map, script);
 
       if (range_engine != analysis.lang_engine)
         {
           /* Engine has changed; do the breaking for the current range,
            * then start a new range.
            */
+	  g_message ("log break %d %d", chars_broken, chars_in_range);
           pango_break (range_start,
                        pos - range_start,
                        &analysis,
@@ -1683,7 +1700,7 @@ pango_get_log_attrs (const char    *text,
           chars_broken += chars_in_range;
 
           range_start = pos;
-          range_engine = analysis.lang_engine;
+	  analysis.lang_engine = range_engine;
           chars_in_range = 1;
         }
       else
@@ -1699,9 +1716,11 @@ pango_get_log_attrs (const char    *text,
     g_assert (pos == end);
     g_assert (range_engine == analysis.lang_engine);
 
+    g_message ("log final break %d %d", chars_broken, chars_in_range);
     pango_break (range_start,
                  end - range_start,
                  &analysis,
                  log_attrs + chars_broken,
                  attrs_len - chars_broken);
+  g_message ("logattrs <<<<<");
 }
diff --git a/pango/modules.c b/pango/modules.c
index 57b13443..3ec762cd 100644
--- a/pango/modules.c
+++ b/pango/modules.c
@@ -260,6 +260,7 @@ pango_engine_pair_get_engine (PangoEnginePair *pair)
 	  g_type_module_unuse (G_TYPE_MODULE (pair->module));
 	}
 
+      /* TODO: warn once (per id or something) */
       if (!pair->engine)
 	g_printerr ("Failed to load Pango module for id: '%s'", pair->info.id);
     }
diff --git a/pango/pango-engine.h b/pango/pango-engine.h
index 2bd80c92..44ae681a 100644
--- a/pango/pango-engine.h
+++ b/pango/pango-engine.h
@@ -270,7 +270,7 @@ static GType prefix ## _type;						  \
 static void								  \
 prefix ## _register_type (GTypeModule *module)				  \
 {									  \
-  static const GTypeInfo object_info =					  \
+  const GTypeInfo object_info =						  \
     {									  \
       sizeof (name ## Class),						  \
       (GBaseInitFunc) NULL,						  \
diff --git a/pango/pango-layout.c b/pango/pango-layout.c
index 1389e000..9e6b7374 100644
--- a/pango/pango-layout.c
+++ b/pango/pango-layout.c
@@ -4482,15 +4482,17 @@ cluster_width (PangoGlyphString *gs,
 	       int               cluster_start)
 {
   int i;
-  int log_cluster;
-  int width = 0;
+  int width;
 
-  log_cluster = gs->log_clusters[cluster_start];
-  for (i = cluster_start; i < gs->num_glyphs; i++) 
+  width = gs->glyphs[cluster_start].geometry.width;
+  i = cluster_start + 1;
+  while (i < gs->num_glyphs)
     {
-      if (gs->log_clusters[i] != log_cluster)
-	break;
+      if (gs->glyphs[i].attr.is_cluster_start)
+        break;
+      
       width += gs->glyphs[i].geometry.width;
+      i++;
     }
 
   return width;
@@ -4507,12 +4509,11 @@ offset_y (PangoLayoutIter *iter,
   *y += line_ext->baseline;
 }
 
-/* Sets up the iter for the start of a new cluster. cluster_start_index
+/* Sets up the iter for the start of a new cluster. iter->cluster_start
  * is the byte index of the cluster start relative to the run.
  */
 static void 
-update_cluster (PangoLayoutIter *iter,
-		int              cluster_start_index)
+update_cluster (PangoLayoutIter *iter)
 {
   char             *cluster_text;
   PangoGlyphString *gs;
@@ -4530,9 +4531,9 @@ update_cluster (PangoLayoutIter *iter,
        * since logical and visual runs are in the same direction.
        */
       if (iter->next_cluster_glyph < gs->num_glyphs)
-	cluster_length = gs->log_clusters[iter->next_cluster_glyph] - cluster_start_index;
+	cluster_length = gs->log_clusters[iter->next_cluster_glyph] - iter->cluster_start;
       else
-	cluster_length = iter->run->item->length - cluster_start_index;
+	cluster_length = iter->run->item->length - iter->cluster_start;
     }
   else
     {
@@ -4540,16 +4541,16 @@ update_cluster (PangoLayoutIter *iter,
        * visual cluster which is the next logical cluster.
        */
       int i = iter->cluster_start;
-      while (i > 0 && gs->log_clusters[i - 1] == cluster_start_index)
+      while (i > 0 && gs->log_clusters[i - 1] == iter->cluster_start)
 	i--;
 
       if (i == 0)
-	cluster_length = iter->run->item->length - cluster_start_index;
+	cluster_length = iter->run->item->length - iter->cluster_start;
       else
-	cluster_length = gs->log_clusters[i - 1] - cluster_start_index;
+	cluster_length = gs->log_clusters[i - 1] - iter->cluster_start;
     }
 
-  cluster_text = iter->layout->text + iter->run->item->offset + cluster_start_index;
+  cluster_text = iter->layout->text + iter->run->item->offset + iter->cluster_start;
   iter->cluster_num_chars = g_utf8_strlen (cluster_text, cluster_length);
 
   if (iter->ltr)
@@ -4606,7 +4607,7 @@ update_run (PangoLayoutIter *iter,
 
   if (iter->run)
     {
-      update_cluster (iter, iter->run->glyphs->log_clusters[0]);
+      update_cluster (iter);
     }
   else
     {
@@ -4646,7 +4647,6 @@ pango_layout_iter_copy (PangoLayoutIter *iter)
       if (l == iter->line_extents_link)
 	new->line_extents_link = new->line_extents;
     }
-
   new->line_extents = g_slist_reverse (new->line_extents);
   
   new->run_x = iter->run_x;
@@ -4654,7 +4654,7 @@ pango_layout_iter_copy (PangoLayoutIter *iter)
   new->ltr = iter->ltr;
   
   new->cluster_x = iter->cluster_x;
-  new->cluster_width = iter->cluster_x;
+  new->cluster_width = iter->cluster_width;
 
   new->cluster_start = iter->cluster_start;
   new->next_cluster_glyph = iter->next_cluster_glyph;
@@ -4684,7 +4684,8 @@ pango_layout_iter_get_type (void)
  * 
  * Returns an iterator to iterate over the visual extents of the layout.
  * 
- * Return value: the new #PangoLayoutIter.
+ * Return value: the new #PangoLayoutIter that should be freed using
+ *               pango_layout_iter_free().
  **/
 PangoLayoutIter*
 pango_layout_get_iter (PangoLayout *layout)
@@ -4716,11 +4717,11 @@ pango_layout_get_iter (PangoLayout *layout)
   else
     iter->run = NULL;
 
-  iter->line_extents = NULL;  
+  iter->line_extents = NULL;
   pango_layout_get_extents_internal (layout,
                                      NULL,
                                      &iter->logical_rect,
-                                     &iter->line_extents);  
+                                     &iter->line_extents);
 
   iter->line_extents_link = iter->line_extents;
 
@@ -4863,7 +4864,7 @@ next_nonempty_line (PangoLayoutIter *iter,
   return result;
 }
 
-/* Moves to the next non-empty line. If @include_terminators
+/* Moves to the next non-empty run. If @include_terminators
  * is set, the trailing run at the end of a line with an explicit
  * paragraph separator is considered non-empty.
  */
@@ -4917,7 +4918,7 @@ next_cluster_internal (PangoLayoutIter *iter,
     {
       iter->cluster_start = next_start;
       iter->cluster_x += iter->cluster_width;
-      update_cluster(iter, gs->log_clusters[iter->cluster_start]);
+      update_cluster(iter);
 
       return TRUE;
     }
@@ -5106,8 +5107,6 @@ pango_layout_iter_get_char_extents (PangoLayoutIter *iter,
       return;
     }
   
-  g_assert (cluster_rect.width == iter->cluster_width);
-
   x0 = (iter->character_position * cluster_rect.width) / iter->cluster_num_chars;
   x1 = ((iter->character_position + 1) * cluster_rect.width) / iter->cluster_num_chars;
 
@@ -5159,6 +5158,7 @@ pango_layout_iter_get_cluster_extents (PangoLayoutIter *iter,
 
   if (logical_rect)
     {
+      g_assert (logical_rect->width == iter->cluster_width);
       logical_rect->x += iter->cluster_x;
       offset_y (iter, &logical_rect->y);
     }
diff --git a/pango/shape.c b/pango/shape.c
index 96abd21b..fc165fbe 100644
--- a/pango/shape.c
+++ b/pango/shape.c
@@ -44,7 +44,7 @@ pango_shape (const gchar      *text,
              PangoGlyphString *glyphs)
 {
   int i;
-  int last_cluster = -1;
+  int last_cluster;
 
   if (G_LIKELY (PANGO_IS_ENGINE_SHAPE (analysis->shape_engine) && PANGO_IS_FONT (analysis->font)))
     {
@@ -109,6 +109,8 @@ pango_shape (const gchar      *text,
 				 text, length, analysis, glyphs);
     }
 
+  /* make sure last_cluster is invalid */
+  last_cluster = glyphs->log_clusters[0] - 1;
   for (i = 0; i < glyphs->num_glyphs; i++)
     {
      /* Set glyphs[i].attr.is_cluster_start based on log_clusters[]