Bug 33164 - [WinCairo] Improper Cairo Surface Type Sometimes Used
Summary: [WinCairo] Improper Cairo Surface Type Sometimes Used
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows XP
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-04 11:47 PST by Brent Fulgham
Modified: 2010-01-04 12:29 PST (History)
1 user (show)

See Also:


Attachments
Check bitmap validity, only construct 32-bit surface when needed. (2.27 KB, patch)
2010-01-04 11:53 PST, Brent Fulgham
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2010-01-04 11:47:08 PST
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.
Comment 1 Brent Fulgham 2010-01-04 11:53:57 PST
Created attachment 45811 [details]
Check bitmap validity, only construct 32-bit surface when needed.
Comment 2 WebKit Review Bot 2010-01-04 11:56:15 PST
style-queue ran check-webkit-style on attachment 45811 [details] without any errors.
Comment 3 Adam Roben (:aroben) 2010-01-04 12:08:13 PST
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
Comment 4 Brent Fulgham 2010-01-04 12:24:47 PST
(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.
Comment 5 Brent Fulgham 2010-01-04 12:29:40 PST
Landed in http://trac.webkit.org/changeset/52752.