glyph extents from cairo dwrite backend may lead to unwanted clipping

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: jfkthame, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
This is essentially a directwrite version of bug 445087. The cairo dwrite backend uses GetDesignGlyphMetrics on the font face to get glyph extents, but this returns "ideal" bounds of the outline in font space; it does not account for hinting, grid-fitting, or antialiasing that will occur when the glyph is actually painted. This means that the glyph extents returned by cairo may not include all pixels that are touched by the glyph; and when these extents are then used to determine the area to be erased or repainted, we can get clipping or other visual glitches.

The problem is less severe than the GDI/ClearType case because dwrite's antialiasing is less aggressive; nevertheless, the results can sometimes be seen in close examination of web content, and lead to numerous reftest failures when dwrite is enabled.
(Reporter)

Updated

8 years ago
Blocks: 549832
(Reporter)

Comment 1

8 years ago
Created attachment 447506 [details]
testcase for dwrite/cairo glyph clipping

This example shows the effect of too-tight glyph extents in cairo, when viewed using the dwrite font backend.
(Reporter)

Comment 2

8 years ago
Created attachment 447507 [details]
screenshot of the testcase displayed with dwrite enabled
(Reporter)

Comment 3

8 years ago
Created attachment 447509 [details] [diff] [review]
patch, v1 - make cairo add padding to the "ideal" glyph extents

This should effectively add one pixel (if I've got the scaling issues right) to each side of the "ideal" glyph bounding box that dwrite returned, to allow for the likely additional pixels that may be painted on the actual device.

We'll need to watch out for the possibility that this triggers unwanted scrollbars due to new overflow areas during layout, but so far I have not seen problems from it. In the horizontal direction it should be comparable to what we already do with cleartype/gdi rendering, but for dwrite it appears to be important to pad vertically as well.
Attachment #447509 - Flags: review?(bas.schouten)
(Reporter)

Comment 4

8 years ago
Created attachment 447510 [details]
screenshot of the testcase with the dwrite/cairo extents patch
(Reporter)

Comment 5

8 years ago
BTW, when I run reftest locally with dwrite enabled, this patch reduces unexpected failures from 53 to 30, because it fixes many cases where antialiasing pixels were not being painted properly.
(Reporter)

Comment 6

8 years ago
Created attachment 449117 [details] [diff] [review]
patch, v2 - make cairo add padding to the "ideal" glyph extents; support tight-hinted-outline-extents for mathml

Patch, v2. Adding "padding" to the glyph extents to allow for antialiasing affects MathML layout, and leads to reftest failures (at least bugs/355548-3.xml). This same issue was seen with the GDI backend.

The fix is also the same as with GDI: support the TIGHT_HINTED_OUTLINE_EXTENTS option by creating a non-antialiased version of the font, and returning unpadded metrics from cairo in this case.

This brings my local reftest failure count with DWrite down to 27.
Attachment #449117 - Flags: review?(bas.schouten)
(Reporter)

Updated

8 years ago
Attachment #447509 - Attachment is obsolete: true
Attachment #447509 - Flags: review?(bas.schouten)
Attachment #449117 - Flags: review?(bas.schouten) → review+
(Reporter)

Comment 7

8 years ago
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/rev/ffa8bc798d99

Also added the patch file to the gfx/cairo directory, and noted it in the README there:
http://hg.mozilla.org/mozilla-central/rev/6efd0963eae9
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.