[cairo-commit] 2 commits - src/cairoint.h src/cairo-paginated-surface.c src/cairo-pdf-operators.c src/cairo-surface.c src/test-meta-surface.c src/test-paginated-surface.c

Behdad Esfahbod behdad at kemper.freedesktop.org
Mon Jun 30 22:04:47 PDT 2008


 src/cairo-paginated-surface.c |   43 ++---------------------------------
 src/cairo-pdf-operators.c     |    8 +++---
 src/cairo-surface.c           |   22 +-----------------
 src/cairoint.h                |    8 ------
 src/test-meta-surface.c       |   50 +++++++++++++++++++++++++++++------------
 src/test-paginated-surface.c  |   51 +++++++++++++++++++++++++++++++-----------
 6 files changed, 83 insertions(+), 99 deletions(-)

New commits:
commit d19d9b414946f668f0c073f39bc5e413cabdb069
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Mon Jun 30 22:13:44 2008 -0400

    [cairo-pdf-operators] Fix backward cluster iteration
    
    It's trickier than it appears...
    
    There still remains the question of whether the loop should iterate
    utf8 forward and glyphs backward (as it does now), or iterate glyphs
    forward and utf8 backward.
    
    Doing utf8 forward has the benefit that glyphs appear in the PDF stream
    in the logical order of text.  That may be a good thing.
    
    Doing glyphs forward has the benefit of most glyphs taking their natural
    width, providing for a more compact PDF output.
    
    Firefox 3 currently calls cairo_show_glyphs() with glyphs going
    right-to-left for RTL runs.  That's like going utf8 forward.
    
    On the other hand, if we are fine doing glyphs backward, then we may as
    well remove the "backward" flag and ask callers of show_text_glyphs to
    produce glyph array going right to left for RTL runs.  The idea behind
    having a backward flag in the first place was to allow keeping utf8 in
    logical order and glyphs in visual left-to-right order.  If we don't
    do that in practice, there's no point in having the flag.  Another design
    idea of the flag was to mark right-to-left runs explicitly so the backend
    can use this information.  In that case we probably better rename it to
    "rtl" as the current semantics make it perfectly fine to set a LTR segment
    with backward flag on: just revers the glyph array...
    
    Anyway, the way to determine the right thing to do is to see how Acrobat
    extracts text for RTL text in logical vs visual order.  Then we can decide
    what to do in the PDF backend and whether the flag is worth keeping.
    Going to experiment with that.

