WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(2.35 KB, patch)
2012-03-27 13:01 PDT
,
Mike Reed
no flags
Details
Formatted Diff
Diff
Patch
(2.37 KB, patch)
2012-03-28 07:20 PDT
,
Mike Reed
no flags
Details
Formatted Diff
Diff
Patch
(2.66 KB, patch)
2012-03-29 06:36 PDT
,
Mike Reed
no flags
Details
Formatted Diff
Diff
Patch
(2.82 KB, patch)
2012-03-30 05:10 PDT
,
Mike Reed
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Mike Reed
Comment 1
2012-03-27 11:38:38 PDT
Created
attachment 134105
[details]
Patch
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
Created
attachment 134119
[details]
Patch
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
Created
attachment 134290
[details]
Patch
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
Created
attachment 134565
[details]
Patch
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
Created
attachment 134785
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug