Bug 64302

Summary: [skia] optimize getImageData to avoid a copy when not needed. lockPixels() now does the right thing.
Product: WebKit Reporter: Mike Reed <reed>
Component: New BugsAssignee: Mike Reed <reed>
Status: RESOLVED WONTFIX    
Severity: Normal CC: bsalomon, dglazkov, jamesr, kbr, schenney, senorblanco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 64389    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Mike Reed 2011-07-11 11:54:54 PDT
[skia] optimize getImageData to avoid a copy when not needed. lockPixels() now does the right thing.
Comment 1 Mike Reed 2011-07-11 11:56:41 PDT
Created attachment 100341 [details]
Patch
Comment 2 WebKit Review Bot 2011-07-11 12:06:32 PDT
Comment on attachment 100341 [details]
Patch

Attachment 100341 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9014296
Comment 3 Kenneth Russell 2011-07-11 12:11:21 PDT
Comment on attachment 100341 [details]
Patch

Does this need a roll of the Chromium revision in WebKit/Source/WebKit/chromium/DEPS, and is that why the patch doesn't build on the cr-linux EWS?
Comment 4 Mike Reed 2011-07-11 12:12:16 PDT
Created attachment 100344 [details]
Patch
Comment 5 Mike Reed 2011-07-11 12:12:51 PDT
may need webkit to update to a newer chrome/skia DEPS for this to build.
Comment 6 WebKit Review Bot 2011-07-11 12:24:08 PDT
Comment on attachment 100344 [details]
Patch

Attachment 100344 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9010567
Comment 7 Kenneth Russell 2011-07-11 12:26:22 PDT
Comment on attachment 100344 [details]
Patch

Per above comment, please update chromium_rev in Source/WebKit/chromium/DEPS.
Comment 8 Mike Reed 2011-07-12 05:24:14 PDT
Created attachment 100470 [details]
Patch
Comment 9 Mike Reed 2011-07-12 05:35:06 PDT
no code change for this patch, just rebaselined the ChangeLog file
Comment 10 Kenneth Russell 2011-07-12 11:20:15 PDT
Comment on attachment 100470 [details]
Patch

Looks good to me.
Comment 11 Stephen White 2011-07-12 11:22:42 PDT
Comment on attachment 100470 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100470&action=review

> Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp:228
> +        return result.release();

Just for my own edification:  is this early-out for failure of the accessBitmap(), the extractSubset() or both?
Comment 12 Mike Reed 2011-07-12 11:23:20 PDT
accessBitmap() never fails. The boolean check is on extractSubset().
Comment 13 Stephen White 2011-07-12 11:23:28 PDT
Comment on attachment 100470 [details]
Patch

Looks good to me too (restoring r+).
Comment 14 WebKit Review Bot 2011-07-12 11:32:06 PDT
Comment on attachment 100470 [details]
Patch

Clearing flags on attachment: 100470

Committed r90831: <http://trac.webkit.org/changeset/90831>
Comment 15 WebKit Review Bot 2011-07-12 11:32:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 16 James Robinson 2011-07-12 18:20:25 PDT
Reverted http://trac.webkit.org/changeset/90847