Bug 20779

Summary: text improperly positioned - scaled font glyph device transform misapplied
Product: cairo Reporter: Tony Garland <tony3>
Component: freetype font backendAssignee: David Turner <david>
Status: RESOLVED NOTOURBUG QA Contact: cairo-bugs mailing list <cairo-bugs>
Severity: normal    
Priority: medium CC: freedesktop, tony3
Version: 1.8.6   
Hardware: ARM   
OS: other   
Whiteboard:
i915 platform: i915 features:
Attachments: Original patchset from David to allow creation of freetype fonts without using fontconfig.
Patchset which includes changes to work with cairo 1.8.6.

Description Tony Garland 2009-03-20 14:08:04 UTC
When attempting to draw a truetype font an an embedded project where Cairo was ported to run on an ARM9, the glyphs are drawn using _cairo_scaled_font_show_glyphs().  Unfortunately, the device_transform offset which is used to round glyph locations to the nearest pixel appears to subtract the transform offsets when they should be added.

This typically causes the text to be positioned at a higher Y coordinate than should be the case leaving the glyph to be rendered outside of the bounding extent for the entire text string which causes the text to be completely clipped and to not appear.

The call path leading to this behavior is:

main
cairo_show_text
_cairo_gstate_show_text_glyphs
_cairo_surface_fallback_show_glyphs
_clip_and_composite
_cairo_surface_old_show_glyphs
_cairo_scaled_font_show_glyphs

I'm not all that well-versed in cairo (my first time sleuthing around in the code in am embedded debugger). But everywhere else that the device_transform.{x0,y0} values appear to be used, they are considered to be positive offsets and ADDED to the absolute x,y location.  In this one case, they are being SUBTRACTED.

The fix, on our system, is to change the subtractions to additions (see below).

As to why this isn't causing serious text rendering problems for everyone using Cairo 1.8.6 I'm not sure--perhaps the way the font is being rendered in our system differs from typical somehow.

One thing we are doing differently: we are operating without fontconfig support (we have no OS or real file system) by applying some patches that were originally found here:

http://david.freetype.org/cairo/

which were mentioned here:

http://lists.cairographics.org/archives/cairo/2007-February/009838.html

I don't see off-hand how these patches (and their hand-corrections since cairo has moved a bit since they were originally published) would have an effect upon this bug.

Here's the patch which corrects the problem on our system:


