Bug 57848

Summary: [Chromium] allow concurrent Skia and CG datatypes
Product: WebKit Reporter: Cary Clark <caryclark>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric, thakis
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
patch to change preprocessor conditions
none
Patch none

Description Cary Clark 2011-04-05 08:01:17 PDT
The Chromium port is experimenting with running Skia as the WebKit rendering engine, and CoreGraphics as the UI rendering engine. This permits Chromium to unify its graphics story while leveraging OS X to draw elements like scrollbars and buttons.

Restructure the common graphics units, points and rectangles, to convert to Sk-types and CG-types at the same time.

A CL describing this change in more detail is forthcoming.
Comment 1 Cary Clark 2011-04-06 06:47:10 PDT
Created attachment 88408 [details]
patch to change preprocessor conditions
Comment 2 David Levin 2011-04-06 08:43:16 PDT
Comment on attachment 88408 [details]
patch to change preprocessor conditions

View in context: https://bugs.webkit.org/attachment.cgi?id=88408&action=review

> WebCore/ChangeLog:12
> +        https://bugs.webkit.org/show_bug.cgi?id=57848

Note that typically this is:
bug title
bug link

description if any

Per file comments (which may be ditto).

But ok this time.
Comment 3 Nico Weber 2011-04-06 09:25:32 PDT
Are these the only |#if PLATFORM(CG)| changes you will need?

Would it be worth to add USE(CG_PRIMITIVES) and define that if PLATFORM(CG) || (USE(SKIA)…), for less repetiton?
Comment 4 Cary Clark 2011-04-06 10:27:18 PDT
Nico: There are other places where CG-specific code is required. However, these files together make up a nice chunk and are interdependent.

I thought about creating a new preprocessor definition, but did not see precedent for doing things that way. The current way, while verbose, is completely self-documenting, where USE(CG_PRIMITIVES) would hide slightly that this code is used when CoreGraphics is the platform graphics implementation, and when Skia+Chromium+Darwin is chosen.

However, I'm happy to change it to match WebKit conventions.
Comment 5 Nico Weber 2011-04-06 10:35:12 PDT
I guess if levin is fine with it, then the current patch matches WebKit conventions; the other way just seemed cleaner to me personally.
Comment 6 Cary Clark 2011-04-13 12:49:22 PDT
Created attachment 89438 [details]
Patch
Comment 7 Eric Seidel (no email) 2011-04-13 13:09:22 PDT
Comment on attachment 89438 [details]
Patch

LGTM.
Comment 8 WebKit Commit Bot 2011-04-13 23:26:14 PDT
Comment on attachment 89438 [details]
Patch

Clearing flags on attachment: 89438

Committed r83818: <http://trac.webkit.org/changeset/83818>
Comment 9 WebKit Commit Bot 2011-04-13 23:26:20 PDT
All reviewed patches have been landed.  Closing bug.