462798 - don't pass cairo_font_options_t* between system and moz cairo (wrong hint style)

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Karl Tomlinson (:karlt)]

In gfxPangoFonts.cpp:

    const cairo_font_options_t *options =
        gdk_screen_get_font_options(gdk_screen_get_default());

    cairo_ft_font_options_substitute(options, aPattern);

This would be a problem, for example, if moz cairo added support for lcd_filter options, but system cairo did not have such support.

I think the best thing to do here would be to ensure that
cairo_ft_font_options_substitute is the function from system cairo.
aPattern is an FcPattern* which can then be safely passed between cairos.

Comment 1

[Karl Tomlinson (:karlt)]

We'll need to fix this.
Ubuntu have an extra font_option field that is not at then end of the struct:

+ struct _cairo_font_options {
+     cairo_antialias_t antialias;
+     cairo_subpixel_order_t subpixel_order;
++    cairo_lcd_filter_t lcd_filter;
+     cairo_hint_style_t hint_style;
+     cairo_hint_metrics_t hint_metrics;
+ };

http://archive.ubuntu.com/ubuntu/pool/main/c/cairo/cairo_1.8.0-0ubuntu1.diff.gz

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I'm going to say that this is ubuntu breakage; they should not be adding things to structs, much less not adding them to the end.  We should try to fix this, but I have no idea how, at least not easily.

Comment 3

[Karl Tomlinson (:karlt)]

