Re: [cairo] cairo-pdf glyph positioning bug with librsvg

[Behdad Esfahbod <behdad behdad org>]

Ok, I've got a patch that mostly fixes this now.  The bug was simple:
librsvg's Pango use was a mess.  My patch fixes most of the issues, but
one still remains.  In particular:

  - In most places, librsvg was converting from PangoUnits to pixels,
then using that as double.  That is, it was throwing away the fractional
part of the size.  This only shows without metrics-hinting, that is,
when rendering to PDF.  Fixed in the patch.

  - librsvg also was using the ink extents in some places where logical
extents should be used.  Fixed in the patch.

  - librsvg was not updating the pango context to match the cairo_t
before doing measurements.  It only did after measurements where made
and the layout was being painted.  This caused the measurements to be
done with different font options than the actual rendering.  Fixed in
the patch.

  - librsvg apparently breaks the text upon seeing the first non-ASCII
text character.  I have no idea why that is.  I guess something to do
with XML parsing.  This is why the bug looked so ridiculous, because the
text was being rendered as two PangoLayout's, so it looked like it's a
pango issue.  The bug will not be completely fixed until this issue
exists.

CC'ing gtk-i18n-list and Dom.  Is there any better list to discuss
librsvg?


Cheers,

behdad

On Fri, 2008-02-15 at 16:55 -0500, Behdad Esfahbod wrote:
> On Fri, 2008-02-15 at 08:36 -0800, Carl Worth wrote:
> > Here's a bug report I just got that looks worth fixing.
> > 
> > Here's an SVG file:
> > 
> > 	http://cairographics.org/~cworth/images/svgtest.svg
> > 
> > And here's a correct rendering of it with librsvg and cairo's image
> > backend (svg2png):
> > 
> > 	http://cairographics.org/~cworth/images/svgtest.png
> > 
> > And here's an incorrect rendering of it with librsvg and cairo's PDF
> > backend (svg2pdf):
> > 
> > 	http://cairographics.org/~cworth/images/svgtest.pdf
> > 	http://cairographics.org/~cworth/images/svgtest-pdf.png
> > 
> > Ideas? Adrian?
> > 
> > -Carl
> 
> So, we had a bug for this in bugzilla that I could reproduce at the
> time, but a few weeks ago I tried and couldn't.  Looking into it now.
> (Adrian is doing the same right now).
> 
-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759

Index: configure.in
===================================================================
--- configure.in	(revision 1146)
+++ configure.in	(working copy)
@@ -9,7 +9,7 @@ GIO_REQUIRED=2.15.4
 LIBXML_REQUIRED=2.4.7
 CAIRO_REQUIRED=1.2.0
 PANGOFT2_REQUIRED=1.2.0
-PANGOCAIRO_REQUIRED=1.2.0
+PANGOCAIRO_REQUIRED=1.10.0
 
 AC_SUBST(GDK_PIXBUF_REQUIRED)
 AC_SUBST(GLIB_REQUIRED)
