WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(2.03 KB, patch)
2011-05-31 07:49 PDT
,
Cary Clark
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Cary Clark
Comment 1
2011-05-26 11:57:16 PDT
Created
attachment 95014
[details]
Patch
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
Created
attachment 95433
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug