495455 - Visual glitches on hover with @font-face font

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Steven Garrity [:sgarrity]]

Attachment: Screenshot of glitch during :hover

Using a custom font with the (awesome) new @font-face feature, I'm seeing a glitch. If you change the font color on :hover, most of the font changes, except for part of the font at the very top (ascenders?). These same parts of the letters that are not changing color also appear to not be clickable.

Test case coming...

Comment 1

[Steven Garrity [:sgarrity]]

Attachment: Simple HTML test-case

Comment 2

[Steven Garrity [:sgarrity]]

Attachment: True Type Font used in test-case

Here's the font used in the test-case. I'm not sure if it's specific to this font or not.

Comment 3

[Steven Garrity [:sgarrity]]

I should have pointed out, this is with the Shiretoko nightly, 20090529, on both Linux and Mac OS X.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is because by default we don't compute exact glyph bounds, for performance reasons, when the font size is less than 20 pixels. You can work around this by adding text-rendering:optimizeLegibility to your content.

I suppose it would be a good idea to also take the slow path (which also enables Opentype features on Windows and Linux) for all @font-face text. John, what do you think?

Comment 5

[Christopher Blizzard (:blizzard)]

As mentioned here: http://weblogs.mozillazine.org/roc/archives/2007/07/status_3.html

Steven, if this fixes your problem let us know.

We should document this - seems pretty important.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This patch marks gfxFontEntries that correspond to user fonts (@font-face), and when we measure text that uses those fonts, we always get precise glyph bounds. That should fix this bug on all platforms. It's very simple and safe IMHO.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Force Windows to take Uniscribe path for @font-face fonts

I had also been thinking it would be a good idea to take the shaping path for all gfxFontGroups that contain a user font, on the assumption that authors want us to use typographic features of those fonts. On Mac this is a non-issue, we take the shaping path for all fonts. On GTK I need to talk to Karl to figure out what to do.

This patch does it for Windows, but we have the problem that Uniscribe completely barfs on CFF fonts, so this patch has the side effect of *completely disabling* use of CFF fonts with @font-face, on Windows --- such as the font Steven is using in his testcase. OTOH it looks like Steven's example doesn't work on trunk already --- John, are CFF fonts already completely disabled in @font-face on Windows? If so, we could take this patch since it won't make anything worse.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Karl, see comment #7. The problem on GTK/Pango is that at least for the first time we use a fontgroup, we don't build the list of fonts until after we've chosen which path to take. So some code restructuring is required.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #7)
> This patch does it for Windows, but we have the problem that Uniscribe
> completely barfs on CFF fonts,

Dynamically loaded fonts, that is.

Comment 10

[Steven Garrity [:sgarrity]]

Yeah, text-rendering:optimizeLegibility will do the trick for me for now. Thanks.

Comment 11

[John Daggett (:jtd)]

(In reply to comment #7)
> This patch does it for Windows, but we have the problem that Uniscribe
> completely barfs on CFF fonts, so this patch has the side effect of *completely
> disabling* use of CFF fonts with @font-face, on Windows --- such as the font
> Steven is using in his testcase. OTOH it looks like Steven's example doesn't
> work on trunk already --- John, are CFF fonts already completely disabled in
> @font-face on Windows? If so, we could take this patch since it won't make
> anything worse.

CFF user fonts can only be rendered with the GDI path, Uniscribe doesn't like CFF fonts loaded via AddFontMemResourceEx.  So they will render currently but without kerning/ligatures:

http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/src/gfxWindowsFonts.cpp#1805

But even if you force the Uniscribe shaping path, things should still work, since the mForceGDI flag should catch this case.  So I'm puzzled why your patch would disable CFF fonts.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't know, but I can't get Steven's font to work with @font-face in my Windows build.

Comment 13

[Karl Tomlinson (:karlt)]

(In reply to comment #7)
> I had also been thinking it would be a good idea to take the shaping path for
> all gfxFontGroups that contain a user font, on the assumption that authors want
> us to use typographic features of those fonts.

(In reply to comment #8)
> Karl, see comment #7. The problem on GTK/Pango is that at least for the first
> time we use a fontgroup, we don't build the list of fonts until after we've
> chosen which path to take. So some code restructuring is required.

Yes, in the code for GTK/Pango, it is not so easy to determine whether a user font might get used (without writing extra code to work it out).

One thing to consider is just testing mUserFontSet.
That would be simple but would mean the shaping path would be used for the whole document.

Note that attachment 380668 [details] [diff] [review] will cause the shaping path to be used even if the user font is only a fallback font (and doesn't actually get used).  So it also can change the rendering with system fonts.  Also, IIUC text using @font-face { src: local(); } will render differently to text using system "font-family"s.
I don't know whether that's bad, but something to consider.

With the Pango shaping path, I don't know whether its the font-selection (pango_itemize) or the shaping (pango_shape) that makes the path slower.
If it is (only) the shaping, then the fast path could be implemented as a really-basic shaper (instead of pango_shape) within CreateGlyphRunsItemizing.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #13)
> Note that attachment 380668 [details] [diff] [review] will cause the shaping path to be used
> even if the user font is only a fallback font (and doesn't actually get used).
> So it also can change the rendering with system fonts.  Also, IIUC text using
> @font-face { src: local(); } will render differently to text using system
> "font-family"s. I don't know whether that's bad, but something to consider.

Yeah. That wouldn't be bad IMHO.

Comment 15

[Karl Tomlinson (:karlt)]

As a data point, I got the try server to run Tp with the fast path completely disabled with GTK/Pango.  This increased Tp from 224 to 230, a 2-3% increase.

Comment 16

[John Daggett (:jtd)]

(In reply to comment #12)
> I don't know, but I can't get Steven's font to work with @font-face in my
> Windows build.

The TTLoadEmbeddedFont API is returning an error for this font but Webkit/Opera10 load this fine.  So that's a different bug...

Comment 17

[John Daggett (:jtd)]

Comment on attachment 380667 [details] [diff] [review]
fix

Looks fine, couple small cleanup details.

> +    PRPackedBool     mIsUserFont  : 1;

There's already a mUserFont in the Windows FontEntry class, that needs to be trimmed out.

>          if (IsType1())
>              mForceGDI = PR_TRUE;
> +        mIsUserFont = aUserFontData != nsnull;

This is already set in Windows code so this can be trimmed I think.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/97f8d01a5585

Comment 19

[Samuel Sidler (old account; do not CC)]

Wanted, but I don't see any reason for it to block 1.9.1.1.

Comment 20

[Eric Shepherd [:sheppy]]

This looks like the patch here undoes the need for any documentation change as specified in comment #5; the patch makes Gecko automatically calculate glyph sizes so that optimizeLegibility is no longer necessary, correct?

If that's true, I think I can simply remove the doc needed tag from this.

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes.

Comment 22

[Samuel Sidler (old account; do not CC)]

Is this still wanted for 1.9.1? If so, can we get a patch made for that branch? (Or if the current one applies, please request approval.)

(For drivers: Note that this is a known issue we're listing in our release notes.)

Comment 23

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

Removing relnote

Comment 24

[Tim (fmdeveloper)]

This bug states that relnote was removed, but it is still appearing in the 3.5.7 release notes (http://en-us.www.mozilla.com/en-US/firefox/3.5.7/releasenotes/). Shouldn't it have been removed?

Comment 25

[Samuel Sidler (:ss)]

It was removed from the 3.6 release notes and no one backported that change to the 3.5.x release notes. Feel free to file a Websites::www.mozilla.com bug if you care, but it'll be fixed in 3.6.