Bug 166643 - [Cairo] Ensure depth and stencil renderbuffers are created on GLESv2
Summary: [Cairo] Ensure depth and stencil renderbuffers are created on GLESv2
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Gtk (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-02 15:58 PST by Emanuele Aina
Modified: 2017-03-21 04:01 PDT (History)
12 users (show)

See Also:


Attachments
Patch (4.10 KB, patch)
2017-01-02 16:00 PST, Emanuele Aina
no flags Details | Formatted Diff | Diff
Patch for landing (4.76 KB, patch)
2017-03-20 01:12 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Emanuele Aina 2017-01-02 15:58:32 PST
If the gfx device doesn't support GL_OES_packed_depth_stencil, the separate depth and stencil buffers are not generated.

Copy what GraphicsContext3DEfl does and apply it in GraphicsContext3DCairo.
                                                                                                                        
The Intel gfx driver seem to tolerate unbound renderbuffers, but enabling debugging in Mesa yields an error:

$ MESA_DEBUG=1 \
  MESA_EXTENSION_OVERRIDE=-GL_OES_packed_depth_stencil
  ./bin/MiniBrowser http://webglsamples.org/aquarium/aquarium.html
Mesa: User error: GL_INVALID_OPERATION in glRenderbufferStorage(no renderbuffer bound)
Comment 1 Emanuele Aina 2017-01-02 16:00:39 PST
Created attachment 297917 [details]
Patch
Comment 2 Darin Adler 2017-01-02 18:31:45 PST
Comment on attachment 297917 [details]
Patch

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

> w/Source/WebCore/ChangeLog:3
> +        Ensure depth and stencil renderbuffers are created on GLESv2

Bug should have [Cairo] in the subject since it’s a Cairo-specific patch and bug fix.

> w/Source/WebCore/platform/graphics/cairo/GraphicsContext3DCairo.cpp:91
> +    , m_depthBuffer(0)
> +    , m_stencilBuffer(0)

Should initialize these in the class definition rather than adding more conditionals to the .cpp file.
Comment 3 Alex Christensen 2017-01-05 12:04:15 PST
Comment on attachment 297917 [details]
Patch

Yep, we want more things like m_depthBuffer { nullptr }; in the header instead of in the cpp.
Comment 4 Zan Dobersek 2017-03-20 01:12:26 PDT
Created attachment 304919 [details]
Patch for landing
Comment 5 Zan Dobersek 2017-03-20 01:17:18 PDT
Addressed the review comments.

Sorry for taking this over, but I want to finally push it through into the source tree.
Comment 6 WebKit Commit Bot 2017-03-20 01:47:26 PDT
Comment on attachment 304919 [details]
Patch for landing

Clearing flags on attachment: 304919

Committed r214162: <http://trac.webkit.org/changeset/214162>
Comment 7 WebKit Commit Bot 2017-03-20 01:47:33 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Emanuele Aina 2017-03-21 04:01:20 PDT
You're more than welcome to take over any patch from me. My time to work on WebKit is sadly spotty at best, so I really appreciate any help.

Thanks Zan for landing this! :D