Bug 236290 - [GTK][WPE] Fallback path for WebGL rendering with ANGLE is incorrectly scheduling on the compositor thread
Summary: [GTK][WPE] Fallback path for WebGL rendering with ANGLE is incorrectly schedu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Lord
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-08 02:02 PST by Chris Lord
Modified: 2022-02-08 12:47 PST (History)
13 users (show)

See Also:


Attachments
Patch (33.52 KB, patch)
2022-02-08 02:07 PST, Chris Lord
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Lord 2022-02-08 02:02:33 PST
The fallback path for WebGL ANGLE rendering on Linux works, but was based on some bad assumptions. These have been fixed for the main path in bug 235946, but the fallback path remains. Filing this bug for the fix-up.
Comment 1 Chris Lord 2022-02-08 02:07:42 PST
Created attachment 451224 [details]
Patch
Comment 2 Chris Lord 2022-02-08 02:13:50 PST
The diff viewer has not been clever it appears, so for ease of review, this is the actual diff for the renamed files:

diff --git a/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.cpp b/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.cpp
similarity index 75%
rename from Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.cpp
rename to Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.cpp
index 51f57e116f6c..d06d826bf839 100644
--- a/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.cpp
+++ b/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.cpp
@@ -27,7 +27,7 @@
  */
 
 #include "config.h"
-#include "NicosiaGCGLANGLEPipe.h"
+#include "NicosiaGCGLANGLELayer.h"
 
 #if USE(NICOSIA) && USE(TEXTURE_MAPPER)
 
