RESOLVED FIXED 82360
pass alpha directly to player, rather than creating a layer (for performance)
https://bugs.webkit.org/show_bug.cgi?id=82360
Summary pass alpha directly to player, rather than creating a layer (for performance)
Mike Reed
Reported 2012-03-27 11:35:46 PDT
pass alpha directly to player, rather than creating a layer (for performance)
Attachments
Patch (2.35 KB, patch)
2012-03-27 11:38 PDT, Mike Reed
no flags
Patch (2.35 KB, patch)
2012-03-27 13:01 PDT, Mike Reed
no flags
Patch (2.37 KB, patch)
2012-03-28 07:20 PDT, Mike Reed
no flags
Patch (2.66 KB, patch)
2012-03-29 06:36 PDT, Mike Reed
no flags
Patch (2.82 KB, patch)
2012-03-30 05:10 PDT, Mike Reed
no flags
Mike Reed
Comment 1 2012-03-27 11:38:38 PDT
WebKit Review Bot
Comment 2 2012-03-27 11:40:23 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
WebKit Review Bot
Comment 3 2012-03-27 11:53:17 PDT
Comment on attachment 134105 [details] Patch Attachment 134105 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12143684
Mike Reed
Comment 4 2012-03-27 13:01:37 PDT
Mike Reed
Comment 5 2012-03-27 13:06:55 PDT
need to wait for webkit's DEPS on chrome to update to get the api change already landed there...
WebKit Review Bot
Comment 6 2012-03-27 13:24:59 PDT
Comment on attachment 134119 [details] Patch Attachment 134119 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12148612
Mike Reed
Comment 7 2012-03-28 07:20:57 PDT
Mike Reed
Comment 8 2012-03-28 09:08:15 PDT
Comment on attachment 134290 [details] Patch mis-fired on the +. Just meant to say ?
Mike Reed
Comment 9 2012-03-28 11:19:08 PDT
PTAL
Darin Fisher (:fishd, Google)
Comment 10 2012-03-28 15:24:28 PDT
Comment on attachment 134290 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134290&action=review > Source/WebKit/chromium/public/WebMediaPlayer.h:119 > + virtual void paint(WebCanvas* canvas, const WebRect& rect) nit: Can you delete this old method and just fix-up the WEBKIT_USING_CG path to pass 0xFF?
James Robinson
Comment 11 2012-03-28 16:19:01 PDT
Can we just delete the WEBKIT_USING_CG code from **/chromium/** completely? We don't compile in this mode and are unlikely to start doing so spontaneously.
Adam Barth
Comment 12 2012-03-28 17:12:10 PDT
> Can we just delete the WEBKIT_USING_CG code from **/chromium/** completely? We don't compile in this mode and are unlikely to start doing so spontaneously. My understanding is that we now delete WEBKIT_USING_CG. It's untested, therefore it's broken.
Mike Reed
Comment 13 2012-03-29 05:42:13 PDT
I would prefer to delete dead code unrelated to this change, in a separate CL, so I don't conflate the two ideas (and slightly complicate testing and revert possibilities). However, if deleting that code in just these files will get me approved, then I'll do it :)
Mike Reed
Comment 14 2012-03-29 06:36:23 PDT
Mike Reed
Comment 15 2012-03-29 06:37:37 PDT
this patch also deletes (dead) code behind WEBKIT_USING_CG, and assumes WEBKIT_USING_SKIA
Mike Reed
Comment 16 2012-03-29 07:10:59 PDT
PTAL
James Robinson
Comment 17 2012-03-29 14:37:18 PDT
Comment on attachment 134565 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134565&action=review I don't know enough about what this does to review it. You will need to find someone who is familiar with this code. > Source/WebKit/chromium/src/WebMediaPlayerClientImpl.cpp:45 > // WebCommon.h defines WEBKIT_USING_SKIA so this has to be included last. > -#if WEBKIT_USING_SKIA > #include "PlatformContextSkia.h" now that you've removed the #ifdef you should delete the comment and put this in the normal include location (sorted with other includes)
Andrew Scherkus
Comment 18 2012-03-29 15:05:03 PDT
jamesr: I've been working with reed on the chromium-side changes patch is r+ to me
James Robinson
Comment 19 2012-03-29 16:49:32 PDT
Comment on attachment 134565 [details] Patch OK, that works for me (and would have been useful information to have on this bug earlier)
Mike Reed
Comment 20 2012-03-30 05:10:27 PDT
Mike Reed
Comment 21 2012-03-30 05:17:06 PDT
#include moved into upper section with comment removed. PTAL
Stephen White
Comment 22 2012-03-30 08:12:51 PDT
Comment on attachment 134785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134785&action=review Looks good. r=me > Source/WebKit/chromium/ChangeLog:3 > + pass alpha directly to player, rather than creating a layer (for performance) Super-nit: This should probably be a real sentence.
WebKit Review Bot
Comment 23 2012-03-30 10:09:10 PDT
Comment on attachment 134785 [details] Patch Clearing flags on attachment: 134785 Committed r112682: <http://trac.webkit.org/changeset/112682>
WebKit Review Bot
Comment 24 2012-03-30 10:09:16 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.