Bug 72188 – Expose compositeAndReadback in WebLayerTreeView

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

Expose compositeAndReadback in WebLayerTreeView

Created attachment 114777 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail] before submitting because this patch contains changes to the Chromium public API.

[Antoine Labour]

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

[James Robinson]

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

Created attachment 114792 [details]
Patch

[Darin Fisher (:fishd, Google)]

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

Created attachment 114947 [details]
Patch

fishd: There was not a good reason for not exposing the subset functionality. It's now exposed.

[WebKit Review Bot]

Comment on attachment 114947 [details]
Patch

Clearing flags on attachment: 114947

Committed r100217: <http://trac.webkit.org/changeset/100217>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.