@@ -42,29 +42,21 @@ namespace Nicosia {
 
 using namespace WebCore;
 
-GCGLANGLEPipeSource::GCGLANGLEPipeSource(WebCore::GraphicsContextGLANGLE& context)
-    : m_context(context)
-{
-}
-
-GCGLANGLEPipeSource::~GCGLANGLEPipeSource()
-{
-}
-
-void GCGLANGLEPipeSource::swapBuffersIfNeeded()
+void GCGLANGLELayer::swapBuffersIfNeeded()
 {
     if (m_context.layerComposited())
         return;
 
     m_context.prepareTexture();
 
+    auto& proxy = downcast<Nicosia::ContentLayerTextureMapperImpl>(contentLayer().impl()).proxy();
+    auto size = m_context.getInternalFramebufferSize();
+
     if (m_context.m_compositorTextureBacking) {
-        auto size = m_context.getInternalFramebufferSize();
         auto format = m_context.m_compositorTextureBacking->format();
         auto stride = m_context.m_compositorTextureBacking->stride();
         auto fd = m_context.m_compositorTextureBacking->fd();
 
-        auto& proxy = downcast<Nicosia::ContentLayerTextureMapperImpl>(platformLayer()->impl()).proxy();
         {
             Locker locker { proxy.lock() };
             proxy.pushNextBuffer(makeUnique<TextureMapperPlatformLayerDmabuf>(size, format, stride, fd));
@@ -75,18 +67,27 @@ void GCGLANGLEPipeSource::swapBuffersIfNeeded()
     }
 
     // Fallback path, read back texture to main memory
-    RefPtr<WebCore::ImageBuffer> imageBuffer = ImageBuffer::create(m_context.getInternalFramebufferSize(), RenderingMode::Unaccelerated, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
+    RefPtr<WebCore::ImageBuffer> imageBuffer = ImageBuffer::create(size, RenderingMode::Unaccelerated, 1, DestinationColorSpace::SRGB(), PixelFormat::BGRA8);
     if (!imageBuffer)
         return;
-
     m_context.paintRenderingResultsToCanvas(*imageBuffer.get());
 
-    handle(WTFMove(imageBuffer));
+    auto flags = m_context.contextAttributes().alpha ? BitmapTexture::SupportsAlpha : BitmapTexture::NoFlag;
+    {
+        Locker locker { proxy.lock() };
+        std::unique_ptr<TextureMapperPlatformLayerBuffer> layerBuffer = proxy.getAvailableBuffer(size, m_context.m_internalColorFormat);
+        if (!layerBuffer) {
+            auto texture = BitmapTextureGL::create(TextureMapperContextAttributes::get(), flags, m_context.m_internalColorFormat);
+            layerBuffer = makeUnique<TextureMapperPlatformLayerBuffer>(WTFMove(texture), flags);
+        }
 
+        layerBuffer->textureGL().setPendingContents(ImageBuffer::sinkIntoImage(WTFMove(imageBuffer)));
+        proxy.pushNextBuffer(WTFMove(layerBuffer));
+    }
     m_context.markLayerComposited();
 }
 
-const char* GCGLANGLEPipe::ANGLEContext::errorString(int statusCode)
+const char* GCGLANGLELayer::ANGLEContext::errorString(int statusCode)
 {
     static_assert(sizeof(int) >= sizeof(EGLint), "EGLint must not be wider than int");
     switch (statusCode) {
@@ -112,12 +113,12 @@ const char* GCGLANGLEPipe::ANGLEContext::errorString(int statusCode)
     }
 }
 
-const char* GCGLANGLEPipe::ANGLEContext::lastErrorString()
+const char* GCGLANGLELayer::ANGLEContext::lastErrorString()
 {
     return errorString(EGL_GetError());
 }
 
-std::unique_ptr<GCGLANGLEPipe::ANGLEContext> GCGLANGLEPipe::ANGLEContext::createContext()
+std::unique_ptr<GCGLANGLELayer::ANGLEContext> GCGLANGLELayer::ANGLEContext::createContext()
 {
     EGLDisplay display = EGL_GetDisplay(EGL_DEFAULT_DISPLAY);
     if (display == EGL_NO_DISPLAY)
@@ -181,7 +182,7 @@ std::unique_ptr<GCGLANGLEPipe::ANGLEContext> GCGLANGLEPipe::ANGLEContext::create
     return std::unique_ptr<ANGLEContext>(new ANGLEContext(display, config, context, EGL_NO_SURFACE));
 }
 
-GCGLANGLEPipe::ANGLEContext::ANGLEContext(EGLDisplay display, EGLConfig config, EGLContext context, EGLSurface surface)
+GCGLANGLELayer::ANGLEContext::ANGLEContext(EGLDisplay display, EGLConfig config, EGLContext context, EGLSurface surface)
     : m_display(display)
     , m_config(config)
     , m_context(context)
@@ -189,7 +190,7 @@ GCGLANGLEPipe::ANGLEContext::ANGLEContext(EGLDisplay display, EGLConfig config,
 {
 }
 
-GCGLANGLEPipe::ANGLEContext::~ANGLEContext()
+GCGLANGLELayer::ANGLEContext::~ANGLEContext()
 {
     if (m_context) {
         GL_BindFramebuffer(GL_FRAMEBUFFER, 0);
@@ -201,7 +202,7 @@ GCGLANGLEPipe::ANGLEContext::~ANGLEContext()
         EGL_DestroySurface(m_display, m_surface);
 }
 
-bool GCGLANGLEPipe::ANGLEContext::makeContextCurrent()
+bool GCGLANGLELayer::ANGLEContext::makeContextCurrent()
 {
     ASSERT(m_context);
 
@@ -210,67 +211,58 @@ bool GCGLANGLEPipe::ANGLEContext::makeContextCurrent()
     return true;
 }
 
-PlatformGraphicsContextGL GCGLANGLEPipe::ANGLEContext::platformContext() const
+PlatformGraphicsContextGL GCGLANGLELayer::ANGLEContext::platformContext() const
 {
     return m_context;
 }
 
-PlatformGraphicsContextGLDisplay GCGLANGLEPipe::ANGLEContext::platformDisplay() const
+PlatformGraphicsContextGLDisplay GCGLANGLELayer::ANGLEContext::platformDisplay() const
 {
     return m_display;
 }
 
-PlatformGraphicsContextGLConfig GCGLANGLEPipe::ANGLEContext::platformConfig() const
+PlatformGraphicsContextGLConfig GCGLANGLELayer::ANGLEContext::platformConfig() const
 {
     return m_config;
 }
 
-GCGLANGLEPipe::GCGLANGLEPipe(GraphicsContextGLANGLE& context)
-    : m_angleContext(ANGLEContext::createContext())
-    , m_source(adoptRef(*new GCGLANGLEPipeSource(context)))
-    , m_layerContentsDisplayDelegate(NicosiaImageBufferPipeSourceDisplayDelegate::create(m_source->platformLayer()))
+GCGLANGLELayer::GCGLANGLELayer(GraphicsContextGLANGLE& context)
+    : m_context(context)
+    , m_angleContext(ANGLEContext::createContext())
+    , m_contentLayer(Nicosia::ContentLayer::create(Nicosia::ContentLayerTextureMapperImpl::createFactory(*this)))
 {
 }
 
-GCGLANGLEPipe::~GCGLANGLEPipe()
+GCGLANGLELayer::~GCGLANGLELayer()
 {
+    downcast<ContentLayerTextureMapperImpl>(m_contentLayer->impl()).invalidateClient();
 }
 
-bool GCGLANGLEPipe::makeContextCurrent()
+bool GCGLANGLELayer::makeContextCurrent()
 {
     ASSERT(m_angleContext);
     return m_angleContext->makeContextCurrent();
 
 }
 
-PlatformGraphicsContextGL GCGLANGLEPipe::platformContext() const
+PlatformGraphicsContextGL GCGLANGLELayer::platformContext() const
 {
     ASSERT(m_angleContext);
     return m_angleContext->platformContext();
 }
 
-PlatformGraphicsContextGLDisplay GCGLANGLEPipe::platformDisplay() const
+PlatformGraphicsContextGLDisplay GCGLANGLELayer::platformDisplay() const
 {
     ASSERT(m_angleContext);
     return m_angleContext->platformDisplay();
 }
 
-PlatformGraphicsContextGLConfig GCGLANGLEPipe::platformConfig() const
+PlatformGraphicsContextGLConfig GCGLANGLELayer::platformConfig() const
 {
     ASSERT(m_angleContext);
     return m_angleContext->platformContext();
 }
 
-RefPtr<ImageBufferPipe::Source> GCGLANGLEPipe::source() const
-{
-    return m_source.ptr();
-}
-
-RefPtr<GraphicsLayerContentsDisplayDelegate> GCGLANGLEPipe::layerContentsDisplayDelegate()
-{
-    return m_layerContentsDisplayDelegate.ptr();
-}
-
 } // namespace Nicosia
 
 #endif // USE(NICOSIA) && USE(TEXTURE_MAPPER)
diff --git a/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.h b/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.h
similarity index 80%
rename from Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.h
rename to Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.h
index 1f353360840c..4f8df6ece06a 100644
--- a/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLEPipe.h
+++ b/Source/WebCore/platform/graphics/nicosia/texmap/NicosiaGCGLANGLELayer.h
@@ -32,7 +32,7 @@
 
 #include "GLContext.h"
 #include "GraphicsContextGLANGLE.h"
-#include "NicosiaImageBufferPipe.h"
+#include "NicosiaContentLayerTextureMapperImpl.h"
 #include <memory>
 
 typedef void *EGLConfig;
@@ -48,18 +48,7 @@ class PlatformDisplay;
 
 namespace Nicosia {
 
-class GCGLANGLEPipeSource : public NicosiaImageBufferPipeSource {
-public:
-    GCGLANGLEPipeSource(WebCore::GraphicsContextGLANGLE&);
-    virtual ~GCGLANGLEPipeSource();
-
-    // ContentLayerTextureMapperImpl::Client overrides.
-    void swapBuffersIfNeeded() final;
-private:
-    WebCore::GraphicsContextGLANGLE& m_context;
-};
-
-class GCGLANGLEPipe final : public WebCore::ImageBufferPipe {
+class GCGLANGLELayer final : public ContentLayerTextureMapperImpl::Client {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     class ANGLEContext {
@@ -87,22 +76,21 @@ public:
         EGLSurface m_surface { nullptr };
     };
 
-    GCGLANGLEPipe(WebCore::GraphicsContextGLANGLE&);
-    virtual ~GCGLANGLEPipe();
+    GCGLANGLELayer(WebCore::GraphicsContextGLANGLE&);
+    virtual ~GCGLANGLELayer();
 
     bool makeContextCurrent();
     PlatformGraphicsContextGL platformContext() const;
     PlatformGraphicsContextGLDisplay platformDisplay() const;
     PlatformGraphicsContextGLConfig platformConfig() const;
 
-    // ImageBufferPipe overrides.
-    RefPtr<WebCore::ImageBufferPipe::Source> source() const final;
-    RefPtr<WebCore::GraphicsLayerContentsDisplayDelegate> layerContentsDisplayDelegate() final;
+    ContentLayer& contentLayer() const { return m_contentLayer; }
+    void swapBuffersIfNeeded() final;
 
 private:
+    WebCore::GraphicsContextGLANGLE& m_context;
     std::unique_ptr<ANGLEContext> m_angleContext;
-    Ref<GCGLANGLEPipeSource> m_source;
-    Ref<NicosiaImageBufferPipeSourceDisplayDelegate> m_layerContentsDisplayDelegate;
+    Ref<ContentLayer> m_contentLayer;
 };
 
 } // namespace Nicosia
Comment 3 Alejandro G. Castro 2022-02-08 11:33:22 PST
Comment on attachment 451224 [details]
Patch

LGTM! Great!
Comment 4 EWS 2022-02-08 12:46:54 PST
Committed r289418 (246981@main): <https://commits.webkit.org/246981@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451224 [details].
Comment 5 Radar WebKit Bug Importer 2022-02-08 12:47:21 PST
<rdar://problem/88646986>