[webkit-reviews] review denied: [Bug 41828] [chromium] Allow resizing an offscreen GLES2Context's backing store and getting its texture ID for compositing : [Attachment 60829] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied James Robinson
<jamesr at chromium.org>'s request for review:
Bug 41828: [chromium] Allow resizing an offscreen GLES2Context's backing store
and getting its texture ID for compositing
https://bugs.webkit.org/show_bug.cgi?id=41828

Attachment 60829: Patch
https://bugs.webkit.org/attachment.cgi?id=60829&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebCore/platform/chromium/GLES2Context.h:65
 +	unsigned getOffscreenContentParentTextureId();
i like this name for this method.  it helps me understand its distinction
better.

WebCore/platform/chromium/GLES2Context.h:62
 +	void resizeOffscreenContent(const IntSize& size);
nit: webkit style is to leave off parameter names that do not add information.
"size" is redundant with "IntSize"

WebCore/platform/chromium/GLES2Context.h:63
 +	// Returns the ID of the texture used for offscreen rendering in the
context of the parent.
nit: a new line above this comment block would be nice for readability.

WebKit/chromium/public/WebGLES2Context.h:52
 +	virtual void resizeOffscreenContent(int width, int height) = 0;
nit: use WebSize in the WebKit API.

WebKit/chromium/public/WebGLES2Context.h:53
 +	virtual unsigned getOffscreenContentParentTextureId() = 0;
how about documenting these methods like you did for GLES2Context.h?
it is important to document the WebKit API methods since that helps
make the contract with the embedder clearer.

WebKit/chromium/src/GLES2Context.cpp:148
 +	webContext->resizeOffscreenContent(size.width(), size.height());
WebSize can be constructed implicitly from IntSize :)

otherwise, LGTM based on this being a faithful interpretation of
what kbr wrote on the whiteboard earlier today :-)