vcl/inc/skia/utils.hxx | 2 -
vcl/inc/skia/x11/gdiimpl.hxx | 6 ++--
vcl/skia/SkiaHelper.cxx | 37 +++++++++++++++++++++++----
vcl/skia/win/gdiimpl.cxx | 2 -
vcl/skia/x11/gdiimpl.cxx | 49 +++++++++++++++++++++++++++++--------
vcl/source/opengl/OpenGLHelper.cxx | 5 +++
vcl/unx/generic/app/saldisp.cxx | 4 ++-
7 files changed, 83 insertions(+), 22 deletions(-)
New commits:
commit b275cce0878ddb7a56361c8d626c76ad57ad7d72
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Wed Mar 25 12:27:50 2020 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Wed Mar 25 19:15:26 2020 +0100
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.lu...@collabora.com>
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 )
{
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