diff --git a/src/cairo-pdf-operators.c b/src/cairo-pdf-operators.c
index ad62cfa..abaa4d0 100644
--- a/src/cairo-pdf-operators.c
+++ b/src/cairo-pdf-operators.c
@@ -1335,10 +1335,12 @@ _cairo_pdf_operators_show_text_glyphs (cairo_pdf_operators_t	  *pdf_operators,
     if (num_clusters > 0) {
 	cur_text = utf8;
 	if (backward)
-	    cur_glyph = glyphs + num_glyphs - 1;
+	    cur_glyph = glyphs + num_glyphs;
 	else
 	    cur_glyph = glyphs;
 	for (i = 0; i < num_clusters; i++) {
+	    if (backward)
+		cur_glyph -= clusters[i].num_glyphs;
 	    status = _cairo_pdf_operators_emit_cluster (pdf_operators,
 							cur_text,
 							clusters[i].num_bytes,
@@ -1347,9 +1349,7 @@ _cairo_pdf_operators_show_text_glyphs (cairo_pdf_operators_t	  *pdf_operators,
 							backward,
 							scaled_font);
 	    cur_text += clusters[i].num_bytes;
-	    if (backward)
-		cur_glyph -= clusters[i].num_glyphs;
-	    else
+	    if (!backward)
 		cur_glyph += clusters[i].num_glyphs;
 	}
     } else {
commit c29029ce436709fc3f3ce651a92eebcee11f3203
Author: Behdad Esfahbod <behdad at behdad.org>
Date:   Mon Jun 30 22:11:48 2008 -0400

    Remove _cairo_surface_show_glyphs() in favor of _cairo_surface_show_text_glyphs()

diff --git a/src/cairo-paginated-surface.c b/src/cairo-paginated-surface.c
index f25e792..9deefc6 100644
--- a/src/cairo-paginated-surface.c
+++ b/src/cairo-paginated-surface.c
@@ -595,43 +595,6 @@ _cairo_paginated_surface_fill (void			*abstract_surface,
 				tolerance, antialias);
 }
 
-static cairo_int_status_t
-_cairo_paginated_surface_show_glyphs (void			*abstract_surface,
-				      cairo_operator_t		 op,
-				      cairo_pattern_t		*source,
-				      cairo_glyph_t		*glyphs,
-				      int			 num_glyphs,
-				      cairo_scaled_font_t	*scaled_font,
-				      int			*remaining_glyphs)
-{
-    cairo_paginated_surface_t *surface = abstract_surface;
-    cairo_int_status_t status;
-
-    /* Optimize away erasing of nothing. */
-    if (surface->page_is_blank && op == CAIRO_OPERATOR_CLEAR)
-	return CAIRO_STATUS_SUCCESS;
-
-    surface->page_is_blank = FALSE;
-
-    /* Since this is a "wrapping" surface, we're calling back into
-     * _cairo_surface_show_glyphs from within a call to the same.
-     * Since _cairo_surface_show_glyphs acquires a mutex, we release
-     * and re-acquire the mutex around this nested call.
-     *
-     * Yes, this is ugly, but we consider it pragmatic as compared to
-     * adding locking code to all 18 surface-backend-specific
-     * show_glyphs functions, (which would get less testing and likely
-     * lead to bugs).
-     */
-    CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
-    status = _cairo_surface_show_glyphs (surface->meta, op, source,
-					 glyphs, num_glyphs,
-					 scaled_font);
-    CAIRO_MUTEX_LOCK (scaled_font->mutex);
-
-    return status;
-}
-
 static cairo_bool_t
 _cairo_paginated_surface_has_show_text_glyphs (void *abstract_surface)
 {
@@ -663,8 +626,8 @@ _cairo_paginated_surface_show_text_glyphs (void			    *abstract_surface,
     surface->page_is_blank = FALSE;
 
     /* Since this is a "wrapping" surface, we're calling back into
-     * _cairo_surface_show_glyphs from within a call to the same.
-     * Since _cairo_surface_show_glyphs acquires a mutex, we release
+     * _cairo_surface_show_text_glyphs from within a call to the same.
+     * Since _cairo_surface_show_text_glyphs acquires a mutex, we release
      * and re-acquire the mutex around this nested call.
      *
      * Yes, this is ugly, but we consider it pragmatic as compared to
@@ -719,7 +682,7 @@ static const cairo_surface_backend_t cairo_paginated_surface_backend = {
     _cairo_paginated_surface_mask,
     _cairo_paginated_surface_stroke,
     _cairo_paginated_surface_fill,
-    _cairo_paginated_surface_show_glyphs,
+    NULL, /* show_glyphs */
     _cairo_paginated_surface_snapshot,
     NULL, /* is_similar */
     NULL, /* reset */
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index 6e6f7fc..bceb7a3 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -2135,24 +2135,6 @@ _cairo_surface_get_extents (cairo_surface_t         *surface,
     return status;
 }
 
-cairo_status_t
-_cairo_surface_show_glyphs (cairo_surface_t	*surface,
-			    cairo_operator_t	 op,
-			    cairo_pattern_t	*source,
-			    cairo_glyph_t	*glyphs,
-			    int			 num_glyphs,
-			    cairo_scaled_font_t	*scaled_font)
-{
-    return _cairo_surface_show_text_glyphs (surface,
-					    op,
-					    source,
-					    NULL, 0,
-					    glyphs, num_glyphs,
-					    NULL, 0,
-					    FALSE,
-					    scaled_font);
-}
-
 cairo_bool_t
 _cairo_surface_has_show_text_glyphs (cairo_surface_t	    *surface)
 {
@@ -2301,8 +2283,8 @@ _cairo_surface_show_text_glyphs (cairo_surface_t	    *surface,
 }
 
 /* XXX: Previously, we had a function named _cairo_surface_show_glyphs
- * with not-so-useful semantics. We've now got a new
- * _cairo_surface_show_glyphs with the proper semantics, and its
+ * with not-so-useful semantics. We've now got a
+ * _cairo_surface_show_text_glyphs with the proper semantics, and its
  * fallback still uses this old function (which still needs to be
  * cleaned up in terms of both semantics and naming). */
 cairo_status_t
diff --git a/src/cairoint.h b/src/cairoint.h
index bd33c44..a377a6b 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -1747,14 +1747,6 @@ _cairo_surface_fill (cairo_surface_t	*surface,
 		     double		 tolerance,
 		     cairo_antialias_t	 antialias);
 
-cairo_private cairo_status_t
-_cairo_surface_show_glyphs (cairo_surface_t	*surface,
-			    cairo_operator_t	 op,
-			    cairo_pattern_t	*source,
-			    cairo_glyph_t	*glyphs,
-			    int			 num_glyphs,
-			    cairo_scaled_font_t	*scaled_font);
-
 cairo_private cairo_bool_t
 _cairo_surface_has_show_text_glyphs (cairo_surface_t	    *surface);
 
diff --git a/src/test-meta-surface.c b/src/test-meta-surface.c
index a6f8b97..5425c0c 100644
--- a/src/test-meta-surface.c
+++ b/src/test-meta-surface.c
@@ -255,14 +255,26 @@ _test_meta_surface_fill (void			*abstract_surface,
 				tolerance, antialias);
 }
 
+static cairo_bool_t
+_test_meta_surface_has_show_text_glyphs (void *abstract_surface)
+{
+    test_meta_surface_t *surface = abstract_surface;
+
+    return _cairo_surface_has_show_text_glyphs (surface->meta);
+}
+
 static cairo_int_status_t
-_test_meta_surface_show_glyphs (void			*abstract_surface,
-				cairo_operator_t	 op,
-				cairo_pattern_t		*source,
-				cairo_glyph_t		*glyphs,
-				int			 num_glyphs,
-				cairo_scaled_font_t	*scaled_font,
-				int			*remaining_glyphs)
+_test_meta_surface_show_text_glyphs (void		    *abstract_surface,
+				     cairo_operator_t	     op,
+				     cairo_pattern_t	    *source,
+				     const char		    *utf8,
+				     int		     utf8_len,
+				     cairo_glyph_t	    *glyphs,
+				     int		     num_glyphs,
+				     const cairo_text_cluster_t *clusters,
+				     int		     num_clusters,
+				     cairo_bool_t	     backward,
+				     cairo_scaled_font_t    *scaled_font)
 {
     test_meta_surface_t *surface = abstract_surface;
     cairo_int_status_t status;
@@ -270,8 +282,8 @@ _test_meta_surface_show_glyphs (void			*abstract_surface,
     surface->image_reflects_meta = FALSE;
 
     /* Since this is a "wrapping" surface, we're calling back into
-     * _cairo_surface_show_glyphs from within a call to the same.
-     * Since _cairo_surface_show_glyphs acquires a mutex, we release
+     * _cairo_surface_show_text_glyphs from within a call to the same.
+     * Since _cairo_surface_show_text_glyphs acquires a mutex, we release
      * and re-acquire the mutex around this nested call.
      *
      * Yes, this is ugly, but we consider it pragmatic as compared to
@@ -280,14 +292,18 @@ _test_meta_surface_show_glyphs (void			*abstract_surface,
      * lead to bugs).
      */
     CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
-    status = _cairo_surface_show_glyphs (surface->meta, op, source,
-					 glyphs, num_glyphs,
-					 scaled_font);
+    status = _cairo_surface_show_text_glyphs (surface->meta, op, source,
+					      utf8, utf8_len,
+					      glyphs, num_glyphs,
+					      clusters, num_clusters,
+					      backward,
+					      scaled_font);
     CAIRO_MUTEX_LOCK (scaled_font->mutex);
 
     return status;
 }
 
+
 static cairo_surface_t *
 _test_meta_surface_snapshot (void *abstract_other)
 {
@@ -358,6 +374,12 @@ static const cairo_surface_backend_t test_meta_surface_backend = {
     _test_meta_surface_mask,
     _test_meta_surface_stroke,
     _test_meta_surface_fill,
-    _test_meta_surface_show_glyphs,
-    _test_meta_surface_snapshot
+    NULL, /* show_glyphs */
+    _test_meta_surface_snapshot,
+    NULL, /* is_similar */
+    NULL, /* reset */
+    NULL, /* fill_stroke */
+    NULL, /* create_solid_pattern_surface */
+    _test_meta_surface_has_show_text_glyphs,
+    _test_meta_surface_show_text_glyphs
 };
diff --git a/src/test-paginated-surface.c b/src/test-paginated-surface.c
index 6b85a13..48c7308 100644
--- a/src/test-paginated-surface.c
+++ b/src/test-paginated-surface.c
@@ -232,14 +232,26 @@ _test_paginated_surface_fill (void			*abstract_surface,
 				tolerance, antialias);
 }
 
+static cairo_bool_t
+_test_paginated_surface_has_show_text_glyphs (void *abstract_surface)
+{
+    test_paginated_surface_t *surface = abstract_surface;
+
+    return _cairo_surface_has_show_text_glyphs (surface->target);
+}
+
 static cairo_int_status_t
-_test_paginated_surface_show_glyphs (void			*abstract_surface,
-				     cairo_operator_t		 op,
-				     cairo_pattern_t		*source,
-				     cairo_glyph_t		*glyphs,
-				     int			 num_glyphs,
-				     cairo_scaled_font_t	*scaled_font,
-				     int			*remaining_glyphs)
+_test_paginated_surface_show_text_glyphs (void			    *abstract_surface,
+					  cairo_operator_t	     op,
+					  cairo_pattern_t	    *source,
+					  const char		    *utf8,
+					  int			     utf8_len,
+					  cairo_glyph_t		    *glyphs,
+					  int			     num_glyphs,
+					  const cairo_text_cluster_t *clusters,
+					  int			     num_clusters,
+					  cairo_bool_t		     backward,
+					  cairo_scaled_font_t	    *scaled_font)
 {
     test_paginated_surface_t *surface = abstract_surface;
     cairo_int_status_t status;
@@ -248,8 +260,8 @@ _test_paginated_surface_show_glyphs (void			*abstract_surface,
 	return CAIRO_STATUS_SUCCESS;
 
     /* Since this is a "wrapping" surface, we're calling back into
-     * _cairo_surface_show_glyphs from within a call to the same.
-     * Since _cairo_surface_show_glyphs acquires a mutex, we release
+     * _cairo_surface_show_text_glyphs from within a call to the same.
+     * Since _cairo_surface_show_text_glyphs acquires a mutex, we release
      * and re-acquire the mutex around this nested call.
      *
      * Yes, this is ugly, but we consider it pragmatic as compared to
@@ -258,13 +270,18 @@ _test_paginated_surface_show_glyphs (void			*abstract_surface,
      * lead to bugs).
      */
     CAIRO_MUTEX_UNLOCK (scaled_font->mutex);
-    status = _cairo_surface_show_glyphs (surface->target, op, source,
-					 glyphs, num_glyphs, scaled_font);
+    status = _cairo_surface_show_text_glyphs (surface->target, op, source,
+					      utf8, utf8_len,
+					      glyphs, num_glyphs,
+					      clusters, num_clusters,
+					      backward,
+					      scaled_font);
     CAIRO_MUTEX_LOCK (scaled_font->mutex);
 
     return status;
 }
 
+
 static void
 _test_paginated_surface_set_paginated_mode (void			*abstract_surface,
 					    cairo_paginated_mode_t	 mode)
@@ -309,8 +326,16 @@ static const cairo_surface_backend_t test_paginated_surface_backend = {
     _test_paginated_surface_mask,
     _test_paginated_surface_stroke,
     _test_paginated_surface_fill,
-    _test_paginated_surface_show_glyphs,
-    NULL /* snapshot */
+    NULL, /* show_glyphs */
+
+    NULL, /* snapshot */
+    NULL, /* is_similar */
+    NULL, /* reset */
+    NULL, /* fill_stroke */
+    NULL, /* create_solid_pattern_surface */
+
+    _test_paginated_surface_has_show_text_glyphs,
+    _test_paginated_surface_show_text_glyphs
 };
 
 static const cairo_paginated_surface_backend_t test_paginated_surface_paginated_backend = {


More information about the cairo-commit mailing list