[webkit-reviews] review denied: [Bug 61444] [chromium] Cannot create stencil render-buffer for accelerated drawing on desktop GL : [Attachment 94849] proposed patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Alok Priyadarshi
<alokp at chromium.org>'s request for review:
Bug 61444: [chromium] Cannot create stencil render-buffer for accelerated
drawing on desktop GL
https://bugs.webkit.org/show_bug.cgi?id=61444

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

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

As I'm sure you know, I'd much prefer that you reuse existing code and refactor
it to meet your needs rather than partially reimplementing it.

Do you want to try falling back to color+stencil if the packed extension is
unavailable?  I think that in practice color+stencil is very rare, and it only
works in ANGLE because it's allocating a packed buffer behind the scenes, but
it might be worth trying.

> Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.cpp:163
> +    if (m_stencilBuffer) {

this member variable is now misnamed - it's really a depthStencilBuffer

> Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.cpp:195
> +    Extensions3D* extensions = context()->getExtensions();
> +    if (!extensions->supports("GL_OES_packed_depth_stencil"))
> +	   return false;
> +    extensions->ensureEnabled("GL_OES_packed_depth_stencil");

you should query+cached this once per context, not once per fbo