Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REGRESSION(2.41.6): [GTK] Yelp help viewer and Epiphany browser do not show content on a virtual machine (llvmpipe?) with WebKitGTK 2.41.6 #15929

Merged

Conversation

carlosgcampos
Copy link
Contributor

@carlosgcampos carlosgcampos commented Jul 19, 2023

ad44d7b

REGRESSION(2.41.6): [GTK] Yelp help viewer and Epiphany browser do not show content on a virtual machine (llvmpipe?) with WebKitGTK 2.41.6
https://bugs.webkit.org/show_bug.cgi?id=259320

Reviewed by Michael Catanzaro.

The problem is that for some reason the surfaceless platform is claiming
to support EGL_MESA_image_dma_buf_export with llvmpipe driver, while the
display in the UI process doesn't support EGL_EXT_image_dma_buf_import
with llvmpipe as expected. So, we end up exporting DMABuf buffers (or
trying) that the UI process can't handle. We should always pass to the
web process the buffers supported by the UI process (hardware or shared
memory) and ensure we only export buffers supported by the UI process.

* Source/WebKit/Shared/WebProcessCreationParameters.h:
* Source/WebKit/Shared/WebProcessCreationParameters.serialization.in:
* Source/WebKit/Shared/glib/DMABufRendererBufferMode.h: Added.
* Source/WebKit/UIProcess/API/glib/WebKitProtocolHandler.cpp:
(WebKit::dmabufRendererWithSupportedBuffers):
(WebKit::WebKitProtocolHandler::handleGPU):
* Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:
(WebKit::WebProcessPool::platformInitializeWebProcess):
* Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp:
(WebKit::AcceleratedBackingStoreDMABuf::rendererBufferMode):
(WebKit::AcceleratedBackingStoreDMABuf::checkRequirements):
* Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.h:
* Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceDMABuf.cpp:
(WebKit::AcceleratedSurfaceDMABuf::clientResize):
* Source/WebKit/WebProcess/WebProcess.h:
(WebKit::WebProcess::dmaBufRendererBufferMode const):
* Source/WebKit/WebProcess/glib/WebProcessGLib.cpp:
(WebKit::WebProcess::platformInitializeWebProcess):

Canonical link: https://commits.webkit.org/266201@main

d645998

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim
βœ… πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim

@carlosgcampos carlosgcampos requested review from a team and cdumez as code owners July 19, 2023 12:01
@carlosgcampos carlosgcampos self-assigned this Jul 19, 2023
@carlosgcampos carlosgcampos added the WebKitGTK Bugs related to the Gtk API layer. label Jul 19, 2023
@AdamWill
Copy link

I tested this, it works. Sorry for the delay, the scratch build took so long I couldn't check it yesterday.

@carlosgcampos carlosgcampos added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 21, 2023
…t show content on a virtual machine (llvmpipe?) with WebKitGTK 2.41.6

https://bugs.webkit.org/show_bug.cgi?id=259320

Reviewed by Michael Catanzaro.

The problem is that for some reason the surfaceless platform is claiming
to support EGL_MESA_image_dma_buf_export with llvmpipe driver, while the
display in the UI process doesn't support EGL_EXT_image_dma_buf_import
with llvmpipe as expected. So, we end up exporting DMABuf buffers (or
trying) that the UI process can't handle. We should always pass to the
web process the buffers supported by the UI process (hardware or shared
memory) and ensure we only export buffers supported by the UI process.

* Source/WebKit/Shared/WebProcessCreationParameters.h:
* Source/WebKit/Shared/WebProcessCreationParameters.serialization.in:
* Source/WebKit/Shared/glib/DMABufRendererBufferMode.h: Added.
* Source/WebKit/UIProcess/API/glib/WebKitProtocolHandler.cpp:
(WebKit::dmabufRendererWithSupportedBuffers):
(WebKit::WebKitProtocolHandler::handleGPU):
* Source/WebKit/UIProcess/glib/WebProcessPoolGLib.cpp:
(WebKit::WebProcessPool::platformInitializeWebProcess):
* Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp:
(WebKit::AcceleratedBackingStoreDMABuf::rendererBufferMode):
(WebKit::AcceleratedBackingStoreDMABuf::checkRequirements):
* Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.h:
* Source/WebKit/WebProcess/WebPage/gtk/AcceleratedSurfaceDMABuf.cpp:
(WebKit::AcceleratedSurfaceDMABuf::clientResize):
* Source/WebKit/WebProcess/WebProcess.h:
(WebKit::WebProcess::dmaBufRendererBufferMode const):
* Source/WebKit/WebProcess/glib/WebProcessGLib.cpp:
(WebKit::WebProcess::platformInitializeWebProcess):

Canonical link: https://commits.webkit.org/266201@main
@webkit-commit-queue
Copy link
Collaborator

Committed 266201@main (ad44d7b): https://commits.webkit.org/266201@main

Reviewed commits have been landed. Closing PR #15929 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit ad44d7b into WebKit:main Jul 21, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 21, 2023
@carlosgcampos carlosgcampos deleted the gtk-dmabuf-renderer-buffer-mode branch July 21, 2023 09:56
@ThomasDevoogdt
Copy link
Contributor

Hi, this broke the build when GBM was not enabled. I'm searching for how to fix it, but it isn't that clear.
Will send an update if I have some progress.

This makes AcceleratedBackingStoreDMABuf compiling, but still stuck with AcceleratedSurfaceDMABuf.

diff --git a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp
index 63b699e8944e..8fbff0272c90 100644
--- a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp
+++ b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp
@@ -33,14 +33,14 @@
 #include "ShareableBitmap.h"
 #include "WebPageProxy.h"
 #include "WebProcessProxy.h"
