Changeset 240712 in webkit


Ignore:
Timestamp:
Jan 30, 2019 8:14:52 AM (5 years ago)
Author:
Carlos Garcia Campos
Message:

[GTK] gdk_cairo_draw_from_gl() in AcceleratedBackingStoreWayland fails in GtkInspector's magnifier
​https://bugs.webkit.org/show_bug.cgi?id=193903

Reviewed by Michael Catanzaro.

The problem is that the GL context used by WaylandCompositor can't share resources with the one used by GTK+
when painting with gdk_cairo_draw_from_gl(). Accelerated compositing in Wayland works only because
WaylandCompositor makes the context current only once on initialization. So, when we render the first frame on
accelerated compositing mode, GTK+ is rendering in non-GL mode, and switches to the GL mode when
gdk_cairo_draw_from_gl() is called. Since GTK+ didn't have a GL context yet, the first frame is always rendered
by GTK+ using the software fallback (glReadPixels). The thing is that the first time gdk_cairo_draw_from_gl() is
called, GTK+ creates a GL context for painting that is made current, and it will remain the current one
forever. The first frame fails to render with "GL_INVALID_OPERATION in glBindTexture(non-gen name)" because the
texture created in WaylandCompositor GL context can't be accessed from GTK+ GL context. The following frames are
handled with the GTK+ GL context. I would say this works by casuality and it could be the cause of other
accelerated compositing issues in Wayland.

We need to create our own GdkGLContext for the WebView, and use that in the WaylandCompositor. When the
GdkGLContext is created, the GTK+ GL context for painting is used as a shared context, ensuring that resources
created in the new context will be accessible from the painting one.

  • UIProcess/API/gtk/WebKitWebViewBase.cpp:

(webkitWebViewBaseMakeGLContextCurrent): Call AcceleratedBackingStore::makeContextCurrent().

  • UIProcess/API/gtk/WebKitWebViewBasePrivate.h:
  • UIProcess/WebPageProxy.h:
  • UIProcess/gtk/AcceleratedBackingStore.h:

(WebKit::AcceleratedBackingStore::makeContextCurrent): New virtual method only implemented by Wayland backend.

  • UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:

(WebKit::AcceleratedBackingStoreWayland::tryEnsureGLContext): Try to create a GL context with
gdk_window_create_gl_context(), falling back to a WebCore::GLContext if it fails or GTK+ version is not new enough.
(WebKit::AcceleratedBackingStoreWayland::makeContextCurrent): Make the GL context current.
(WebKit::AcceleratedBackingStoreWayland::paint): Check if we have a GdkGLContext before trying to use gdk_cairo_draw_from_gl().
(WebKit::AcceleratedBackingStoreWayland::canGdkUseGL const): Deleted.

  • UIProcess/gtk/AcceleratedBackingStoreWayland.h:
  • UIProcess/gtk/WaylandCompositor.cpp:

(WebKit::WaylandCompositor::Surface::Surface): Move the texture creation to setWebPage(), since we need the
WebView GL context.
(WebKit::WaylandCompositor::Surface::~Surface): Move the code to destroy GL resources to setWebPage().
(WebKit::WaylandCompositor::Surface::setWebPage): Create the texture when a new page is set and destroy GL
resources when unset.
(WebKit::WaylandCompositor::Surface::prepareTextureForPainting): Make WebView GL context current.
(WebKit::WaylandCompositor::Surface::commit): Ditto.
(WebKit::WaylandCompositor::initializeEGL): Use a temporary GLContext.

  • UIProcess/gtk/WaylandCompositor.h:
  • UIProcess/gtk/WebPageProxyGtk.cpp:

(WebKit::WebPageProxy::makeGLContextCurrent): Call webkitWebViewBaseMakeGLContextCurrent().

