[chromium] Cannot create stencil render-buffer for accelerated drawing on desktop GL

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Alok Priyadarshi]

LayerTextureUpdaterCanvas.cpp:137: ASSERT(context()->checkFramebufferStatus(GraphicsContext3D::FRAMEBUFFER) == GraphicsContext3D::FRAMEBUFFER_COMPLETE);

This asserts for every composited page with desktop GL and --enable-accelerated-drawing. It basically fails to create a stencil render buffer.

[Alok Priyadarshi]

Created attachment 94849 [details]
proposed patch

[James Robinson]

Comment on attachment 94849 [details]
proposed patch

This is partially duplicating even more code from DrawingBuffer :(

[Alok Priyadarshi]

(In reply to comment #2)
> (From update of attachment 94849 [details])
> This is partially duplicating even more code from DrawingBuffer :(

All this code will go away once we delegate the creation of frame-buffer to SKIA. I have sent a proposal to skia team to make skia GrContext::createPlatformSurface more generic.

[James Robinson]

Comment on attachment 94849 [details]
proposed patch

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

[James Robinson]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 94849 [details] [details])
> > This is partially duplicating even more code from DrawingBuffer :(
> 
> All this code will go away once we delegate the creation of frame-buffer to SKIA. I have sent a proposal to skia team to make skia GrContext::createPlatformSurface more generic.

Yes but currently you and I are both wasting time maintaining it.

[Alok Priyadarshi]

Comment on attachment 94849 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=94849&action=review

Ideally you should try all the following configs:
1. STENCIL_INDEX8
2. DEPTH24_STENCIL8
3. STENCIL_INDEX4
4. STENCIL_INDEX16
5. STENCIL_INDEX
6. DEPTH_STENCIL

Since we are not checking all the possible configs, I would rather keep it simple and just try the most common one. SKIA does a pretty good job of iterating through all configs. So once we delegate the creation of frame-buffer to SKIA, we would not need to worry about it.

>> Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.cpp:163
>> +    if (m_stencilBuffer) {
> 
> this member variable is now misnamed - it's really a depthStencilBuffer

Done.

>> Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.cpp:195
>> +    extensions->ensureEnabled("GL_OES_packed_depth_stencil");
> 
> you should query+cached this once per context, not once per fbo

Caching (sort of) is already being done by Extensions3DChromium. It does not hit the IPC or command-buffer; just a hash-table look-up.

[Alok Priyadarshi]

Created attachment 94869 [details]
proposed patch

renamed m_stencilBuffer to m_depthStencilBuffer

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 94869 [details]:

http/tests/websocket/tests/frame-length-overflow.html bug 61507 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 94869 [details]
proposed patch

Clearing flags on attachment: 94869

Committed r87365: <http://trac.webkit.org/changeset/87365>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.