Bug 64302 - [skia] optimize getImageData to avoid a copy when not needed. lockPixels() now does the right thing.
Summary: [skia] optimize getImageData to avoid a copy when not needed. lockPixels() no...
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mike Reed
URL:
Keywords:
Depends on: 64389
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-11 11:54 PDT by Mike Reed
Modified: 2013-04-09 13:03 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.10 KB, patch)
2011-07-11 11:56 PDT, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2011-07-11 12:12 PDT, Mike Reed
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2011-07-12 05:24 PDT, Mike Reed
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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