-#include <WebCore/DMABufFormat.h>
 #include <WebCore/GLContext.h>
 #include <WebCore/IntRect.h>
 #include <WebCore/PlatformDisplay.h>
-#include <epoxy/egl.h>
 #include <wtf/glib/GUniquePtr.h>
 
 #if USE(GBM)
+#include <WebCore/DMABufFormat.h>
+#include <epoxy/egl.h>
 #include <gbm.h>
 #endif
 
@@ -53,6 +53,7 @@ namespace WebKit {
 OptionSet<DMABufRendererBufferMode> AcceleratedBackingStoreDMABuf::rendererBufferMode()
 {
     static OptionSet<DMABufRendererBufferMode> mode;
+#if USE(GBM)
     static std::once_flag onceFlag;
     std::call_once(onceFlag, [] {
         const char* disableDMABuf = getenv("WEBKIT_DISABLE_DMABUF_RENDERER");
@@ -71,6 +72,7 @@ OptionSet<DMABufRendererBufferMode> AcceleratedBackingStoreDMABuf::rendererBuffe
         if (eglExtensions.KHR_image_base && eglExtensions.EXT_image_dma_buf_import)
             mode.add(DMABufRendererBufferMode::Hardware);
     });
+#endif
     return mode;
 }
 bool AcceleratedBackingStoreDMABuf::checkRequirements()
@@ -104,6 +106,7 @@ AcceleratedBackingStoreDMABuf::RenderSource::RenderSource(const WebCore::IntSize
 {
 }
 
+#if USE(GBM)
 AcceleratedBackingStoreDMABuf::Texture::Texture(GdkGLContext* glContext, const UnixFileDescriptor& backFD, const UnixFileDescriptor& frontFD, const UnixFileDescriptor& displayFD, const WebCore::IntSize& size, uint32_t format, uint32_t offset, uint32_t stride, uint64_t modifier, float deviceScaleFactor)
     : RenderSource(size, deviceScaleFactor)
     , m_context(glContext)
@@ -226,7 +229,6 @@ void AcceleratedBackingStoreDMABuf::Texture::paint(GtkWidget* widget, cairo_t* c
 }
 #endif
 
-#if USE(GBM)
 AcceleratedBackingStoreDMABuf::Surface::Surface(const UnixFileDescriptor& backFD, const UnixFileDescriptor& frontFD, const UnixFileDescriptor& displayFD, const WebCore::IntSize& size, uint32_t format, uint32_t offset, uint32_t stride, float deviceScaleFactor)
     : RenderSource(size, deviceScaleFactor)
 {
@@ -419,10 +421,12 @@ std::unique_ptr<AcceleratedBackingStoreDMABuf::RenderSource> AcceleratedBackingS
 #if USE(GBM)
     if (!WebCore::PlatformDisplay::sharedDisplay().gtkEGLDisplay())
         return makeUnique<Surface>(m_surface.backFD, m_surface.frontFD, m_surface.displayFD, m_surface.size, m_surface.format, m_surface.offset, m_surface.stride, m_webPage.deviceScaleFactor());
-#endif
 
     ensureGLContext();
     return makeUnique<Texture>(m_gdkGLContext.get(), m_surface.backFD, m_surface.frontFD, m_surface.displayFD, m_surface.size, m_surface.format, m_surface.offset, m_surface.stride, m_surface.modifier, m_webPage.deviceScaleFactor());
+#else
+    return nullptr;
+#endif
 }
 
 void AcceleratedBackingStoreDMABuf::frame()
@@ -510,8 +514,10 @@ void AcceleratedBackingStoreDMABuf::ensureGLContext()
 
 bool AcceleratedBackingStoreDMABuf::makeContextCurrent()
 {
+#if USE(GBM)
     if (!WebCore::PlatformDisplay::sharedDisplay().gtkEGLDisplay())
         return false;
+#endif
 
     if (!gtk_widget_get_realized(m_webPage.viewWidget()))
         return false;
diff --git a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.h b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.h
index 1bc769ada313..0f9dc5496947 100644
--- a/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.h
+++ b/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.h
@@ -109,6 +109,7 @@ private:
         float m_deviceScaleFactor { 1 };
     };
 
+#if USE(GBM)
     class Texture final : public RenderSource {
     public:
         Texture(GdkGLContext*, const WTF::UnixFileDescriptor&, const WTF::UnixFileDescriptor&, const WTF::UnixFileDescriptor&, const WebCore::IntSize&, uint32_t format, uint32_t offset, uint32_t stride, uint64_t modifier, float deviceScaleFactor);
@@ -136,6 +137,7 @@ private:
         uint16_t m_textureIndex : 1;
 #endif
     };
+#endif
 
     class Surface final : public RenderSource {
     public:

@carlosgcampos
Copy link
Contributor Author

It was fixed by https://commits.webkit.org/268112@main

@ThomasDevoogdt
Copy link
Contributor

ThomasDevoogdt commented Sep 27, 2023

It was fixed by https://commits.webkit.org/268112@main

Thx, I saw it, and cherry-picked it.
But now I still get this:

webkitgtk-2.42.0/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreDMABuf.cpp:39:10: fatal error: epoxy/egl.h: No such file or directory
   39 | #include <epoxy/egl.h>
      |          ^~~~~~~~~~~~~

Simply moving the egl import under USE_GBM is not enough, there is more that has to be moved.

(https://bugs.webkit.org/show_bug.cgi?id=262169)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebKitGTK Bugs related to the Gtk API layer.
Projects
None yet
6 participants