Bug 33164 – [WinCairo] Improper Cairo Surface Type Sometimes Used

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

I tracked down the cause of an annoying ASSERTION hit during some WebKit operations when running under the WinCairo port.  WebKit would assert when checking that the target bitsPerPixel was 32.

This was caused by two cases:
(1) Sometimes a device context to a 24-bits-per-pixel context was being used (for example, when printing).
(2) Sometimes a new device context was passed that did not have a corresponding (existing) bitmap that could be selected from 'GetCurrentObject'.

In the second case, the bitmap structure returned by GetObject was filled with garbage, causing the assertion and possibly causing construction of extremely large image surfaces (as the info.bmWidth/bmHeight field were filled in with uninitialized data).

This patch corrects both problems by confirming that a valid bitmap handle is returned by GetCurrentObject, and falling back to the default cairo_win32_surface_create method, which constructs a standard GDI-compatible 24-bits-per-pixel surface from a raw device context handle.

We only want to pay the price of a 32-bit cairo surface for rendering when its needed, otherwise we take no advantage of native GDI drawing operations inside Cairo.

[Brent Fulgham]

Created attachment 45811 [details]
Check bitmap validity, only construct 32-bit surface when needed.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45811 [details] without any errors.

[Adam Roben (:aroben)]

Comment on attachment 45811 [details]
Check bitmap validity, only construct 32-bit surface when needed.

> +    if (!GetObject(bitmap, sizeof(info), &info))
> +        surface = cairo_win32_surface_create(hdc);

Does this surface have an associated bitmap? Do we need to select it into the DC?

Please add a link to this bug in the ChangeLog.

r=me

[Brent Fulgham]

(In reply to comment #3)
> (From update of attachment 45811 [details])
> > +    if (!GetObject(bitmap, sizeof(info), &info))
> > +        surface = cairo_win32_surface_create(hdc);
> 
> Does this surface have an associated bitmap? Do we need to select it into the
> DC?
> 
> Please add a link to this bug in the ChangeLog.
> 
> r=me

No, the created surface references whatever is currently in the DC.  The only reason for the second path is to create a 32-bit image bitmap backing store so that alpha effects can be handled properly.

[Brent Fulgham]

Landed in http://trac.webkit.org/changeset/52752.