Index: rsvg-cairo-draw.c
===================================================================
--- rsvg-cairo-draw.c	(revision 1146)
+++ rsvg-cairo-draw.c	(working copy)
@@ -460,9 +460,11 @@ rsvg_cairo_create_pango_context (RsvgDra
 {
     PangoFontMap *fontmap;
     PangoContext *context;
+    RsvgCairoRender *render = (RsvgCairoRender *) ctx->render;
 
     fontmap = pango_cairo_font_map_get_default ();
     context = pango_cairo_font_map_create_context (PANGO_CAIRO_FONT_MAP (fontmap));
+    pango_cairo_update_context (render->cr, context);
     pango_cairo_context_set_resolution (context, ctx->dpi_y);
     return context;
 }
@@ -472,26 +474,23 @@ rsvg_cairo_render_pango_layout (RsvgDraw
 {
     RsvgCairoRender *render = (RsvgCairoRender *) ctx->render;
     RsvgState *state = rsvg_state_current (ctx);
-    PangoRectangle logical;
+    PangoRectangle ink;
     RsvgBbox bbox;
 
-	_rsvg_cairo_set_text_antialias (render->cr, state->text_rendering_type);
+    _rsvg_cairo_set_text_antialias (render->cr, state->text_rendering_type);
 
     _set_rsvg_affine (render, state->affine);
-    cairo_set_line_width (render->cr, _rsvg_css_normalize_length (&state->stroke_width, ctx, 'h'));
-
-    pango_cairo_update_layout (render->cr, layout);
 
-    pango_layout_get_pixel_extents (layout, NULL, &logical);
+    pango_layout_get_extents (layout, &ink, NULL);
 
     rsvg_bbox_init (&bbox, state->affine);
-    bbox.x = x;
-    bbox.y = y;
-    bbox.w = logical.width;
-    bbox.h = logical.height;
+    bbox.x = x + ink.x / (double)PANGO_SCALE;
+    bbox.y = y + ink.y / (double)PANGO_SCALE;
+    bbox.w = ink.width / (double)PANGO_SCALE;
+    bbox.h = ink.height / (double)PANGO_SCALE;
     bbox.virgin = 0;
 
-    cairo_translate (render->cr, x, y);
+    cairo_move_to (render->cr, x, y);
 
     rsvg_bbox_insert (&render->bbox, &bbox);
 
@@ -513,6 +512,8 @@ rsvg_cairo_render_pango_layout (RsvgDraw
                                        state->stroke,
                                        state->stroke_opacity,
                                        bbox, rsvg_state_current (ctx)->current_color);
+
+	cairo_set_line_width (render->cr, _rsvg_css_normalize_length (&state->stroke_width, ctx, 'h'));
         cairo_stroke (render->cr);
     }
 }
Index: rsvg-text.c
===================================================================
--- rsvg-text.c	(revision 1146)
+++ rsvg-text.c	(working copy)
@@ -550,23 +550,6 @@ rsvg_text_layout_new (RsvgDrawingCtx * c
     return layout;
 }
 
-static void
-rsvg_text_layout_get_offsets (RsvgTextLayout * layout, gint * x, gint * y)
-{
-    PangoRectangle ink;
-    PangoRectangle logical;
-
-    pango_layout_get_pixel_extents (layout->layout, &ink, &logical);
-
-    if (ink.width < 1 || ink.height < 1) {
-        *x = *y = 0;
-        return;
-    }
-
-    *x = MIN (ink.x, logical.x);
-    *y = MIN (ink.y, logical.y);
-}
-
 static FT_Int32
 rsvg_text_layout_render_flags (RsvgTextLayout * layout)
 {
@@ -775,7 +758,6 @@ rsvg_text_layout_render_line (RsvgTextLa
     for (list = line->runs; list; list = list->next) {
         PangoLayoutRun *run = list->data;
 
-        pango_glyph_string_extents (run->glyphs, run->item->analysis.font, NULL, &rect);
         x_off += rsvg_text_layout_render_glyphs (layout,
                                                  run->item->analysis.font, run->glyphs,
                                                  render_func, x + x_off, y, render_data);
@@ -788,13 +770,10 @@ rsvg_text_layout_render (RsvgTextLayout 
                          RsvgTextRenderFunc render_func, gpointer render_data)
 {
     PangoLayoutIter *iter;
-    gint offx, offy;
     gint x, y;
 
-    rsvg_text_layout_get_offsets (layout, &offx, &offy);
-
-    x = offx + layout->x;
-    y = offy + layout->y;
+    x = layout->x;
+    y = layout->y;
 
     x *= PANGO_SCALE;
     y *= PANGO_SCALE;
@@ -802,19 +781,19 @@ rsvg_text_layout_render (RsvgTextLayout 
     iter = pango_layout_get_iter (layout->layout);
 
     if (iter) {
-        PangoRectangle rect;
+        PangoRectangle logical;
         PangoLayoutLine *line;
         gint baseline;
 
         line = pango_layout_iter_get_line (iter);
 
-        pango_layout_iter_get_line_extents (iter, NULL, &rect);
+        pango_layout_iter_get_line_extents (iter, NULL, &logical);
         baseline = pango_layout_iter_get_baseline (iter);
 
         rsvg_text_layout_render_line (layout, line,
-                                      render_func, x + rect.x, y + baseline, render_data);
+                                      render_func, x, y + baseline, render_data);
 
-        layout->x += rect.width / PANGO_SCALE + offx;
+        layout->x += logical.width / (double)PANGO_SCALE;
     }
 
     pango_layout_iter_free (iter);
@@ -866,12 +845,12 @@ rsvg_text_render_text (RsvgDrawingCtx * 
         state = rsvg_state_current (ctx);
         context = ctx->render->create_pango_context (ctx);
         layout = rsvg_text_create_layout (ctx, state, text, context);
-        pango_layout_get_pixel_size (layout, &w, &h);
+        pango_layout_get_size (layout, &w, &h);
         iter = pango_layout_get_iter (layout);
-        baseline = pango_layout_iter_get_baseline (iter) / PANGO_SCALE;
+        baseline = pango_layout_iter_get_baseline (iter) / (double)PANGO_SCALE;
         pango_layout_iter_free (iter);
         ctx->render->render_pango_layout (ctx, layout, *x, *y - baseline);
-        *x += w;
+        *x += w / (double)PANGO_SCALE;
         g_object_unref (layout);
         g_object_unref (context);
     } else {
@@ -885,26 +864,11 @@ rsvg_text_render_text (RsvgDrawingCtx * 
 static gdouble
 rsvg_text_layout_width (RsvgTextLayout * layout)
 {
-    PangoLayoutIter *iter;
-    gint offx, offy;
-
-    rsvg_text_layout_get_offsets (layout, &offx, &offy);
+    gint width;
 
-    iter = pango_layout_get_iter (layout->layout);
-
-    if (iter) {
-        PangoRectangle rect;
-        PangoLayoutLine *line;
-
-        line = pango_layout_iter_get_line (iter);
+    pango_layout_get_size (layout->layout, &width, NULL);
 
-        pango_layout_iter_get_line_extents (iter, NULL, &rect);
-
-        pango_layout_iter_free (iter);
-        return rect.width / PANGO_SCALE + offx;
-    }
-
-    return 0;
+    return width / (double)PANGO_SCALE;
 }
 
 static gdouble