make sure only VCL-Skia or VCL-OpenGL is enabled (tdf#131543)

The bug is most likely caused by some code checking only the OpenGL
setting and doing something, even though the Skia setting takes
precedence. So make sure VCL OpenGL is considered disabled if Skia
is enabled.
Since isVCLOpenGLEnabled() is called right before setting up
the X11 visual, which is normally needed by isVCLSkiaEnabled()
to get Vulkan info to check blacklisting, this also requires using
a temporary Skia Vulkan context using the default X11 visual
to avoid the dependency loop.

Change-Id: I2d9d9e81ab4ed5021b5f42e3c272dcd10fd32cce
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/91044
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lunak@collabora.com>

7 files changed, 83 insertions, 22 deletions

diff --git a/vcl/inc/skia/utils.hxx b/vcl/inc/skia/utils.hxx
index 0a93b3b16863..c1f35e812d00 100644
--- a/vcl/inc/skia/utils.hxx
+++ b/vcl/inc/skia/utils.hxx
@@ -46,7 +46,7 @@ inline sk_sp<SkSurface> createSkSurface(const Size& size, SkColorType type = kN3
 // Must be called in any VCL backend before any Skia functionality is used.
 // If not set, Skia will be disabled.
 VCL_DLLPUBLIC void
-    prepareSkia(std::unique_ptr<sk_app::WindowContext> (*createVulkanWindowContext)());
+    prepareSkia(std::unique_ptr<sk_app::WindowContext> (*createVulkanWindowContext)(bool));
 
 #ifdef DBG_UTIL
 void prefillSurface(sk_sp<SkSurface>& surface);
diff --git a/vcl/inc/skia/x11/gdiimpl.hxx b/vcl/inc/skia/x11/gdiimpl.hxx
index acfe9f7ce192..0f86f0907dcd 100644
--- a/vcl/inc/skia/x11/gdiimpl.hxx
+++ b/vcl/inc/skia/x11/gdiimpl.hxx
@@ -38,9 +38,9 @@ private:
     virtual void performFlush() override;
     virtual bool avoidRecreateByResize() const override;
     static std::unique_ptr<sk_app::WindowContext>
-    createWindowContext(Display* display, Drawable drawable, const SalVisual* visual, int width,
-                        int height, SkiaHelper::RenderMethod renderMethod);
-    friend std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext();
+    createWindowContext(Display* display, Drawable drawable, const XVisualInfo* visual, int width,
+                        int height, SkiaHelper::RenderMethod renderMethod, bool temporary);
+    friend std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext(bool);
 };
 
 #endif // INCLUDED_VCL_INC_SKIA_X11_GDIIMPL_HXX
diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx
index 4b21e6740603..ff2a2a588741 100644
--- a/vcl/skia/SkiaHelper.cxx
+++ b/vcl/skia/SkiaHelper.cxx
@@ -69,6 +69,8 @@ static bool isVulkanBlacklisted(const VkPhysicalDeviceProperties& props)
                                             deviceIdStr);
 }
 
