Bug 63075

Summary: REGRESSION (r88978): Text inside form controls looks really awful on Windows XP
Product: WebKit Reporter: Adam Roben (:aroben) <aroben>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: adachan, bweinstein, jhoneycutt
Priority: P2 Keywords: InRadar, PlatformOnly, Regression
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
URL: data:text/html,<input value=foo>
Description Flags
Don't use an alpha channel when drawing the web page on Windows andersca: review+

Description Adam Roben (:aroben) 2011-06-21 09:56:55 PDT
Created attachment 98001 [details]

To reproduce:

1. Go to data:text/html,<input value=foo>

The text inside the text field has this horrible black fringing which makes it look too bold and chunky. See screenshot.
Comment 1 Adam Roben (:aroben) 2011-06-21 10:12:45 PDT
Looks like this was caused by r88978: http://trac.webkit.org/changeset/88978
Comment 2 Adam Roben (:aroben) 2011-06-21 10:13:40 PDT
Comment 3 Adam Roben (:aroben) 2011-06-21 10:37:20 PDT
Other folks at Apple have failed to repro this on Windows 7, even though I can repro on XP. My guess is that there's a difference in GDI behavior between XP and 7 that's causing the difference.
Comment 4 Adam Roben (:aroben) 2011-06-21 12:12:48 PDT
The bug only occurs when subpixel antialiasing is enabled. Turning off ClearType, or setting Safari's font-smoothing preference to "Best for CRT", makes the bug go away.
Comment 5 Adam Roben (:aroben) 2011-06-22 07:02:07 PDT
Created attachment 98164 [details]

This testcase has the nice property that the upper-left pixel has a different appearance when the bug occurs and when it does not occur. This makes it easy to inspect the memory for the page's bitmap to see what's going on.
Comment 6 Adam Roben (:aroben) 2011-06-22 07:09:38 PDT
I'm stepping through the code using the attached testcase.

Before RenderThemeWin::paintTextField is called, the bitmap is filled with 0xffffffff. RenderThemeWin::paintTextField then fills the text field with 0x00ffffff. Note that the alpha channel has been changed to 0.

Soon later, Font::drawGlyphs is called. At this point the first few pixels are:

0x08000808 0xff728080 0xff808080 0xff808080

The first pixel here is nearly-transparent nearly-black. The second pixel is opaque gray with the red channel darkened a little. The third and fourth pixels are opaque gray.

Over on the UI process side, ::BitBlt does preserve the alpha channel, so these same pixels end up written into the page's BackingStore bitmap. But when the page is displayed on screen, the alpha channel will be ignored, so the nearly-transparent nearly-black pixel will just become a nearly-black pixel, causing the ugly appearance.
Comment 7 Adam Roben (:aroben) 2011-06-22 07:16:57 PDT
One potential fix is to change DrawingAreaImpl::createGraphicsContext to create a GraphicsContext that does not support alpha. I.e., change this:

    return adoptPtr(new GraphicsContext(bitmapDC, true));

to this:

    return adoptPtr(new GraphicsContext(bitmapDC, false));

Doing so gives the following pixel values:

0xffffebbf 0xff918080 0xff808080 0xff808080

You can see that the first two pixels have changed to be opaque nearly-white and opaque nearly-gray, which is much better than opaque nearly-black!
Comment 8 Adam Roben (:aroben) 2011-06-22 07:17:43 PDT
Making this change also does not reintroduce bug 62690.
Comment 9 Adam Roben (:aroben) 2011-06-22 08:52:13 PDT
Rolling out r88978 changes the behavior of RenderThemeWin::paintTextField so that the 0xffffffff pixels keep that value. DrawThemeBackground still fills the temporary bitmap inside the LocalWindowsContext with 0x00ffffff as before, but GraphicsContext::releaseWindowsContext ignores the alpha channel when blitting back into the original GraphicsContext, so opaque white is preserved.
Comment 10 Adam Roben (:aroben) 2011-06-22 09:05:34 PDT
Created attachment 98180 [details]
Don't use an alpha channel when drawing the web page on Windows
Comment 11 Adam Roben (:aroben) 2011-06-22 09:12:27 PDT
Committed r89436: <http://trac.webkit.org/changeset/89436>