[webkit-reviews] review denied: [Bug 79169] [Qt][WK2] Enable WebGL for WebKit2-Qt Linux/GLX : [Attachment 152206] patch for review.

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Zeno Albisser
<zeno at webkit.org>'s request for review:
Bug 79169: [Qt][WK2] Enable WebGL for WebKit2-Qt Linux/GLX
https://bugs.webkit.org/show_bug.cgi?id=79169

Attachment 152206: patch for review.
https://bugs.webkit.org/attachment.cgi?id=152206&action=review

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=152206&action=review


A bit too #ifdeffy... i'd rather it would be capability based and not OS based.


>> Source/WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:88
>> +#endif
> 
> I supposed that there are two users of GraphicsSurface: Those who need
double-buffering and those who don't (they write to the surface only once and
then are done with it).
> 
> I think should be encapsulated in GraphicsSurface itself (for example having
a front and a back PlatformGraphicsSurface) and be a parameter of the creating
API. Requiring OS(DARWIN) on the
> _caller_ side feels wrong.
> 
> Is that the direction you plan to take after this patch or in another
iteration?

I'd rather this be done with a runtime flag like
GraphicsSurface::isDoubleBufferred, even now. Then we can keep m_front... and
initialize the back one only if double buffered is not supported.