RESOLVED FIXED 61548
Add 'Skia for rendering, CG for UI' option to Chromium porting layer
https://bugs.webkit.org/show_bug.cgi?id=61548
Summary Add 'Skia for rendering, CG for UI' option to Chromium porting layer
Cary Clark
Reported 2011-05-26 11:44:43 PDT
Add 'Skia for rendering, CG for UI' option to Chromium porting layer
Attachments
Patch (5.05 KB, patch)
2011-05-26 11:57 PDT, Cary Clark
no flags
Patch (2.03 KB, patch)
2011-05-31 07:49 PDT, Cary Clark
no flags
Cary Clark
Comment 1 2011-05-26 11:57:16 PDT
Eric Seidel (no email)
Comment 2 2011-05-26 12:04:36 PDT
Comment on attachment 95014 [details] Patch I'm confused. Can we draw the UI with anything other than CG? If not, seems we wouldn't need this.
Cary Clark
Comment 3 2011-05-26 12:14:52 PDT
As checked in, if WebKit uses Skia, the UI doesn't use CG, and vice-versa. Windows and Linux ports use Skia, but don't use CG for the UI. In the future, the Mac port uses Skia for WebKit, and uses CG for the UI. In the distant future, the Mac port uses hardware-accelerated Skia, and may not use CG for the UI. Some of the current scaffolding will go away once all of Chrome uses Skia for WebKit. Allowing the scaffolding for now allows checking in Skia support incrementally instead of all at once.
Darin Fisher (:fishd, Google)
Comment 4 2011-05-26 14:45:40 PDT
Comment on attachment 95014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95014&action=review > Source/WebKit/chromium/public/WebCommon.h:44 > + #define UI_USING_CG 0 this seems like the wrong place for this define. this is the webkit layer, which either renders using SKIA or CG. that's what WEBKIT_USING_SKIA conveys. it sounds like maybe you want to add a #define in the chromium tree somewhere. > Source/WebKit/chromium/src/DragClientImpl.cpp:91 > +#if WEBKIT_USING_SKIA && !UI_USING_CG this seems problematic. it seems like we should modify the chromium code which implements WebViewClient::startDragging to understand that it is getting a SkBitmap instead of a CGImage here. it would be much better to patch some conversion code into chromium than to confuse webkit with the notion of supporting two graphics libraries in different contexts.
Cary Clark
Comment 5 2011-05-27 05:13:07 PDT
Comment on attachment 95014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95014&action=review >> Source/WebKit/chromium/public/WebCommon.h:44 >> + #define UI_USING_CG 0 > > this seems like the wrong place for this define. this is the webkit layer, > which either renders using SKIA or CG. that's what WEBKIT_USING_SKIA conveys. > > it sounds like maybe you want to add a #define in the chromium tree somewhere. Could you please be more specific and suggest where 'somewhere' is? >> Source/WebKit/chromium/src/DragClientImpl.cpp:91 >> +#if WEBKIT_USING_SKIA && !UI_USING_CG > > this seems problematic. it seems like we should modify the chromium > code which implements WebViewClient::startDragging to understand that > it is getting a SkBitmap instead of a CGImage here. > > it would be much better to patch some conversion code into chromium > than to confuse webkit with the notion of supporting two graphics > libraries in different contexts. I thought that this code was chromium-specific already. Since there are no references to 'startDrag' or 'DragImageRef' outside of Source/WebCore and Source/WebKit, I'm at a loss as to what I should do. Who can I work with to restructure this as you suggest?
Cary Clark
Comment 6 2011-05-31 07:49:03 PDT
Eric Seidel (no email)
Comment 7 2011-05-31 07:59:19 PDT
Comment on attachment 95433 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95433&action=review > Source/WebKit/chromium/public/WebCommon.h:50 > + #if defined(__APPLE__) && !WEBKIT_USING_SKIA What if WEBKIT_USING_SKIA is not defined? I assume ! will do the right thing?
Cary Clark
Comment 8 2011-05-31 08:39:39 PDT
Comment on attachment 95433 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=95433&action=review >> Source/WebKit/chromium/public/WebCommon.h:50 >> + #if defined(__APPLE__) && !WEBKIT_USING_SKIA > > What if WEBKIT_USING_SKIA is not defined? I assume ! will do the right thing? WEBKIT_USING_SKIA is always defined either on line 43 or line 45, just above these lines.
Eric Seidel (no email)
Comment 9 2011-05-31 08:43:44 PDT
Comment on attachment 95433 [details] Patch OK.
WebKit Commit Bot
Comment 10 2011-05-31 09:05:45 PDT
Comment on attachment 95433 [details] Patch Clearing flags on attachment: 95433 Committed r87728: <http://trac.webkit.org/changeset/87728>
WebKit Commit Bot
Comment 11 2011-05-31 09:05:52 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.