[webkit-reviews] review denied: [Bug 65658] Readback composited webgl results for printing : [Attachment 103544] remove now-unnecessary DEPS roll

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 10 16:22:12 PDT 2011


James Robinson <jamesr at chromium.org> has denied John Bauman
<jbauman at chromium.org>'s request for review:
Bug 65658: Readback composited webgl results for printing
https://bugs.webkit.org/show_bug.cgi?id=65658

Attachment 103544: remove now-unnecessary DEPS roll
https://bugs.webkit.org/attachment.cgi?id=103544&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103544&action=review


> Source/WebCore/platform/graphics/GraphicsContext3D.h:797
> +#if PLATFORM(CHROMIUM)
> +    void paintFramebufferToCanvas(int framebuffer, int width, int height,
bool premultiplyAlpha, CanvasRenderingContext*);
> +#endif

This seems wrong, if it's chromium-specific shouldn't this be on
Extensions3DChromium?

Additionally it's a layering violation to have a dependency on
CanvasRenderingContext here.  CanvasRenderingContext is defined in
WebCore/html/canvas/.  Classes in WebCore/platform/ should not depend on
anything in WebCore/ outside of the platform/ directory.

It seems that this parameter is just used to get at the underlying
ImageBuffer*.  Could we pass in an ImageBuffer* instead, perhaps)?

> Source/WebCore/platform/graphics/chromium/WebGLLayerChromium.h:42
> +class CanvasRenderingContext;

Newp, you can't depend on CanvasRenderingContext here.


More information about the webkit-reviews mailing list