[webkit-reviews] review denied: [Bug 53201] Make GraphicsContext3D use DrawingBuffer : [Attachment 111190] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Jeff Timanus
<twiz at chromium.org>'s request for review:
Bug 53201: Make GraphicsContext3D use DrawingBuffer
https://bugs.webkit.org/show_bug.cgi?id=53201

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

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


The relationship between DrawingBuffer, GraphicsContext3D,
WebGLRenderingContext and the PlatformLayer still doesn't feel quite right. 
With this setup it seems like responsibility for setting up the compositing
layer is spread across too many different objects.  Would it make more sense
for the DrawingBuffer itself own the PlatformLayer?  That way in Chromium
DrawingBufferChromium can handle setting up the correct WebGLLayerChromium
objects, on the Mac port DrawingBufferMac can do the right setup, etc. 
WebGLRenderingContext wouldn't need to do any special setup.

> Source/WebCore/platform/graphics/chromium/LayerChromium.h:206
> +    void setDrawingBuffer(PassRefPtr<DrawingBuffer> drawingBuffer) {
m_drawingBuffer = drawingBuffer; }
> +    PassRefPtr<DrawingBuffer> drawingBuffer() { return m_drawingBuffer; }
> +

this doesn't make any sense to have on LayerChromium - it's only relevant for
WebGL layers. I think you need to either downcast at callsites (yuck) or
rethink the object responsibilities.