Summary: | [Chromium] allow concurrent Skia and CG datatypes | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Cary Clark <caryclark> | ||||||
Component: | Platform | Assignee: | 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
Cary Clark
2011-04-05 08:01:17 PDT
Created attachment 88408 [details]
patch to change preprocessor conditions
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. 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? 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. I guess if levin is fine with it, then the current patch matches WebKit conventions; the other way just seemed cleaner to me personally. Created attachment 89438 [details]
Patch
Comment on attachment 89438 [details]
Patch
LGTM.
Comment on attachment 89438 [details] Patch Clearing flags on attachment: 89438 Committed r83818: <http://trac.webkit.org/changeset/83818> All reviewed patches have been landed. Closing bug. |