Location:
trunk/Source/WebKit
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r240702 r240712  
     12019-01-30  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] gdk_cairo_draw_from_gl() in AcceleratedBackingStoreWayland fails in GtkInspector's magnifier
     4        https://bugs.webkit.org/show_bug.cgi?id=193903
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        The problem is that the GL context used by WaylandCompositor can't share resources with the one used by GTK+
     9        when painting with gdk_cairo_draw_from_gl(). Accelerated compositing in Wayland works only because
     10        WaylandCompositor makes the context current only once on initialization. So, when we render the first frame on
     11        accelerated compositing mode, GTK+ is rendering in non-GL mode, and switches to the GL mode when
     12        gdk_cairo_draw_from_gl() is called. Since GTK+ didn't have a GL context yet, the first frame is always rendered
     13        by GTK+ using the software fallback (glReadPixels). The thing is that the first time gdk_cairo_draw_from_gl() is
     14        called, GTK+ creates a GL context for painting that is made current, and it will remain the current one
     15        forever. The first frame fails to render with "GL_INVALID_OPERATION in glBindTexture(non-gen name)" because the
     16        texture created in WaylandCompositor GL context can't be accessed from GTK+ GL context. The following frames are
     17        handled with the GTK+ GL context. I would say this works by casuality and it could be the cause of other
     18        accelerated compositing issues in Wayland.
     19
     20        We need to create our own GdkGLContext for the WebView, and use that in the WaylandCompositor. When the
     21        GdkGLContext is created, the GTK+ GL context for painting is used as a shared context, ensuring that resources
     22        created in the new context will be accessible from the painting one.
     23
     24        * UIProcess/API/gtk/WebKitWebViewBase.cpp:
     25        (webkitWebViewBaseMakeGLContextCurrent): Call AcceleratedBackingStore::makeContextCurrent().
     26        * UIProcess/API/gtk/WebKitWebViewBasePrivate.h:
     27        * UIProcess/WebPageProxy.h:
     28        * UIProcess/gtk/AcceleratedBackingStore.h:
     29        (WebKit::AcceleratedBackingStore::makeContextCurrent): New virtual method only implemented by Wayland backend.
     30        * UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:
     31        (WebKit::AcceleratedBackingStoreWayland::tryEnsureGLContext): Try to create a GL context with
     32        gdk_window_create_gl_context(), falling back to a WebCore::GLContext if it fails or GTK+ version is not new enough.
     33        (WebKit::AcceleratedBackingStoreWayland::makeContextCurrent): Make the GL context current.
     34        (WebKit::AcceleratedBackingStoreWayland::paint): Check if we have a GdkGLContext before trying to use gdk_cairo_draw_from_gl().
     35        (WebKit::AcceleratedBackingStoreWayland::canGdkUseGL const): Deleted.
     36        * UIProcess/gtk/AcceleratedBackingStoreWayland.h:
     37        * UIProcess/gtk/WaylandCompositor.cpp:
     38        (WebKit::WaylandCompositor::Surface::Surface): Move the texture creation to setWebPage(), since we need the
     39        WebView GL context.
     40        (WebKit::WaylandCompositor::Surface::~Surface): Move the code to destroy GL resources to setWebPage().
     41        (WebKit::WaylandCompositor::Surface::setWebPage): Create the texture when a new page is set and destroy GL
     42        resources when unset.
     43        (WebKit::WaylandCompositor::Surface::prepareTextureForPainting): Make WebView GL context current.
     44        (WebKit::WaylandCompositor::Surface::commit): Ditto.
     45        (WebKit::WaylandCompositor::initializeEGL): Use a temporary GLContext.
     46        * UIProcess/gtk/WaylandCompositor.h:
     47        * UIProcess/gtk/WebPageProxyGtk.cpp:
     48        (WebKit::WebPageProxy::makeGLContextCurrent): Call webkitWebViewBaseMakeGLContextCurrent().
     49
    1502019-01-29  Ryosuke Niwa  <rniwa@webkit.org>
    251

  • trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp

    r239461 r240712  
    15801580}
    15811581
     1582bool webkitWebViewBaseMakeGLContextCurrent(WebKitWebViewBase* webkitWebViewBase)
     1583{
     1584    if (webkitWebViewBase->priv->acceleratedBackingStore)
     1585        return webkitWebViewBase->priv->acceleratedBackingStore->makeContextCurrent();
     1586    return false;
     1587}
     1588
    15821589void webkitWebViewBaseDidRelaunchWebProcess(WebKitWebViewBase* webkitWebViewBase)
    15831590{

  • trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h

    r236004 r240712  
    6868void webkitWebViewBaseUpdateAcceleratedCompositingMode(WebKitWebViewBase*, const WebKit::LayerTreeContext&);
    6969void webkitWebViewBaseExitAcceleratedCompositingMode(WebKitWebViewBase*);
     70bool webkitWebViewBaseMakeGLContextCurrent(WebKitWebViewBase*);
    7071void webkitWebViewBaseDidRelaunchWebProcess(WebKitWebViewBase*);
    7172void webkitWebViewBasePageClosed(WebKitWebViewBase*);

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r240687 r240712  
    760760    const WebCore::Color& backgroundColor() const { return m_backgroundColor; }
    761761    void setBackgroundColor(const WebCore::Color& color) { m_backgroundColor = color; }
     762    bool makeGLContextCurrent();
    762763#endif
    763764

  • trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h

    r205116 r240712  
    4747    virtual void update(const LayerTreeContext&) { }
    4848    virtual bool paint(cairo_t*, const WebCore::IntRect&);
     49    virtual bool makeContextCurrent() { return false; }
    4950
    5051protected:

  • trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp

    r235265 r240712  
    3232#include "WebPageProxy.h"
    3333#include <WebCore/CairoUtilities.h>
    34 #include <WebCore/RefPtrCairo.h>
     34#include <WebCore/GLContext.h>
    3535
    3636#if USE(OPENGL_ES)
    … …  
    6161}
    6262
     63void AcceleratedBackingStoreWayland::tryEnsureGLContext()
     64{
     65    if (m_glContextInitialized)
     66        return;
     67
     68    m_glContextInitialized = true;
     69
    6370#if GTK_CHECK_VERSION(3, 16, 0)
    64 bool AcceleratedBackingStoreWayland::canGdkUseGL() const
    65 {
    66     static bool initialized = false;
    67     static bool canCreateGLContext = false;
    68 
    69     if (initialized)
    70         return canCreateGLContext;
    71 
    72     initialized = true;
    73 
    7471    GUniqueOutPtr<GError> error;
    75     GdkWindow* gdkWindow = gtk_widget_get_window(m_webPage.viewWidget());
    76     GRefPtr<GdkGLContext> gdkContext(gdk_window_create_gl_context(gdkWindow, &error.outPtr()));
    77     if (!gdkContext) {
    78         g_warning("GDK is not able to create a GL context, falling back to glReadPixels (slow!): %s", error->message);
    79         return false;
     72    m_gdkGLContext = adoptGRef(gdk_window_create_gl_context(gtk_widget_get_window(m_webPage.viewWidget()), &error.outPtr()));
     73    if (m_gdkGLContext) {
     74#if USE(OPENGL_ES)
     75        gdk_gl_context_set_use_es(m_gdkGLContext.get(), TRUE);
     76#endif
     77        return;
    8078    }
    8179
    82     canCreateGLContext = true;
     80    g_warning("GDK is not able to create a GL context, falling back to glReadPixels (slow!): %s", error->message);
     81#endif
    8382
    84     return true;
     83    m_glContext = GLContext::createOffscreenContext();
    8584}
     85
     86bool AcceleratedBackingStoreWayland::makeContextCurrent()
     87{
     88    tryEnsureGLContext();
     89
     90#if GTK_CHECK_VERSION(3, 16, 0)
     91    if (m_gdkGLContext) {
     92        gdk_gl_context_make_current(m_gdkGLContext.get());
     93        return true;
     94    }
    8695#endif
     96
     97    return m_glContext ? m_glContext->makeContextCurrent() : false;
     98}
    8799
    88100bool AcceleratedBackingStoreWayland::paint(cairo_t* cr, const IntRect& clipRect)
    … …  
    97109
    98110#if GTK_CHECK_VERSION(3, 16, 0)
    99     if (canGdkUseGL()) {
     111    if (m_gdkGLContext) {
    100112        gdk_cairo_draw_from_gl(cr, gtk_widget_get_window(m_webPage.viewWidget()), texture, GL_TEXTURE, m_webPage.deviceScaleFactor(), 0, 0, textureSize.width(), textureSize.height());
    101113        cairo_restore(cr);
    … …  
    103115    }
    104116#endif
     117
     118    ASSERT(m_glContext);
    105119
    106120    if (!m_surface || cairo_image_surface_get_width(m_surface.get()) != textureSize.width() || cairo_image_surface_get_height(m_surface.get()) != textureSize.height())

  • trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h

    r206424 r240712  
    3232#include <WebCore/RefPtrCairo.h>
    3333#include <gtk/gtk.h>
     34#include <wtf/glib/GRefPtr.h>
     35
     36typedef struct _GdkGLContext GdkGLContext;
     37
     38namespace WebCore {
     39class GLContext;
     40}
    3441
    3542namespace WebKit {
    … …  
    4350    ~AcceleratedBackingStoreWayland();
    4451
    45 #if GTK_CHECK_VERSION(3, 16, 0)
    46     bool canGdkUseGL() const;
    47 #endif
    48 
    4952private:
    5053    AcceleratedBackingStoreWayland(WebPageProxy&);
    5154
     55    void tryEnsureGLContext();
     56
    5257    bool paint(cairo_t*, const WebCore::IntRect&) override;
     58    bool makeContextCurrent() override;
    5359
    5460    RefPtr<cairo_surface_t> m_surface;
     61    bool m_glContextInitialized { false };
     62#if GTK_CHECK_VERSION(3, 16, 0)
     63    GRefPtr<GdkGLContext> m_gdkGLContext;
     64#endif
     65    std::unique_ptr<WebCore::GLContext> m_glContext;
    5566};
    5667

  • trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp

    r234887 r240712  
    3535#include <WebCore/PlatformDisplayWayland.h>
    3636#include <WebCore/Region.h>
     37#include <gtk/gtk.h>
    3738#include <wayland-server-protocol.h>
    3839#include <wtf/UUID.h>
    … …  
    147148    : m_image(EGL_NO_IMAGE_KHR)
    148149{
    149     glGenTextures(1, &m_texture);
    150     glBindTexture(GL_TEXTURE_2D, m_texture);
    151     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
    152     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
    153     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
    154     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
    155150}
    156151
    … …  
    169164    if (m_buffer)
    170165        m_buffer->unuse();
    171 
    172     if (m_image != EGL_NO_IMAGE_KHR)
    173         eglDestroyImage(PlatformDisplay::sharedDisplay().eglDisplay(), m_image);
    174 
    175     glDeleteTextures(1, &m_texture);
    176166}
    177167
    … …  
    183173        gtk_widget_remove_tick_callback(m_webPage->viewWidget(), m_tickCallbackID);
    184174        m_tickCallbackID = 0;
     175
     176        if (m_webPage->makeGLContextCurrent()) {
     177            if (m_image != EGL_NO_IMAGE_KHR)
     178                eglDestroyImage(PlatformDisplay::sharedDisplay().eglDisplay(), m_image);
     179            if (m_texture)
     180                glDeleteTextures(1, &m_texture);
     181        }
     182
     183        m_image = EGL_NO_IMAGE_KHR;
     184        m_texture = 0;
    185185    }
    186186
    … …  
    188188    if (!m_webPage)
    189189        return;
     190
     191    if (m_webPage->makeGLContextCurrent()) {
     192        glGenTextures(1, &m_texture);
     193        glBindTexture(GL_TEXTURE_2D, m_texture);
     194        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
     195        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
     196        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
     197        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
     198    }
    190199
    191200    m_tickCallbackID = gtk_widget_add_tick_callback(m_webPage->viewWidget(), [](GtkWidget*, GdkFrameClock*, gpointer userData) -> gboolean {
    … …  
    233242bool WaylandCompositor::Surface::prepareTextureForPainting(unsigned& texture, IntSize& textureSize)
    234243{
    235     if (m_image == EGL_NO_IMAGE_KHR)
     244    if (!m_texture || m_image == EGL_NO_IMAGE_KHR)
     245        return false;
     246
     247    if (!m_webPage || !m_webPage->makeGLContextCurrent())
    236248        return false;
    237249
    … …  
    264276void WaylandCompositor::Surface::commit()
    265277{
    266     if (!m_webPage) {
     278    if (!m_webPage || !m_webPage->makeGLContextCurrent()) {
    267279        makePendingBufferCurrent();
    268280        flushPendingFrameCallbacks();
    … …  
    401413    }
    402414
    403     m_eglContext = GLContext::createOffscreenContext();
    404     if (!m_eglContext)
    405         return false;
    406 
    407     if (!m_eglContext->makeContextCurrent())
     415    std::unique_ptr<WebCore::GLContext> eglContext = GLContext::createOffscreenContext();
     416    if (!eglContext)
     417        return false;
     418
     419    if (!eglContext->makeContextCurrent())
    408420        return false;
    409421

  • trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h

    r234887 r240712  
    3131#include <WebCore/RefPtrCairo.h>
    3232#include <WebCore/WlUniquePtr.h>
    33 #include <gtk/gtk.h>
    3433#include <wayland-server.h>
    3534#include <wtf/HashMap.h>
    … …  
    4140
    4241typedef void *EGLImageKHR;
    43 
    44 namespace WebCore {
    45 class GLContext;
    46 }
    4742
    4843namespace WebKit {
    … …  
    9792        WeakPtr<Buffer> m_buffer;
    9893        WeakPtr<Buffer> m_pendingBuffer;
    99         unsigned m_texture;
     94        unsigned m_texture { 0 };
    10095        EGLImageKHR m_image;
    10196        WebCore::IntSize m_imageSize;
    … …  
    130125    WebCore::WlUniquePtr<struct wl_global> m_webkitgtkGlobal;
    131126    GRefPtr<GSource> m_eventSource;
    132     std::unique_ptr<WebCore::GLContext> m_eglContext;
    133127    HashMap<WebPageProxy*, WeakPtr<Surface>> m_pageMap;
    134128};

  • trunk/Source/WebKit/UIProcess/gtk/WebPageProxyGtk.cpp

    r235903 r240712  
    155155#endif
    156156
     157bool WebPageProxy::makeGLContextCurrent()
     158{
     159    return webkitWebViewBaseMakeGLContextCurrent(WEBKIT_WEB_VIEW_BASE(viewWidget()));
     160}
     161
    157162} // namespace WebKit
Note: See TracChangeset for help on using the changeset viewer.