+static sk_app::VulkanWindowContext::SharedGrContext getTemporaryGrContext();
+
 static void checkDeviceBlacklisted(bool blockDisable = false)
 {
     static bool done = false;
@@ -80,9 +82,23 @@ static void checkDeviceBlacklisted(bool blockDisable = false)
         {
             case RenderVulkan:
             {
-                GrContext* grContext = SkiaHelper::getSharedGrContext();
+                // First try if a GrContext already exists.
+                sk_app::VulkanWindowContext::SharedGrContext grContext
+                    = sk_app::VulkanWindowContext::getSharedGrContext();
+                if (!grContext.getGrContext())
+                {
+                    // This function is called from isVclSkiaEnabled(), which
+                    // may be called when deciding which X11 visual to use,
+                    // and that visual is normally needed when creating
+                    // Skia's VulkanWindowContext, which is needed for the GrContext.
+                    // Avoid the loop by creating a temporary GrContext
+                    // that will use the default X11 visual (that shouldn't matter
+                    // for just finding out information about Vulkan) and destroying
+                    // the temporary context will clean up again.
+                    grContext = getTemporaryGrContext();
+                }
                 bool blacklisted = true; // assume the worst
-                if (grContext) // Vulkan was initialized properly
+                if (grContext.getGrContext()) // Vulkan was initialized properly
                 {
                     blacklisted = isVulkanBlacklisted(
                         sk_app::VulkanWindowContext::getPhysDeviceProperties());
@@ -230,8 +246,8 @@ void disableRenderMethod(RenderMethod method)
 
 static sk_app::VulkanWindowContext::SharedGrContext* sharedGrContext;
 
-static std::unique_ptr<sk_app::WindowContext> (*createVulkanWindowContextFunction)() = nullptr;
-static void setCreateVulkanWindowContext(std::unique_ptr<sk_app::WindowContext> (*function)())
+static std::unique_ptr<sk_app::WindowContext> (*createVulkanWindowContextFunction)(bool) = nullptr;
+static void setCreateVulkanWindowContext(std::unique_ptr<sk_app::WindowContext> (*function)(bool))
 {
     createVulkanWindowContextFunction = function;
 }
@@ -259,7 +275,7 @@ GrContext* getSharedGrContext()
     done = true;
     if (createVulkanWindowContextFunction == nullptr)
         return nullptr; // not initialized properly (e.g. used from a VCL backend with no Skia support)
-    std::unique_ptr<sk_app::WindowContext> tmpContext = createVulkanWindowContextFunction();
+    std::unique_ptr<sk_app::WindowContext> tmpContext = createVulkanWindowContextFunction(false);
     // Set up using the shared context created by the call above, if successful.
     context = sk_app::VulkanWindowContext::getSharedGrContext();
     grContext = context.getGrContext();
@@ -272,6 +288,15 @@ GrContext* getSharedGrContext()
     return nullptr;
 }
 
+static sk_app::VulkanWindowContext::SharedGrContext getTemporaryGrContext()
+{
+    if (createVulkanWindowContextFunction == nullptr)
+        return sk_app::VulkanWindowContext::SharedGrContext();
+    std::unique_ptr<sk_app::WindowContext> tmpContext = createVulkanWindowContextFunction(true);
+    // Set up using the shared context created by the call above, if successful.
+    return sk_app::VulkanWindowContext::getSharedGrContext();
+}
+
 sk_sp<SkSurface> createSkSurface(int width, int height, SkColorType type)
 {
     SkiaZone zone;
@@ -315,7 +340,7 @@ void cleanup()
 // Skia should not be used from VCL backends that do not actually support it, as there will be setup missing.
 // The code here (that is in the vcl lib) needs a function for creating Vulkan context that is
 // usually available only in the backend libs.
-void prepareSkia(std::unique_ptr<sk_app::WindowContext> (*createVulkanWindowContext)())
+void prepareSkia(std::unique_ptr<sk_app::WindowContext> (*createVulkanWindowContext)(bool))
 {
     setCreateVulkanWindowContext(createVulkanWindowContext);
     skiaSupportedByBackend = true;
diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx
index 0708c8ea633f..221819ca1c4f 100644
--- a/vcl/skia/win/gdiimpl.cxx
+++ b/vcl/skia/win/gdiimpl.cxx
@@ -349,7 +349,7 @@ SkiaControlCacheType& SkiaControlsCache::get()
 
 namespace
 {
-std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext()
+std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext(bool /*temporary*/)
 {
     SkiaZone zone;
     sk_app::DisplayParams displayParams;
diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx
index fe4553814758..0ed4b6a0ccbd 100644
--- a/vcl/skia/x11/gdiimpl.cxx
+++ b/vcl/skia/x11/gdiimpl.cxx
@@ -23,6 +23,8 @@
 #include <skia/utils.hxx>
 #include <skia/zone.hxx>
 
+#include <X11/Xutil.h>
+
 X11SkiaSalGraphicsImpl::X11SkiaSalGraphicsImpl(X11SalGraphics& rParent)
     : SkiaSalGraphicsImpl(rParent, rParent.GetGeometryProvider())
     , mX11Parent(rParent)
@@ -43,7 +45,7 @@ void X11SkiaSalGraphicsImpl::createWindowContext()
     assert(mX11Parent.GetDrawable() != None);
     mWindowContext = createWindowContext(mX11Parent.GetXDisplay(), mX11Parent.GetDrawable(),
                                          &mX11Parent.GetVisual(), GetWidth(), GetHeight(),
-                                         SkiaHelper::renderMethodToUse());
+                                         SkiaHelper::renderMethodToUse(), false);
     if (mWindowContext && SkiaHelper::renderMethodToUse() == SkiaHelper::RenderVulkan)
         mIsGPU = true;
     else
@@ -52,8 +54,8 @@ void X11SkiaSalGraphicsImpl::createWindowContext()
 
 std::unique_ptr<sk_app::WindowContext>
 X11SkiaSalGraphicsImpl::createWindowContext(Display* display, Drawable drawable,
-                                            const SalVisual* visual, int width, int height,
-                                            SkiaHelper::RenderMethod renderMethod)
+                                            const XVisualInfo* visual, int width, int height,
+                                            SkiaHelper::RenderMethod renderMethod, bool temporary)
 {
     SkiaZone zone;
     sk_app::DisplayParams displayParams;
@@ -63,15 +65,24 @@ X11SkiaSalGraphicsImpl::createWindowContext(Display* display, Drawable drawable,
     winInfo.fDisplay = display;
     winInfo.fWindow = drawable;
     winInfo.fFBConfig = nullptr; // not used
-    winInfo.fVisualInfo = const_cast<SalVisual*>(visual);
+    winInfo.fVisualInfo = const_cast<XVisualInfo*>(visual);
+    assert(winInfo.fVisualInfo->visual != nullptr); // make sure it's not an uninitialized SalVisual
     winInfo.fWidth = width;
     winInfo.fHeight = height;
 #ifdef DBG_UTIL
     // Our patched Skia has VulkanWindowContext that shares GrContext, which requires
     // that the X11 visual is always the same. Ensure it is so.
     static VisualID checkVisualID = -1U;
-    assert(checkVisualID == -1U || winInfo.fVisualInfo->visualid == checkVisualID);
-    checkVisualID = winInfo.fVisualInfo->visualid;
+    // Exception is for the temporary case during startup, when SkiaHelper's
+    // checkDeviceBlacklisted() needs a WindowContext and may be called before SalVisual
+    // is ready.
+    if (!temporary)
+    {
+        assert(checkVisualID == -1U || winInfo.fVisualInfo->visualid == checkVisualID);
+        checkVisualID = winInfo.fVisualInfo->visualid;
+    }
+#else
+    (void)temporary;
 #endif
     switch (renderMethod)
     {
@@ -122,12 +133,30 @@ void X11SkiaSalGraphicsImpl::performFlush()
     mWindowContext->swapBuffers();
 }
 
-std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext()
+std::unique_ptr<sk_app::WindowContext> createVulkanWindowContext(bool temporary)
 {
     SalDisplay* salDisplay = vcl_sal::getSalDisplay(GetGenericUnixSalData());
-    return X11SkiaSalGraphicsImpl::createWindowContext(
-        salDisplay->GetDisplay(), None, &salDisplay->GetVisual(salDisplay->GetDefaultXScreen()), 1,
-        1, SkiaHelper::RenderVulkan);
+    const XVisualInfo* visual;
+    XVisualInfo* visuals = nullptr;
+    if (!temporary)
+        visual = &salDisplay->GetVisual(salDisplay->GetDefaultXScreen());
+    else
+    {
+        // SalVisual from salDisplay may not be setup yet at this point, get
+        // info for the default visual.
+        XVisualInfo search;
+        search.visualid = XVisualIDFromVisual(
+            DefaultVisual(salDisplay->GetDisplay(), salDisplay->GetDefaultXScreen().getXScreen()));
+        int count;
+        visuals = XGetVisualInfo(salDisplay->GetDisplay(), VisualIDMask, &search, &count);
+        assert(count == 1);
+        visual = visuals;
+    }
+    std::unique_ptr<sk_app::WindowContext> ret = X11SkiaSalGraphicsImpl::createWindowContext(
+        salDisplay->GetDisplay(), None, visual, 1, 1, SkiaHelper::RenderVulkan, temporary);
+    if (temporary)
+        XFree(visuals);
+    return ret;
 }
 
 void X11SkiaSalGraphicsImpl::prepareSkia() { SkiaHelper::prepareSkia(createVulkanWindowContext); }
diff --git a/vcl/source/opengl/OpenGLHelper.cxx b/vcl/source/opengl/OpenGLHelper.cxx
index 93f606930b99..60f023b7dc4c 100644
--- a/vcl/source/opengl/OpenGLHelper.cxx
+++ b/vcl/source/opengl/OpenGLHelper.cxx
@@ -34,6 +34,7 @@
 #include <desktop/crashreport.hxx>
 #include <bitmapwriteaccess.hxx>
 #include <watchdog.hxx>
+#include <vcl/skia/SkiaHelper.hxx>
 
 #if defined UNX && !defined MACOSX && !defined IOS && !defined ANDROID && !defined HAIKU
 #include <opengl/x11/X11DeviceInfo.hxx>
@@ -904,6 +905,10 @@ PreDefaultWinNoOpenGLZone::~PreDefaultWinNoOpenGLZone()
 
 bool OpenGLHelper::isVCLOpenGLEnabled()
 {
+    // Skia always takes precedence if enabled
+    if( SkiaHelper::isVCLSkiaEnabled())
+        return false;
+
     /**
      * The !bSet part should only be called once! Changing the results in the same
      * run will mix OpenGL and normal rendering.
diff --git a/vcl/unx/generic/app/saldisp.cxx b/vcl/unx/generic/app/saldisp.cxx
index 99d0652df7ff..b26fe54323bd 100644
--- a/vcl/unx/generic/app/saldisp.cxx
+++ b/vcl/unx/generic/app/saldisp.cxx
@@ -2390,7 +2390,9 @@ bool SalDisplay::XIfEventWithTimeout( XEvent* o_pEvent, XPointer i_pPredicateDat
 SalVisual::SalVisual():
     eRGBMode_(SalRGB::RGB), nRedShift_(0), nGreenShift_(0), nBlueShift_(0), nRedBits_(0), nGreenBits_(0),
     nBlueBits_(0)
-{}
+{
+    visual = nullptr;
+}
 
 SalVisual::SalVisual( const XVisualInfo* pXVI )
 {