Bug 72188 - Expose compositeAndReadback in WebLayerTreeView
Summary: Expose compositeAndReadback in WebLayerTreeView
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-11 14:47 PST by vollick
Modified: 2011-11-14 16:51 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.50 KB, patch)
2011-11-11 14:56 PST, vollick
no flags Details | Formatted Diff | Diff
Patch (2.75 KB, patch)
2011-11-11 15:58 PST, vollick
no flags Details | Formatted Diff | Diff
Patch (2.85 KB, patch)
2011-11-14 07:35 PST, vollick
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description vollick 2011-11-11 14:47:52 PST
Expose compositeAndReadback in WebLayerTreeView
Comment 1 vollick 2011-11-11 14:56:26 PST
Created attachment 114777 [details]
Patch
Comment 2 WebKit Review Bot 2011-11-11 14:58:34 PST
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
Comment 3 Antoine Labour 2011-11-11 15:24:43 PST
View in context: https://bugs.webkit.org/attachment.cgi?id=114777&action=review

> Source/WebKit/chromium/public/WebLayerTreeView.h:86
> +                                            int height);

WebKit is not subject to the 80 cols restrictions, so you can make it a single line.

> Source/WebKit/chromium/src/WebLayerTreeView.cpp:90
> +                                            int height)

Single line

> Source/WebKit/chromium/src/WebLayerTreeView.cpp:93
> +        pixels, WebCore::IntRect(0, 0, width, height));

Single line
Comment 4 James Robinson 2011-11-11 15:34:08 PST
Comment on attachment 114777 [details]
Patch

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

> Source/WebKit/chromium/public/WebLayerTreeView.h:86
> +                                            int width,
> +                                            int height);

this should not line wrap and it should use a WebSize parameter instead of a pair of ints

can you document the ownership model and requirements of the pixels buffer, and what goes into them (pretty sure it's rgba values)

you also need to document the return value and what happens to *pixels when this function returns false

> Source/WebKit/chromium/src/WebLayerTreeView.cpp:93
> +        pixels, WebCore::IntRect(0, 0, width, height));

don't line wrap this either
Comment 5 vollick 2011-11-11 15:58:18 PST
Created attachment 114792 [details]
Patch
Comment 6 Darin Fisher (:fishd, Google) 2011-11-13 20:40:00 PST
Comment on attachment 114792 [details]
Patch

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

> Source/WebKit/chromium/src/WebLayerTreeView.cpp:91
> +    return m_private->compositeAndReadback(pixels, WebCore::IntRect(0, 0, size.width, size.height));

any reason not to expose the underlying subset capability?

WebKit API changes LGTM
Comment 7 vollick 2011-11-14 07:35:55 PST
Created attachment 114947 [details]
Patch
Comment 8 vollick 2011-11-14 07:37:48 PST
fishd: There was not a good reason for not exposing the subset functionality. It's now exposed.
Comment 9 WebKit Review Bot 2011-11-14 16:51:47 PST
Comment on attachment 114947 [details]
Patch

Clearing flags on attachment: 114947

Committed r100217: <http://trac.webkit.org/changeset/100217>
Comment 10 WebKit Review Bot 2011-11-14 16:51:51 PST
All reviewed patches have been landed.  Closing bug.