RESOLVED FIXED 61800
Ready Chromium port for Skia on Mac 
https://bugs.webkit.org/show_bug.cgi?id=61800
Summary Ready Chromium port for Skia on Mac
Cary Clark
Reported 2011-05-31 12:56:54 PDT
Ready Chromium port for Skia on Mac
Attachments
Patch (4.72 KB, patch)
2011-05-31 13:04 PDT, Cary Clark 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Cary Clark
Comment 1 2011-05-31 13:04:19 PDT
Created attachment 95466 [details] Patch
Eric Seidel (no email)
Comment 2 2011-05-31 19:35:11 PDT
Comment on attachment 95466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95466&action=review Seems OK, but could be cleaner. > Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:412 > + SkCanvas* canvas = context->platformContext()->canvas(); Seems like we want a direct accessor here somehow. Or maybe a skia(context) helper function. > Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438 > + gfx::SkiaBitLocker bitLocker(drawingContext->platformContext()->canvas()); We might want a WebCore helper which knows how to bitlock directly from a GraphicsContext instead of having to do this canvas lookup every time. We could also use an abstraction around the drawingContext which would lock in teh skia case, and otherwise just give you back the CGContext. That would remove a bunch of these #if USE(SKIA) branches.
Cary Clark
Comment 3 2011-06-01 05:40:16 PDT
Comment on attachment 95466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95466&action=review >> Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438 >> + gfx::SkiaBitLocker bitLocker(drawingContext->platformContext()->canvas()); > > We might want a WebCore helper which knows how to bitlock directly from a GraphicsContext instead of having to do this canvas lookup every time. > > We could also use an abstraction around the drawingContext which would lock in teh skia case, and otherwise just give you back the CGContext. That would remove a bunch of these #if USE(SKIA) branches. The bitlock/canvas lookup combination only happens four times in the sources. Also, the #if USE(SKIA) scaffolding is temporary; once the Chrome Mac port uses Skia, it will be removed.
Eric Seidel (no email)
Comment 4 2011-06-01 08:57:11 PDT
Comment on attachment 95466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95466&action=review >>> Source/WebCore/platform/chromium/ScrollbarThemeChromiumMac.mm:438 >>> + gfx::SkiaBitLocker bitLocker(drawingContext->platformContext()->canvas()); >> >> We might want a WebCore helper which knows how to bitlock directly from a GraphicsContext instead of having to do this canvas lookup every time. >> >> We could also use an abstraction around the drawingContext which would lock in teh skia case, and otherwise just give you back the CGContext. That would remove a bunch of these #if USE(SKIA) branches. > > The bitlock/canvas lookup combination only happens four times in the sources. Also, the #if USE(SKIA) scaffolding is temporary; once the Chrome Mac port uses Skia, it will be removed. I don't think "temporary" is every a very good excuse for ugly code. But I think this patch is OK. Best of luck. :)
WebKit Commit Bot
Comment 5 2011-06-01 10:19:55 PDT
Comment on attachment 95466 [details] Patch Clearing flags on attachment: 95466 Committed r87821: <http://trac.webkit.org/changeset/87821>
WebKit Commit Bot
Comment 6 2011-06-01 10:20:00 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.