Bug 61548 – Add 'Skia for rendering, CG for UI' option to Chromium porting layer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Cary Clark]

Add 'Skia for rendering, CG for UI' option to Chromium porting layer

[Cary Clark]

Created attachment 95014 [details]
Patch

[Eric Seidel (no email)]

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]

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 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 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]

Created attachment 95433 [details]
Patch

[Eric Seidel (no email)]

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 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 on attachment 95433 [details]
Patch

OK.

[WebKit Commit Bot]

Comment on attachment 95433 [details]
Patch

Clearing flags on attachment: 95433

Committed r87728: <http://trac.webkit.org/changeset/87728>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.