Bug 57848 - [Chromium] allow concurrent Skia and CG datatypes
Summary: [Chromium] allow concurrent Skia and CG datatypes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-05 08:01 PDT by Cary Clark
Modified: 2011-04-13 23:26 PDT (History)
3 users (show)

See Also:


Attachments
patch to change preprocessor conditions (8.70 KB, patch)
2011-04-06 06:47 PDT, Cary Clark
no flags Details | Formatted Diff | Diff
Patch (8.94 KB, patch)
2011-04-13 12:49 PDT, Cary Clark
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.