*** a/cairo-scaled-font.c       Fri Mar 20 13:33:25 2009
--- b/cairo-scaled-font.c       Fri Mar 20 13:33:40 2009
*************** _cairo_scaled_font_show_glyphs (cairo_sc
*** 1919,1926 ****

        /* round glyph locations to the nearest pixel */
        /* XXX: FRAGILE: We're ignoring device_transform scaling here. A bug? */
!       x = _cairo_lround (glyphs[i].x - glyph_surface->base.device_transform.x0);
!       y = _cairo_lround (glyphs[i].y - glyph_surface->base.device_transform.y0);

        _cairo_pattern_init_for_surface (&glyph_pattern, &glyph_surface->base);

--- 1919,1926 ----

        /* round glyph locations to the nearest pixel */
        /* XXX: FRAGILE: We're ignoring device_transform scaling here. A bug? */
!       x = _cairo_lround (glyphs[i].x + glyph_surface->base.device_transform.x0);
!       y = _cairo_lround (glyphs[i].y + glyph_surface->base.device_transform.y0);

        _cairo_pattern_init_for_surface (&glyph_pattern, &glyph_surface->base);
Comment 1 Behdad Esfahbod 2009-03-21 13:26:19 UTC
That's because of your patch.  Perhaps because the patch was written before the following commit.  At any rate, cairo master compiles without fontconfig.

commit 31f5aafa36015ee6ea8ff769c2e1d5841f62642f
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Mon Feb 5 20:46:48 2007 -0500

    Fix device_offset misuse in all glyph surface uses
    
    Seems like all over the code, we have been using negated device_offset
    values for glyph surfaces.  Here is all the math(!):
    
    A device_transform converts from device space (a conceptual space) to
    surface space.  For simple cases of translation only, it's called a
    device_offset and is public API (cairo_surface_[gs]et_device_offset).
    A possibly better name for those functions could have been
    cairo_surface_[gs]et_origing.  So, that's what they do: they set where
    the device-space origin (0,0) is in the surface.  If the origin is inside
    the surface, device_offset values are positive.  It may look like this:
    
    Device space:
          (-x,-y) <-- negative numbers
             +----------------+
             |      .         |
             |      .         |
             |......(0,0) <---|-- device-space origin
             |                |
             |                |
             +----------------+
                      (width-x,height-y)
    
    Surface space:
           (0,0) <-- surface-space origin
             +---------------+
             |      .        |
             |      .        |
             |......(x,y) <--|-- device_offset
             |               |
             |               |
             +---------------+
                       (width,height)
    
    In other words: device_offset is the coordinates of the device-space
    origin relative to the top-left of the surface.
    
    We use device offsets in a couple of places:
    
      - Public API: To let toolkits like Gtk+ give user a surface that
        only represents part of the final destination (say, the expose
        area), but has the same device space as the destination.  In these
        cases device_offset is typically negative.  Example:
    
             application window
             +---------------+
             |      .        |
             | (x,y).        |
             |......+---+    |
             |      |   | <--|-- expose area
             |      +---+    |
             +---------------+
    
        In this case, the user of cairo API can set the device_space on
        the expose area to (-x,-y) to move the device space origin to that
        of the application window, such that drawing in the expose area
        surface and painting it in the application window has the same
        effect as drawing in the application window directly.  Gtk+ has
        been using this feature.
    
      - Glyph surfaces: In most font rendering systems, glyph surfaces
        have an origin at (0,0) and a bounding box that is typically
        represented as (x_bearing,y_bearing,width,height).  Depending on
        which way y progresses in the system, y_bearing may typically be
        negative (for systems similar to cairo, with origin at top left),
        or be positive (in systems like PDF with origin at bottom left).
        No matter which is the case, it is important to note that
        (x_bearing,y_bearing) is the coordinates of top-left of the glyph
        relative to the glyph origin.  That is, for example:
    
        Scaled-glyph space:
    
          (x_bearing,y_bearing) <-- negative numbers
             +----------------+
             |      .         |
             |      .         |
             |......(0,0) <---|-- glyph origin
             |                |
             |                |
             +----------------+
                      (width+x_bearing,height+y_bearing)
    
        Note the similarity of the origin to the device space.  That is
        exactly how we use the device_offset to represent scaled glyphs:
        to use the device-space origin as the glyph origin.
    
    Now compare the scaled-glyph space to device-space and surface-space
    and convince yourself that:
    
        (x_bearing,y_bearing) = (-x,-y) = - device_offset
    
    That's right.  If you are not convinced yet, contrast the definition
    of the two:
    
        "(x_bearing,y_bearing) is the coordinates of top-left of the
         glyph relative to the glyph origin."
    
        "In other words: device_offset is the coordinates of the
         device-space origin relative to the top-left of the surface."
    
    and note that glyph origin = device-space origin.
    
    So, that was the bug.  Fixing it removed lots of wonders and magic
    negation signs.
    
    The way I discovered the bug was that in the user-font API, to make
    rendering the glyph from meta-surface to an image-surface work I had
    to do:
    
        cairo_surface_set_device_offset (surface, -x_bearing, -y_bearing);
        _cairo_meta_surface_replay (meta_surface, surface);
        cairo_surface_set_device_offset (surface, x_bearing, y_bearing);
    
    This suggested that the use of device_offset for glyph origin is
    different from its use for rendering with meta-surface.  This reminded
    me of the large comment in the xlib backend blaming XRender for having
    weird glyph space, and of a similar problem I had in the PS backend
    for bitmap glyph positioning (see d47388ad759b0a1a0869655a87d9b5eb6ae2445d)
    
    ...those are all fixed now.

Comment 2 Tony Garland 2009-03-21 15:29:55 UTC
Hello Behdad,

Being a novice at cairo, I am not in a position to understand all the subtleties of the description which you helpfully posted.  But I did want to clarify that I don't think the operation I'm seeing is related to the patch--at least it isn't at all intuitive that it would be.

I've carefully looked through all the differences introduced by the patch. The patch creates some new files, but makes only very minor modifications to existing cairo files--none of them related to the use of the device_transform or glyph rendering.

The purpose of the patch is to make it easier to make use of freetype fonts without having fontconfig available to correlate font characteristics to a specific font file.  I think this idea is mentioned in the roadmap:

    cairo 1.12 (2010?)
    • David Turner's cairo-ft rewrite. (behdad)

The particular file in question (cairo-scaled-font.c) is not affected by the patch I referred to.  Thus, it is the unmodified file from version 1.8.6 of cairo-scaled-font.c that I had to modify to correct the problem.

Perhaps there are some subtleties that go beyond my meager knowledge of cairo that would explain why I had to modify the code. But, as I say, the patch differences are extremely minor and have no direct bearing on the device_transform use for glyphs.  It simply uses cairo with freetype to open and read a truetype file and the resulting glyph origins wind up being applied in the reverse direction than they should so the text outside of the clip region.

So, in summary: a) I don't see how the patch would cause the problem I'm seeing; b) the change had to be made in the latest version of cairo-scaled-font.c which was not modified by the patch.

