Closed
Bug 666893
Opened 13 years ago
Closed 13 years ago
Windows (ClearType): make all area painted by text be part of text frame's ink overflow area (DirectWrite in GDI mode version)
Categories
(Core :: Layout: Text and Fonts, defect, P3)
Tracking
()
VERIFIED
FIXED
People
(Reporter: ehoogeveen, Assigned: jfkthame)
Details
Attachments
(3 files)
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110623 Firefox/7.0a1 Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110623 Firefox/7.0a1 Followup for bug 475968 to fix the DirectWrite side of things. The patch in that bug was made under the assumption that only GDI was affected. However, it appears that the problem is not limited to GDI, but also shows up with the 'GDI Classic' rendering mode set in the DirectWrite API. Bug 642589 gave users the ability to set gfx.font_rendering.cleartype_params.rendering_mode = 2 to force DirectWrite to use this rendering mode, and bug 661471 landing recently ensured that commonly used web fonts always use this mode. As such, the fix for bug 475968 should be extended to work for DirectWrite as well. Attached are the original testcase from bug 475968 that shows this problem, and a screenshot of the problem happening using DirectWrite. Reproducible: Always Steps to Reproduce: 1. Run the testcase using DirectWrite with the force_gdi_classic* options enabled. Actual Results: Vertical lines appear between 'Click here!' and 'Enter'. Expected Results: Space between 'Click here!' and 'Enter' should be blank.
Reporter | ||
Updated•13 years ago
|
Severity: normal → major
Priority: -- → P3
Reporter | ||
Updated•13 years ago
|
Version: unspecified → Trunk
Assignee | ||
Updated•13 years ago
|
Attachment #541647 -
Attachment mime type: text/plain → text/html
Reporter | ||
Comment 3•13 years ago
|
||
Apologies, I thought I had set it to text/html. Must have mis-clicked.
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → jfkthame
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 4•13 years ago
|
||
This essentially duplicates the GDI patch from bug 475968 in the DWrite font backend, but only when we're using GDI Classic rendering mode; in DW Natural mode, there isn't the same tendency for antialiasing to paint significantly outside the nominal ink bounds - or at least it's so slight that we haven't observed problems arising from it. Pushed to tryserver to see how the tests fare....
Attachment #541831 -
Flags: review?(roc)
Assignee | ||
Comment 5•13 years ago
|
||
No problems on unit tests (which is as I'd expect, given that we already do the same thing in some other backends). Tryserver build is available at https://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-544845e4fae7/try-win32/.
Comment on attachment 541831 [details] [diff] [review] patch, add padding pixels because of ClearType bleed when using GDI Classic mode Review of attachment 541831 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/thebes/gfxDWriteFonts.cpp @@ +627,5 @@ > + // and the underlying cairo font may be antialiased, > + // we can't trust Windows to have considered all the pixels > + // so we need to add "padding" to the bounds. > + // (see bugs 475968, 439831, compare also bug 445087) > + if (aBoundingBoxType == LOOSE_INK_EXTENTS && Shouldn't this be aBoundingBoxType != TIGHT_HINTED_OUTLINE_EXTENTS? Seems like this is needed for TIGHT_INK_EXTENTS as well.
Assignee | ||
Comment 7•13 years ago
|
||
(In reply to comment #6) > Comment on attachment 541831 [details] [diff] [review] [review] > patch, add padding pixels because of ClearType bleed when using GDI Classic > mode > > Review of attachment 541831 [details] [diff] [review] [review]: > ----------------------------------------------------------------- > > ::: gfx/thebes/gfxDWriteFonts.cpp > @@ +627,5 @@ > > + // and the underlying cairo font may be antialiased, > > + // we can't trust Windows to have considered all the pixels > > + // so we need to add "padding" to the bounds. > > + // (see bugs 475968, 439831, compare also bug 445087) > > + if (aBoundingBoxType == LOOSE_INK_EXTENTS && > > Shouldn't this be aBoundingBoxType != TIGHT_HINTED_OUTLINE_EXTENTS? > > Seems like this is needed for TIGHT_INK_EXTENTS as well. This matches what we do in the GDI font code. For TIGHT_INK_EXTENTS, padding is added to the glyph extents within cairo, so we don't need to do it here. (See _cairo_dwrite_scaled_font_init_glyph_metrics for the DWrite case, or _cairo_win32_scaled_font_init_glyph_metrics for the GDI equivalent. This has to be done within cairo to avoid clipping the glyph images when they're painted.) So it's only when we're doing "loose" measurement, so we're not getting actual glyph sidebearings etc. from cairo, that we need to add padding at the gfxFont level in case the glyphs at the edges of the run "bleed" slightly beyond their typographic origin/advance.
Reporter | ||
Comment 8•13 years ago
|
||
I can confirm that the testcase from comment #1 is fixed in the Tryserver build.
Comment on attachment 541831 [details] [diff] [review] patch, add padding pixels because of ClearType bleed when using GDI Classic mode Review of attachment 541831 [details] [diff] [review]: -----------------------------------------------------------------
Attachment #541831 -
Flags: review?(roc) → review+
Assignee | ||
Comment 10•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/592403064072
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 11•13 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110702 Firefox/7.0a1 Using today's nightly I no longer see the rendering glitches in the testcase from comment #1; marking as verified fixed. Thanks!
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•