- Revision
- 240712
- Author
- [email protected]
- Date
- 2019-01-30 08:14:52 -0800 (Wed, 30 Jan 2019)
Log 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().
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (240711 => 240712)
--- trunk/Source/WebKit/ChangeLog 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/ChangeLog 2019-01-30 16:14:52 UTC (rev 240712)
@@ -1,3 +1,52 @@
+2019-01-30 Carlos Garcia Campos <[email protected]>
+
+ [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().
+
2019-01-29 Ryosuke Niwa <[email protected]>
iOS: Nullptr crash in WebPage::getPositionInformation dereferencing an input element for data list
Modified: trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp 2019-01-30 16:14:52 UTC (rev 240712)
@@ -1579,6 +1579,13 @@
webkitWebViewBase->priv->acceleratedBackingStore->update(LayerTreeContext());
}
+bool webkitWebViewBaseMakeGLContextCurrent(WebKitWebViewBase* webkitWebViewBase)
+{
+ if (webkitWebViewBase->priv->acceleratedBackingStore)
+ return webkitWebViewBase->priv->acceleratedBackingStore->makeContextCurrent();
+ return false;
+}
+
void webkitWebViewBaseDidRelaunchWebProcess(WebKitWebViewBase* webkitWebViewBase)
{
// Queue a resize to ensure the new DrawingAreaProxy is resized.
Modified: trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h 2019-01-30 16:14:52 UTC (rev 240712)
@@ -67,6 +67,7 @@
void webkitWebViewBaseEnterAcceleratedCompositingMode(WebKitWebViewBase*, const WebKit::LayerTreeContext&);
void webkitWebViewBaseUpdateAcceleratedCompositingMode(WebKitWebViewBase*, const WebKit::LayerTreeContext&);
void webkitWebViewBaseExitAcceleratedCompositingMode(WebKitWebViewBase*);
+bool webkitWebViewBaseMakeGLContextCurrent(WebKitWebViewBase*);
void webkitWebViewBaseDidRelaunchWebProcess(WebKitWebViewBase*);
void webkitWebViewBasePageClosed(WebKitWebViewBase*);
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2019-01-30 16:14:52 UTC (rev 240712)
@@ -759,6 +759,7 @@
PlatformWidget viewWidget();
const WebCore::Color& backgroundColor() const { return m_backgroundColor; }
void setBackgroundColor(const WebCore::Color& color) { m_backgroundColor = color; }
+ bool makeGLContextCurrent();
#endif
#if PLATFORM(WIN)
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h 2019-01-30 16:14:52 UTC (rev 240712)
@@ -46,6 +46,7 @@
virtual void update(const LayerTreeContext&) { }
virtual bool paint(cairo_t*, const WebCore::IntRect&);
+ virtual bool makeContextCurrent() { return false; }
protected:
AcceleratedBackingStore(WebPageProxy&);
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp 2019-01-30 16:14:52 UTC (rev 240712)
@@ -31,7 +31,7 @@
#include "WaylandCompositor.h"
#include "WebPageProxy.h"
#include <WebCore/CairoUtilities.h>
-#include <WebCore/RefPtrCairo.h>
+#include <WebCore/GLContext.h>
#if USE(OPENGL_ES)
#include <GLES2/gl2.h>
@@ -60,31 +60,43 @@
WaylandCompositor::singleton().unregisterWebPage(m_webPage);
}
-#if GTK_CHECK_VERSION(3, 16, 0)
-bool AcceleratedBackingStoreWayland::canGdkUseGL() const
+void AcceleratedBackingStoreWayland::tryEnsureGLContext()
{
- static bool initialized = false;
- static bool canCreateGLContext = false;
+ if (m_glContextInitialized)
+ return;
- if (initialized)
- return canCreateGLContext;
+ m_glContextInitialized = true;
- initialized = true;
-
+#if GTK_CHECK_VERSION(3, 16, 0)
GUniqueOutPtr<GError> error;
- GdkWindow* gdkWindow = gtk_widget_get_window(m_webPage.viewWidget());
- GRefPtr<GdkGLContext> gdkContext(gdk_window_create_gl_context(gdkWindow, &error.outPtr()));
- if (!gdkContext) {
- g_warning("GDK is not able to create a GL context, falling back to glReadPixels (slow!): %s", error->message);
- return false;
+ m_gdkGLContext = adoptGRef(gdk_window_create_gl_context(gtk_widget_get_window(m_webPage.viewWidget()), &error.outPtr()));
+ if (m_gdkGLContext) {
+#if USE(OPENGL_ES)
+ gdk_gl_context_set_use_es(m_gdkGLContext.get(), TRUE);
+#endif
+ return;
}
- canCreateGLContext = true;
+ g_warning("GDK is not able to create a GL context, falling back to glReadPixels (slow!): %s", error->message);
+#endif
- return true;
+ m_glContext = GLContext::createOffscreenContext();
}
+
+bool AcceleratedBackingStoreWayland::makeContextCurrent()
+{
+ tryEnsureGLContext();
+
+#if GTK_CHECK_VERSION(3, 16, 0)
+ if (m_gdkGLContext) {
+ gdk_gl_context_make_current(m_gdkGLContext.get());
+ return true;
+ }
#endif
+ return m_glContext ? m_glContext->makeContextCurrent() : false;
+}
+
bool AcceleratedBackingStoreWayland::paint(cairo_t* cr, const IntRect& clipRect)
{
GLuint texture;
@@ -96,7 +108,7 @@
AcceleratedBackingStore::paint(cr, clipRect);
#if GTK_CHECK_VERSION(3, 16, 0)
- if (canGdkUseGL()) {
+ if (m_gdkGLContext) {
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());
cairo_restore(cr);
return true;
@@ -103,6 +115,8 @@
}
#endif
+ ASSERT(m_glContext);
+
if (!m_surface || cairo_image_surface_get_width(m_surface.get()) != textureSize.width() || cairo_image_surface_get_height(m_surface.get()) != textureSize.height())
m_surface = adoptRef(cairo_image_surface_create(CAIRO_FORMAT_ARGB32, textureSize.width(), textureSize.height()));
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h 2019-01-30 16:14:52 UTC (rev 240712)
@@ -31,7 +31,14 @@
#include <WebCore/RefPtrCairo.h>
#include <gtk/gtk.h>
+#include <wtf/glib/GRefPtr.h>
+typedef struct _GdkGLContext GdkGLContext;
+
+namespace WebCore {
+class GLContext;
+}
+
namespace WebKit {
class WebPageProxy;
@@ -42,16 +49,20 @@
static std::unique_ptr<AcceleratedBackingStoreWayland> create(WebPageProxy&);
~AcceleratedBackingStoreWayland();
-#if GTK_CHECK_VERSION(3, 16, 0)
- bool canGdkUseGL() const;
-#endif
-
private:
AcceleratedBackingStoreWayland(WebPageProxy&);
+ void tryEnsureGLContext();
+
bool paint(cairo_t*, const WebCore::IntRect&) override;
+ bool makeContextCurrent() override;
RefPtr<cairo_surface_t> m_surface;
+ bool m_glContextInitialized { false };
+#if GTK_CHECK_VERSION(3, 16, 0)
+ GRefPtr<GdkGLContext> m_gdkGLContext;
+#endif
+ std::unique_ptr<WebCore::GLContext> m_glContext;
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp 2019-01-30 16:14:52 UTC (rev 240712)
@@ -34,6 +34,7 @@
#include <WebCore/GLContext.h>
#include <WebCore/PlatformDisplayWayland.h>
#include <WebCore/Region.h>
+#include <gtk/gtk.h>
#include <wayland-server-protocol.h>
#include <wtf/UUID.h>
@@ -146,12 +147,6 @@
WaylandCompositor::Surface::Surface()
: m_image(EGL_NO_IMAGE_KHR)
{
- glGenTextures(1, &m_texture);
- glBindTexture(GL_TEXTURE_2D, m_texture);
- glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
- glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
- glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
- glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
}
WaylandCompositor::Surface::~Surface()
@@ -168,11 +163,6 @@
if (m_buffer)
m_buffer->unuse();
-
- if (m_image != EGL_NO_IMAGE_KHR)
- eglDestroyImage(PlatformDisplay::sharedDisplay().eglDisplay(), m_image);
-
- glDeleteTextures(1, &m_texture);
}
void WaylandCompositor::Surface::setWebPage(WebPageProxy* webPage)
@@ -182,6 +172,16 @@
flushFrameCallbacks();
gtk_widget_remove_tick_callback(m_webPage->viewWidget(), m_tickCallbackID);
m_tickCallbackID = 0;
+
+ if (m_webPage->makeGLContextCurrent()) {
+ if (m_image != EGL_NO_IMAGE_KHR)
+ eglDestroyImage(PlatformDisplay::sharedDisplay().eglDisplay(), m_image);
+ if (m_texture)
+ glDeleteTextures(1, &m_texture);
+ }
+
+ m_image = EGL_NO_IMAGE_KHR;
+ m_texture = 0;
}
m_webPage = webPage;
@@ -188,6 +188,15 @@
if (!m_webPage)
return;
+ if (m_webPage->makeGLContextCurrent()) {
+ glGenTextures(1, &m_texture);
+ glBindTexture(GL_TEXTURE_2D, m_texture);
+ glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+ glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+ glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+ glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+ }
+
m_tickCallbackID = gtk_widget_add_tick_callback(m_webPage->viewWidget(), [](GtkWidget*, GdkFrameClock*, gpointer userData) -> gboolean {
auto* surface = static_cast<Surface*>(userData);
surface->flushFrameCallbacks();
@@ -232,9 +241,12 @@
bool WaylandCompositor::Surface::prepareTextureForPainting(unsigned& texture, IntSize& textureSize)
{
- if (m_image == EGL_NO_IMAGE_KHR)
+ if (!m_texture || m_image == EGL_NO_IMAGE_KHR)
return false;
+ if (!m_webPage || !m_webPage->makeGLContextCurrent())
+ return false;
+
glBindTexture(GL_TEXTURE_2D, m_texture);
glImageTargetTexture2D(GL_TEXTURE_2D, m_image);
@@ -263,7 +275,7 @@
void WaylandCompositor::Surface::commit()
{
- if (!m_webPage) {
+ if (!m_webPage || !m_webPage->makeGLContextCurrent()) {
makePendingBufferCurrent();
flushPendingFrameCallbacks();
return;
@@ -400,11 +412,11 @@
return false;
}
- m_eglContext = GLContext::createOffscreenContext();
- if (!m_eglContext)
+ std::unique_ptr<WebCore::GLContext> eglContext = GLContext::createOffscreenContext();
+ if (!eglContext)
return false;
- if (!m_eglContext->makeContextCurrent())
+ if (!eglContext->makeContextCurrent())
return false;
#if USE(OPENGL_ES)
Modified: trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h 2019-01-30 16:14:52 UTC (rev 240712)
@@ -30,7 +30,6 @@
#include "WebPageProxy.h"
#include <WebCore/RefPtrCairo.h>
#include <WebCore/WlUniquePtr.h>
-#include <gtk/gtk.h>
#include <wayland-server.h>
#include <wtf/HashMap.h>
#include <wtf/NeverDestroyed.h>
@@ -41,10 +40,6 @@
typedef void *EGLImageKHR;
-namespace WebCore {
-class GLContext;
-}
-
namespace WebKit {
class WebPageProxy;
@@ -96,7 +91,7 @@
WeakPtr<Buffer> m_buffer;
WeakPtr<Buffer> m_pendingBuffer;
- unsigned m_texture;
+ unsigned m_texture { 0 };
EGLImageKHR m_image;
WebCore::IntSize m_imageSize;
Vector<wl_resource*> m_pendingFrameCallbackList;
@@ -129,7 +124,6 @@
WebCore::WlUniquePtr<struct wl_global> m_compositorGlobal;
WebCore::WlUniquePtr<struct wl_global> m_webkitgtkGlobal;
GRefPtr<GSource> m_eventSource;
- std::unique_ptr<WebCore::GLContext> m_eglContext;
HashMap<WebPageProxy*, WeakPtr<Surface>> m_pageMap;
};
Modified: trunk/Source/WebKit/UIProcess/gtk/WebPageProxyGtk.cpp (240711 => 240712)
--- trunk/Source/WebKit/UIProcess/gtk/WebPageProxyGtk.cpp 2019-01-30 16:13:07 UTC (rev 240711)
+++ trunk/Source/WebKit/UIProcess/gtk/WebPageProxyGtk.cpp 2019-01-30 16:14:52 UTC (rev 240712)
@@ -154,4 +154,9 @@
}
#endif
+bool WebPageProxy::makeGLContextCurrent()
+{
+ return webkitWebViewBaseMakeGLContextCurrent(WEBKIT_WEB_VIEW_BASE(viewWidget()));
+}
+
} // namespace WebKit