Closed
Bug 495455
Opened 15 years ago
Closed 15 years ago
Visual glitches on hover with @font-face font
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
status1.9.1 | --- | wanted |
People
(Reporter: sgarrity, Assigned: roc)
Details
Attachments
(5 files)
4.94 KB,
image/png
|
Details | |
629 bytes,
text/html
|
Details | |
26.22 KB,
application/octet-stream
|
Details | |
10.29 KB,
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
2.96 KB,
patch
|
Details | Diff | Splinter Review |
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...
Reporter | ||
Comment 1•15 years ago
|
||
Reporter | ||
Comment 2•15 years ago
|
||
Here's the font used in the test-case. I'm not sure if it's specific to this font or not.
Reporter | ||
Comment 3•15 years ago
|
||
I should have pointed out, this is with the Shiretoko nightly, 20090529, on both Linux and Mac OS X.
Updated•15 years ago
|
Component: General → Layout: View Rendering
Product: Firefox → Core
QA Contact: general → layout.view-rendering
Version: 3.5 Branch → 1.9.1 Branch
Assignee | ||
Comment 4•15 years ago
|
||
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•15 years ago
|
||
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.
Keywords: dev-doc-needed
Updated•15 years ago
|
Assignee | ||
Comment 6•15 years ago
|
||
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.
Assignee: nobody → roc
Attachment #380667 -
Flags: review?(jdaggett)
Assignee | ||
Updated•15 years ago
|
Whiteboard: [needs review]
Assignee | ||
Updated•15 years ago
|
Component: Layout: View Rendering → GFX: Thebes
QA Contact: layout.view-rendering → thebes
Assignee | ||
Comment 7•15 years ago
|
||
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.
Assignee | ||
Comment 8•15 years ago
|
||
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.
Assignee | ||
Comment 9•15 years ago
|
||
(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.
Reporter | ||
Comment 10•15 years ago
|
||
Yeah, text-rendering:optimizeLegibility will do the trick for me for now. Thanks.
Comment 11•15 years ago
|
||
(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.
Assignee | ||
Comment 12•15 years ago
|
||
I don't know, but I can't get Steven's font to work with @font-face in my Windows build.
Comment 13•15 years ago
|
||
(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.
Assignee | ||
Comment 14•15 years ago
|
||
(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•15 years ago
|
||
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•15 years ago
|
||
(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...
Updated•15 years ago
|
Attachment #380667 -
Flags: review?(jdaggett) → review+
Comment 17•15 years ago
|
||
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.
Assignee | ||
Comment 18•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/97f8d01a5585
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [needs review]
Updated•15 years ago
|
Flags: blocking1.9.1.1?
Comment 19•14 years ago
|
||
Wanted, but I don't see any reason for it to block 1.9.1.1.
Flags: blocking1.9.1.1? → blocking1.9.1.1-
status1.9.1:
--- → ?
Updated•14 years ago
|
Flags: wanted1.9.1.x+
Comment 20•14 years ago
|
||
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 22•14 years ago
|
||
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 24•14 years ago
|
||
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•14 years ago
|
||
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•