(In reply to comment #2)
> I'm going to say that this is ubuntu breakage; they should not be adding things
> to structs, much less not adding them to the end.

I neglected to mention that this struct is private to cairo, so adding things to the struct is reasonable because no-one else should use it.

The bug is with us (me) passing a handle to a private object from one cairo to another.

> We should try to fix this, but I have no idea how, at least not easily.

I think we can fix it, but there may be some messy #undef, #define stuff to distinguish functions from each cairo (especially if we can get the fix for bug 403513).

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: use cairo_ft_font_options_substitute from system cairo

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 351588 [details] [diff] [review]
use cairo_ft_font_options_substitute from system cairo

Duh, that works -- I wasn't thinking about the symbol renaming.  I guess that since all that's involved is a FcPattern, then there's no overlap.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: use cairo_ft_font_options_substitute from system cairo

Had to add NS_VISIBILITY_DEFAULT to the cairo_ft_font_options_substitute declaration (for some compilers).

Pushed to mozilla-central:

http://hg.mozilla.org/mozilla-central/rev/ad37b3de22b4
http://hg.mozilla.org/mozilla-central/rev/86d0e7bb0672

Not on 1.9.1 yet.

Comment 7

[Kemcy BEST]

I'm not seeing the changes using:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20081205 Minefield/3.2a1pre - Build ID: 20081205235816

Even the Ubuntu build get the same results! Maybe my modifications made to conf.d/ directory and fontconfig-config.config messed up something? Does this landing require more local tuning? Thanks

Xft.antialias:	1
Xft.dpi:	96
Xft.hinting:	1
Xft.hintstyle:	hintslight
Xft.rgba:	rgb

Comment 8

[Kemcy BEST]

If I remove (or move to another directory) 10-antialias.conf,
10-hinting-medium.conf, 10-hinting.conf, 10-no-sub-pixel.conf,
11-lcd-filter-lcddefault.conf, and 53-monospace-lcd-filter.conf from
/etc/fonts/conf.d, I can see a better hinting yet not fully as expected:
http://img210.imageshack.us/my.php?image=afterpatchnk8.png
Pango-view in this case:
http://img525.imageshack.us/img525/1493/pangoviewxt7.png
And if I do so, my 3 version get the same "better" effect (3.0.4, ubuntu build 3.1 and official 3.2a1).

If I keep these files there, 3 version get the same bad effect. But yesterday at least 3.0.4 get the correct hinting, I'm trying to lock down the changes which messed my fontconfig up.

Comment 9

[Kemcy BEST]

If I make change to :/etc/fonts/conf.avail/10-hinting-medium.conf like this
  <match target="font">
	<test name="hintstyle" qual="all"><int>-1</int></test>
	<edit name="hintstyle" mode="assign"><const>hintmedium</const></edit>
  </match>
</fontconfig>

Things get better as indicated
http://img210.imageshack.us/img210/842/afterpatchnk8.png
Yet with a slightly bad hinting color. But why 10-hinting-medium.conf? I'm in fact using 'slight' as hinting style, and if I ln -s 10-hinting-slight.conf to conf.d/ nothing happens.

Comment 10

[Karl Tomlinson (:karlt)]

kmc:  It's hard for me to guess what is happening here.

The png images have been scaled using a filter so they are hard to deconvolve.
Images at the natural size would be easier to analyse.

> my 3 version get the same "better" effect

Use the menu Help -> About <Product> to check that you are looking at the versions that you think you are.

To check what your fontconfig settings are doing, you can use

fc-match -v sans-serif:pixelsize=13:antialias=1:hinting=1:hintstyle=hintslight:rgba=rgb

The parameters here are from the xrdb output in comment 7.
You can check the output from fc-match to see if fontconfig setting have changed these properties.

The latest Firefox 3.2a1 and pango-view with --backend=xft should be using rendering options equivalent to the output from fc-match.
Firefox 3.0.4 and pango-view with --backend=cairo use a more complicated algorithm.

If "ubuntu build 3.1" is built with --enable-system-cairo (check about:buildconfig") then it should be using similar rendering properties to Firefox 3.2a1, but see the next paragraph.

Note that builds with --enable-system-cairo will be using the Ubuntu cairo, which is patched to render using different lcd filters, which can reduce the effect of color fringing, especially when the hintstyle is less than full.  Official mozilla builds use a cairo that does not have this patch.  This is tracked in bug 404637.

(If not built with --enable-system-cairo, "ubuntu build 3.1" will be suffering from the bug reported in comment 1.)

Comment 11

[Kemcy BEST]

Thanks a lot for your effort, one last post in this bug (as I found similar symptom in bug 404637.

My fc-match gives this, (why Tahoma?)

fc-match -v sans-serif:pixelsize=13:antialias=1:hinting=1:hintstyle=hintslight:rgba=rgb
Pattern has 30 elts (size 32)
	family: "Tahoma"(s)
	familylang: "en"(s)
	style: "Normal"(s)
	stylelang: "ca"(s)
	fullname: "Tahoma"(s)
	fullnamelang: "en"(s)
	slant: 0(i)(s)
	weight: 80(i)(s)
	width: 100(i)(s)
	pixelsize: 13(f)(s)
	foundry: "microsoft"(s)
	antialias: FcTrue(w)
	hintstyle: 1(i)(s)
	hinting: FcTrue(w)
	verticallayout: FcFalse(s)
	autohint: FcFalse(s)
	globaladvance: FcTrue(s)
	file: "/usr/share/fonts/truetype/tahoma.ttf"(s)
	index: 0(i)(s)
	outline: FcTrue(s)
	scalable: FcTrue(s)
	rgba: 1(i)(s)
	charset: 0000: 00000000 ffffffff ffffffff 7fffffff 00000000 ffffffff ffffffff ffffffff
	0001: ffffffff ffffffff ffffffff ffffffff 00048000 00818003 00000000 fc00cff0
	0002: 0f000000 00000300 02000000 00000000 00040000 00000000 3f0002c0 00000000
	0003: ffffffff ffffffff 00007fff 40000007 ffffd7f0 fffffffb 00007fff 00000000
	0004: ffffffff ffffffff ffffffff 00000000 3ccf0000 0f0fc00c 03000000 00000300
	0005: 00000000 00000000 00000000 00000000 00000000 ffff0000 ffff000f 001f07ff
	0006: 88001000 07fffffe 003fffff ffff3fff ffffffff ffffffff ffffffff 7fff3fff
	000e: fffffffe 87ffffff 0fffffff 00000000 00000000 00000000 00000000 00000000
	001e: ffffffff ffffffff ffffffff ffffffff 0fffffff ffffffff ffffffff 03ffffff
	001f: 3f3fffff ffffffff aaff3f3f 3fffffff ffffffff ffdfffff efcfffdf 7fdcffff
	0020: 7fb8f000 560d0047 00000010 80000000 00000000 0003ffff 00000000 00000000
	0021: 00480020 00004044 78000000 00000000 00000000 00000000 00000000 00000000
	0022: 46268044 00000800 00000100 00000031 00000000 00000000 00000000 00000000
	0025: 00000000 00000000 00000000 00000000 00000000 00000c02 00008400 00000040
	0026: 00000000 00000000 00000000 00008000 00000000 00000000 00000000 00000000
	00e8: 0100003e 04000000 00000000 00000000 00000000 00000000 00000000 00000000
	00f0: fffffff6 0003ffff 00000000 00000000 00000000 00000000 00000000 00000000
	00f7: 37ffffff 00000000 00000000 00000000 00000000 00000000 00000000 00000000
	00fb: 00000006 5f7ffc01 ffffffdb ffffffff ffffffff 0003ffff fff80000 f00000ff
	00fc: 00000000 00000000 c0000000 00000007 00000000 00000000 00000000 00000000
	00fd: 00000000 c0000000 00000000 00000000 00000000 00000000 00000000 00040000
	00fe: 00000000 00000000 00000000 00000000 ffffffff ffffffff ffffffff 1fffffff
	00ff: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 10000000
(s)
	lang: aa|af|ar|ast|ava|ay|az|az-ir|be|bg|bi|bin|br|bs|bua|ca|ce|ch|co|cs|cy|da|de|el|en|eo|es|et|eu|fa|fi|fj|fo|fr|fur|fy|ga|gd|gl|gn|gv|he|ho|hr|hu|ia|ibo|id|ie|ik|io|is|it|kaa|ki|kk|kl|ku-ir|kum|ky|la|lb|lez|lt|lv|mg|mh|mi|mk|mo|mt|nb|nds|nl|nn|no|nr|nso|ny|oc|om|os|pl|ps-af|ps-pk|pt|rm|ro|ru|se|sel|sh|shs|sk|sl|sma|smj|smn|sms|so|sq|sr|ss|st|sv|sw|th|tk|tn|tr|ts|tt|tyv|ug|uk|ur|ven|vi|vo|vot|wa|wen|wo|xh|yap|yi|zu(s)
	fontversion: 206438(i)(s)
	capability: "otlayout:arab"(s)
	fontformat: "TrueType"(s)
	embeddedbitmap: FcTrue(s)
	decorative: FcFalse(s)
	lcdfilter: 1(i)(w)

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 351661 [details] [diff] [review]
use cairo_ft_font_options_substitute from system cairo

a191=beltzner

Comment 13

[Karl Tomlinson (:karlt)]

Fixed on 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f4a71c7db6ef