Comment 3 Behdad Esfahbod 2009-03-23 16:20:53 UTC
Tony,

  * If you still think it's a cairo bug, please reproduce it with unpatched master.

  * The fact that cairo-scaled-font.c is not affected by the patch is irrelevant.  Your patched cairo-ft backend is setting device offset wrongly.
Comment 4 Behdad Esfahbod 2009-03-23 16:21:41 UTC
And as I said, cairo from master branch can be compiled without fontconfig now.  So you should drop the patch and use master.
Comment 5 Tony Garland 2009-03-23 16:26:02 UTC
I'm not in a position to be able to work with the unpatched cairo on my embedded system.  Nor does the patch modify any code anywhere having to do with device_transform: it is extremely simple.

I reported the bug because I thought it might be of interest. If you are convinced it is not an issue I leave that to you. I've pointed out the possible problem and am not in a position to pursue it at a greater depth.
Comment 6 Behdad Esfahbod 2009-03-23 16:29:47 UTC
(In reply to comment #5)
> I'm not in a position to be able to work with the unpatched cairo on my
> embedded system.  Nor does the patch modify any code anywhere having to do with
> device_transform: it is extremely simple.

Well, maybe you can attach the patch you are using then?



> I reported the bug because I thought it might be of interest. If you are
> convinced it is not an issue I leave that to you. I've pointed out the possible
> problem and am not in a position to pursue it at a greater depth.
> 

Comment 7 Tony Garland 2009-03-24 10:04:32 UTC
Created attachment 24205 [details] [review]
Original patchset from David to allow creation of freetype fonts without using fontconfig.

This is the original patchset obtained from http://david.freetype.org/cairo/

Since cairo had changed since the patchset was made, some of the patches required hand-adjustments to cleanly compile in Cairo 1.8.6 -- I'll attach the resulting patchset which incorporates those edits next.
Comment 8 Tony Garland 2009-03-24 10:12:30 UTC
Created attachment 24206 [details]
Patchset which includes changes to work with cairo 1.8.6.

This patchset includes additional changes made to adapt the previous patchset for compilation with cairo 1.8.6.  The majority of the code in both patchsets represents new files created by the original patchset, although in this patchset cairo-type1-subset.c has changes as well.

I am not well-versed at Cairo's use of fonts. This is my first time using Cairo (building for an embedded target without GNU tools) so I attempted to work around the compilation issues I encountered as best I could to achieve a working solution for our project.

If, after glancing through the final patch, you feel it is pretty obvious that I somehow broke the use of device_transform with glyphs, then please use your judgment and don't waste more time on it.  It could well be there is some subtlety as a side effect to my changes that I'm not qualified to detect, but nothing directly affecting glyph lookup or rendering seems to be involved.
Comment 9 Tony Garland 2009-03-24 10:16:15 UTC
I've added two patchsets.

The first one is the original patch from several years back which allows the use of freetype fonts decoupled from a dependence upon fontconfig.

The second patchset shows further changes made to cleanly compile the first patches with cairo 1.8.6.

If, on glancing at the patches, you feel there is enough going on that I somehow introduced a subtlety that would cause glyph offsets (device_transform) to be handled incorrectly, then please don't waste more time on it.  But it isn't at all obvious to me how the changes would affect that area.  (Then again, I'm not versed in the cairo code base like you are.)
Comment 10 Behdad Esfahbod 2009-03-24 12:51:37 UTC
These snippets are from cairo-ft.c in master:
...
    /*
     * Note: the font's coordinate system is upside down from ours, so the
     * Y coordinate of the control box needs to be negated.  Moreover, device
     * offsets are position of glyph origin relative to top left while xMin
     * and yMax are offsets of top left relative to origin.  Another negation.
     */
    cairo_surface_set_device_offset (&(*surface)->base,
                                     floor (-(double) cbox.xMin / 64.0),
                                     floor (+(double) cbox.yMax / 64.0));
...
    /*
     * Note: the font's coordinate system is upside down from ours, so the
     * Y coordinate of the control box needs to be negated.  Moreover, device
     * offsets are position of glyph origin relative to top left while
     * bitmap_left and bitmap_top are offsets of top left relative to origin.
     * Another negation.
     */
    cairo_surface_set_device_offset (&(*surface)->base,
                                     -glyphslot->bitmap_left,
                                     +glyphslot->bitmap_top);
...
    cairo_surface_set_device_offset (&(*surface)->base,
                                     _cairo_lround (origin_x),
                                     _cairo_lround (origin_y));
...


These ones are in the patch you are using:
...
+    /*
+     * Note: the font's coordinate system is upside down from ours, so the
+     * Y coordinate of the control box needs to be negated.
+     */
+    cairo_surface_set_device_offset (&(*surface)->base,
+                                    floor ((double) cbox.xMin / 64.0),
+                                    floor (-(double) cbox.yMax / 64.0));
...
+    /*
+     * Note: the font's coordinate system is upside down from ours, so the
+     * Y coordinate of the control box needs to be negated.
+     */
+    cairo_surface_set_device_offset (&(*surface)->base,
+                                    glyphslot->bitmap_left,
+                                    -glyphslot->bitmap_top);
...
+    cairo_surface_set_device_offset (&(*surface)->base,
+                                    - _cairo_lround (origin_x),
+                                    - _cairo_lround (origin_y));
...

Just search for cairo_surface_set_device_offset in your patch and negate both arguments and voila.
Comment 11 Tony Garland 2009-03-24 12:58:12 UTC
Behdad,

Thank you so much for continuing to look at this and for your patience with my novice questions.  It looks like you've fingered the problem precisely and I now know what to do to fix it the correct way.

I am encouraged to know that upcoming releases of cairo will make it easier to use fonts in environments which lack a file system like that assumed by fontconfig.

Thanks again--and especially with the patience you showed as I continued interacting without seeing how the problem was patch-related.
Comment 12 Behdad Esfahbod 2009-03-24 13:04:57 UTC
Tony, you're welcome.  I'm glad you're finding cairo useful.

Have fun with cairo, and let us know if we can help more!
behdad

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.