Rebased ref, commits from common ancestor: commit 55483e16b5292c59904da17138ec9ee5de951462 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Mon Nov 18 12:24:59 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:48:03 2019 +0100
make about dialog differentiate between Skia with Vulkan or raster Since they are technically still two different rendering implementations. Change-Id: I83c324b384b7acfcc84e729271d00b995327eec6 diff --git a/vcl/inc/strings.hrc b/vcl/inc/strings.hrc index 65e829648f23..6c86cec3e747 100644 --- a/vcl/inc/strings.hrc +++ b/vcl/inc/strings.hrc @@ -126,7 +126,8 @@ #define SV_APP_OSVERSION NC_("SV_APP_OSVERSION", "OS: ") #define SV_APP_UIRENDER NC_("SV_APP_UIRENDER", "UI render: ") #define SV_APP_GL NC_("SV_APP_GL", "GL") -#define SV_APP_SKIA NC_("SV_APP_SKIA", "Skia") +#define SV_APP_SKIA_VULKAN NC_("SV_APP_SKIA_VULKAN", "Skia/Vulkan") +#define SV_APP_SKIA_RASTER NC_("SV_APP_SKIA_RASTER", "Skia/Raster") #define SV_APP_DEFAULT NC_("SV_APP_DEFAULT", "default") #define SV_MSGBOX_INFO NC_("SV_MSGBOX_INFO", "Information") diff --git a/vcl/source/app/svapp.cxx b/vcl/source/app/svapp.cxx index 3de5548a274e..d3b78a989610 100644 --- a/vcl/source/app/svapp.cxx +++ b/vcl/source/app/svapp.cxx @@ -1155,7 +1155,17 @@ OUString Application::GetHWOSConfInfo() else #endif if ( SkiaHelper::isVCLSkiaEnabled() ) - aDetails.append( VclResId(SV_APP_SKIA) ); + { + switch(SkiaHelper::renderMethodToUse()) + { + case SkiaHelper::RenderVulkan: + aDetails.append( VclResId(SV_APP_SKIA_VULKAN) ); + break; + case SkiaHelper::RenderRaster: + aDetails.append( VclResId(SV_APP_SKIA_RASTER) ); + break; + } + } else aDetails.append( VclResId(SV_APP_DEFAULT) ); aDetails.append( "; " ); commit 73a064203b4ffea9d3665890acfd81fabfd4f433 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Mon Nov 18 12:10:36 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:48:02 2019 +0100 make sure Skia invert with TrackFrame doesn't paint outside According to Tomaž that's a requirement and that is what the test for it tests. This is easy to implement with additional clipping. Change-Id: Ia54489e20ce58ae0624183f2989036e6938cd44f diff --git a/vcl/qa/cppunit/BackendTest.cxx b/vcl/qa/cppunit/BackendTest.cxx index 6dfda5d02d35..1090c33b73e0 100644 --- a/vcl/qa/cppunit/BackendTest.cxx +++ b/vcl/qa/cppunit/BackendTest.cxx @@ -465,8 +465,7 @@ public: CPPUNIT_TEST(testDrawInvertWithRectangle); CPPUNIT_TEST(testDrawInvertN50WithRectangle); - // AFAIK this test (itself) is broken. - // CPPUNIT_TEST(testDrawInvertTrackFrameWithRectangle); + CPPUNIT_TEST(testDrawInvertTrackFrameWithRectangle); CPPUNIT_TEST(testDrawBezierWithPolylineB2D); CPPUNIT_TEST(testDrawBezierAAWithPolylineB2D); diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index 948c85df72e4..271f1a61321c 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -920,6 +920,11 @@ void SkiaSalGraphicsImpl::invert(basegfx::B2DPolygon const& rPoly, SalInvert eFl SkPath aPath; addPolygonToPath(rPoly, aPath); aPath.setFillType(SkPath::kEvenOdd_FillType); + // TrackFrame is not supposed to paint outside of the polygon (usually rectangle), + // but wider stoke width usually results in that, so ensure the requirement + // by clipping. + SkAutoCanvasRestore autoRestore(mSurface->getCanvas(), true); + mSurface->getCanvas()->clipRect(aPath.getBounds(), SkClipOp::kIntersect, false); SkPaint aPaint; aPaint.setStrokeWidth(2); float intervals[] = { 4.0f, 4.0f }; commit 2d0a0a9685a45c3d6f924524c9d54ee8e2235cd4 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Fri Nov 15 17:22:38 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:48:02 2019 +0100 disable Skia resource leak checking on exit This is normally enabled in Skia debug builds and it asserts if there is a problem, which there is with a number of our unittests that leak something (usually a VirtualDevice). Those are non-trivial to find and don't matter in practice (or if they do they should be fixed for all VCL backends), so just disable the Skia check. Change-Id: I0a0721d8a3f0f961e14513574f4b3cc88ec1e62c diff --git a/external/skia/UnpackedTarball_skia.mk b/external/skia/UnpackedTarball_skia.mk index c3988042a012..1fab368cc34b 100644 --- a/external/skia/UnpackedTarball_skia.mk +++ b/external/skia/UnpackedTarball_skia.mk @@ -18,6 +18,7 @@ skia_patches := \ fix-ddi.patch \ make-api-visible.patch.1 \ fix-shader-locale.patch.1 \ + no-trace-resources-on-exit.patch.1 \ share-grcontext.patch.1 $(eval $(call gb_UnpackedTarball_set_patchlevel,skia,1)) diff --git a/external/skia/no-trace-resources-on-exit.patch.1 b/external/skia/no-trace-resources-on-exit.patch.1 new file mode 100644 index 000000000000..7a8567938eba --- /dev/null +++ b/external/skia/no-trace-resources-on-exit.patch.1 @@ -0,0 +1,26 @@ +diff --git a/src/gpu/vk/GrVkCommandPool.h b/src/gpu/vk/GrVkCommandPool.h +index fd44d62e94..f9e90f185f 100644 +--- a/src/gpu/vk/GrVkCommandPool.h ++++ b/src/gpu/vk/GrVkCommandPool.h +@@ -41,7 +41,7 @@ public: + // returns true if close() has not been called + bool isOpen() const { return fOpen; } + +-#ifdef SK_DEBUG ++#ifdef SK_TRACE_VK_RESOURCES + void dumpInfo() const override { + SkDebugf("GrVkCommandPool: %p (%d refs)\n", fCommandPool, this->getRefCnt()); + } +diff --git a/src/gpu/vk/GrVkResource.h b/src/gpu/vk/GrVkResource.h +index 7b9949ba1b..4e8fb48c7c 100644 +--- a/src/gpu/vk/GrVkResource.h ++++ b/src/gpu/vk/GrVkResource.h +@@ -17,7 +17,7 @@ class GrVkGpu; + + // uncomment to enable tracing of resource refs + #ifdef SK_DEBUG +-#define SK_TRACE_VK_RESOURCES ++//#define SK_TRACE_VK_RESOURCES + #endif + + /** \class GrVkResource commit 80d85a162b400a2786d2f4f0acb7a173feb101b7 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Fri Nov 15 17:20:44 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:48:01 2019 +0100 fix memory leak Change-Id: I6148159737edd4fe225d1140606064cdb77ef615 diff --git a/include/vcl/pdfwriter.hxx b/include/vcl/pdfwriter.hxx index fe24e50ad11f..879b39fe3114 100644 --- a/include/vcl/pdfwriter.hxx +++ b/include/vcl/pdfwriter.hxx @@ -74,7 +74,7 @@ class VCL_DLLPUBLIC PDFOutputStream class VCL_DLLPUBLIC PDFWriter { - VclPtr<PDFWriterImpl> xImplementation; + ScopedVclPtr<PDFWriterImpl> xImplementation; PDFWriter(const PDFWriter&) = delete; PDFWriter& operator=(const PDFWriter&) = delete; commit a3d45762cf118bbb9f689ec8842b9038fc0dc3c2 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Fri Nov 15 14:42:05 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:48:00 2019 +0100 make Skia GPU offscreen surfaces work with unittests Skia is now patched to be able to create also invalid sk_app::WindowContext that will just initialize the shared GrContext. And always use that GrContext, even for tests, because some tests first create a offscreen surfaces and only later create windows, which before this patch led to mixing GrContext instances. Change-Id: Ic79c0719f98f6ac48527c2ea2a9a9a69412adeff diff --git a/external/skia/Library_skia.mk b/external/skia/Library_skia.mk index 7d84762a621d..ed24d30fa93f 100644 --- a/external/skia/Library_skia.mk +++ b/external/skia/Library_skia.mk @@ -62,7 +62,6 @@ ifeq ($(OS),LINUX) $(eval $(call gb_Library_add_libs,skia,\ -lm \ -ldl \ - -lGLU \ -lGLX \ -lGL \ -lX11-xcb \ @@ -814,12 +813,6 @@ $(eval $(call gb_Library_add_generated_exception_objects,skia,\ )) $(eval $(call gb_Library_add_generated_exception_objects,skia,\ - UnpackedTarball/skia/tools/gpu/GrContextFactory \ - UnpackedTarball/skia/tools/gpu/TestContext \ - UnpackedTarball/skia/tools/gpu/gl/GLTestContext \ - UnpackedTarball/skia/tools/gpu/gl/command_buffer/GLTestContext_command_buffer \ - UnpackedTarball/skia/tools/gpu/mock/MockTestContext \ - UnpackedTarball/skia/tools/gpu/vk/VkTestContext \ UnpackedTarball/skia/tools/gpu/vk/VkTestUtils \ UnpackedTarball/skia/tools/sk_app/GLWindowContext \ UnpackedTarball/skia/tools/sk_app/VulkanWindowContext \ @@ -848,7 +841,6 @@ $(eval $(call gb_Library_add_generated_exception_objects,skia,\ )) $(eval $(call gb_Library_add_generated_exception_objects,skia,\ - UnpackedTarball/skia/tools/gpu/gl/win/CreatePlatformGLTestContext_win \ UnpackedTarball/skia/tools/sk_app/win/GLWindowContext_win \ UnpackedTarball/skia/tools/sk_app/win/RasterWindowContext_win \ UnpackedTarball/skia/tools/sk_app/win/VulkanWindowContext_win \ @@ -871,7 +863,6 @@ $(eval $(call gb_Library_add_generated_exception_objects,skia,\ )) $(eval $(call gb_Library_add_generated_exception_objects,skia,\ - UnpackedTarball/skia/tools/gpu/gl/glx/CreatePlatformGLTestContext_glx \ UnpackedTarball/skia/tools/sk_app/unix/GLWindowContext_unix \ UnpackedTarball/skia/tools/sk_app/unix/RasterWindowContext_unix \ UnpackedTarball/skia/tools/sk_app/unix/VulkanWindowContext_unix \ diff --git a/external/skia/make-api-visible.patch.1 b/external/skia/make-api-visible.patch.1 index 3c2ff873eabb..2bf3a0f3d73a 100644 --- a/external/skia/make-api-visible.patch.1 +++ b/external/skia/make-api-visible.patch.1 @@ -1,29 +1,3 @@ -diff --git a/tools/gpu/GrContextFactory.h b/tools/gpu/GrContextFactory.h -index d1b7fd5fa0..1b0bc249d2 100644 ---- a/tools/gpu/GrContextFactory.h -+++ b/tools/gpu/GrContextFactory.h -@@ -26,7 +26,7 @@ class ContextInfo; - * factory is destroyed (though the caller can always grab a ref on the returned - * Gr and GL contexts to make them outlive the factory). - */ --class GrContextFactory : SkNoncopyable { -+class SK_API GrContextFactory : SkNoncopyable { - public: - // The availability of context types is subject to platform and build configuration - // restrictions. -diff --git a/tools/gpu/gl/GLTestContext.cpp b/tools/gpu/gl/GLTestContext.cpp -index d4aa605188..5d246f9737 100644 ---- a/tools/gpu/gl/GLTestContext.cpp -+++ b/tools/gpu/gl/GLTestContext.cpp -@@ -298,7 +298,7 @@ void GLTestContext::teardown() { - void GLTestContext::testAbandon() { - INHERITED::testAbandon(); - if (fGL) { -- fGL->abandon(); -+// fGL->abandon(); - } - } - diff --git a/tools/sk_app/unix/WindowContextFactory_unix.h b/tools/sk_app/unix/WindowContextFactory_unix.h index 47310970d5..e02e6eb5b7 100644 --- a/tools/sk_app/unix/WindowContextFactory_unix.h diff --git a/external/skia/share-grcontext.patch.1 b/external/skia/share-grcontext.patch.1 index 357c3a885880..ed81e772aa20 100644 --- a/external/skia/share-grcontext.patch.1 +++ b/external/skia/share-grcontext.patch.1 @@ -1,13 +1,168 @@ -diff --git a/tools/sk_app/VulkanWindowContext.cpp b/tools/sk_app/VulkanWindowContext.cpp -index 793c88c158..21164cac67 100644 ---- a/tools/sk_app/VulkanWindowContext.cpp -+++ b/tools/sk_app/VulkanWindowContext.cpp -@@ -1,4 +1,3 @@ +--- ./tools/sk_app/VulkanWindowContext.h.sav 2019-11-14 16:46:31.218722399 +0100 ++++ ./tools/sk_app/VulkanWindowContext.h 2019-11-15 11:58:46.656455921 +0100 +@@ -23,14 +23,30 @@ class GrRenderTarget; + + namespace sk_app { + +-class VulkanWindowContext : public WindowContext { ++class SK_API VulkanWindowContext : public WindowContext { ++ struct Shared; + public: + ~VulkanWindowContext() override; + ++ class SharedGrContext { ++ public: ++ SharedGrContext() {} ++ GrContext* getGrContext() { return shared ? shared->fContext.get() : nullptr; } ++ ~SharedGrContext() { shared.reset(); checkDestroyShared(); } ++ bool operator!() const { return !shared; } ++ void reset() { shared.reset(); } ++ private: ++ friend class VulkanWindowContext; ++ SharedGrContext(sk_sp<Shared>& sh ) : shared( sh ) {} ++ sk_sp<Shared> shared; ++ }; ++ ++ static SharedGrContext getSharedGrContext() { return SharedGrContext( fGlobalShared ); } ++ + sk_sp<SkSurface> getBackbufferSurface() override; + void swapBuffers() override; + +- bool isValid() override { return fDevice != VK_NULL_HANDLE; } ++ bool isValid() override { return fSurface != VK_NULL_HANDLE; } + + void resize(int w, int h) override { + this->createSwapchain(w, h, fDisplayParams); +@@ -53,6 +69,7 @@ public: + private: + void initializeContext(); + void destroyContext(); ++ static void checkDestroyShared(); + + struct BackbufferInfo { + uint32_t fImageIndex; // image this is associated with +@@ -64,11 +81,6 @@ private: + void createBuffers(VkFormat format, SkColorType colorType); + void destroyBuffers(); + +- VkInstance fInstance = VK_NULL_HANDLE; +- VkPhysicalDevice fPhysicalDevice = VK_NULL_HANDLE; +- VkDevice fDevice = VK_NULL_HANDLE; +- VkDebugReportCallbackEXT fDebugCallback = VK_NULL_HANDLE; - - /* - * Copyright 2015 Google Inc. - * -@@ -24,8 +23,10 @@ + // Create functions + CreateVkSurfaceFn fCreateVkSurfaceFn; + CanPresentFn fCanPresentFn; +@@ -90,20 +102,41 @@ private: + PFN_vkAcquireNextImageKHR fAcquireNextImageKHR = nullptr; + PFN_vkQueuePresentKHR fQueuePresentKHR = nullptr; + +- PFN_vkDestroyInstance fDestroyInstance = nullptr; + PFN_vkDeviceWaitIdle fDeviceWaitIdle = nullptr; +- PFN_vkDestroyDebugReportCallbackEXT fDestroyDebugReportCallbackEXT = nullptr; + PFN_vkQueueWaitIdle fQueueWaitIdle = nullptr; +- PFN_vkDestroyDevice fDestroyDevice = nullptr; + PFN_vkGetDeviceQueue fGetDeviceQueue = nullptr; + ++ // We need to use just one GrContext, so share all the relevant data. ++ struct Shared : public SkRefCnt ++ { ++ PFN_vkDestroyInstance fDestroyInstance = nullptr; ++ PFN_vkDestroyDevice fDestroyDevice = nullptr; ++ PFN_vkDestroyDebugReportCallbackEXT fDestroyDebugReportCallbackEXT = nullptr; ++ ++ VkInstance fInstance = VK_NULL_HANDLE; ++ VkPhysicalDevice fPhysicalDevice = VK_NULL_HANDLE; ++ VkDevice fDevice = VK_NULL_HANDLE; ++ VkDebugReportCallbackEXT fDebugCallback = VK_NULL_HANDLE; ++ + sk_sp<const GrVkInterface> fInterface; + +- VkSurfaceKHR fSurface; +- VkSwapchainKHR fSwapchain; ++ // Original code had this as a function-local variable, but that seems wrong. ++ // It should exist as long as the context exists. ++ VkPhysicalDeviceFeatures2 features; ++ + uint32_t fGraphicsQueueIndex; + VkQueue fGraphicsQueue; + uint32_t fPresentQueueIndex; ++ ++ sk_sp<GrContext> fContext; ++ }; ++ ++ sk_sp<Shared> fShared; ++ ++ static sk_sp<Shared> fGlobalShared; ++ ++ VkSurfaceKHR fSurface; ++ VkSwapchainKHR fSwapchain; + VkQueue fPresentQueue; + + uint32_t fImageCount; +--- ./tools/sk_app/unix/VulkanWindowContext_unix.cpp.sav 2019-10-21 12:03:51.753745188 +0200 ++++ ./tools/sk_app/unix/VulkanWindowContext_unix.cpp 2019-11-15 12:08:01.605967642 +0100 +@@ -30,7 +30,7 @@ std::unique_ptr<WindowContext> MakeVulka + return nullptr; + } + +- auto createVkSurface = [&info, instProc](VkInstance instance) -> VkSurfaceKHR { ++ VulkanWindowContext::CreateVkSurfaceFn createVkSurface = [&info, instProc](VkInstance instance) -> VkSurfaceKHR { + static PFN_vkCreateXcbSurfaceKHR createXcbSurfaceKHR = nullptr; + if (!createXcbSurfaceKHR) { + createXcbSurfaceKHR = +@@ -54,6 +54,9 @@ std::unique_ptr<WindowContext> MakeVulka + + return surface; + }; ++ // Allow creating just the shared context, without an associated window. ++ if(info.fWindow == None) ++ createVkSurface = nullptr; + + auto canPresent = [&info, instProc](VkInstance instance, VkPhysicalDevice physDev, + uint32_t queueFamilyIndex) { +@@ -76,7 +79,7 @@ std::unique_ptr<WindowContext> MakeVulka + }; + std::unique_ptr<WindowContext> ctx( + new VulkanWindowContext(displayParams, createVkSurface, canPresent, instProc, devProc)); +- if (!ctx->isValid()) { ++ if (!ctx->isValid() && createVkSurface != nullptr) { + return nullptr; + } + return ctx; +--- ./tools/sk_app/win/VulkanWindowContext_win.cpp.sav 2019-10-21 12:03:51.753745188 +0200 ++++ ./tools/sk_app/win/VulkanWindowContext_win.cpp 2019-11-15 12:08:21.466022257 +0100 +@@ -30,7 +30,7 @@ std::unique_ptr<WindowContext> MakeVulka + return nullptr; + } + +- auto createVkSurface = [hwnd, instProc] (VkInstance instance) -> VkSurfaceKHR { ++ VulkanWindowContext::CreateVkSurfaceFn createVkSurface = [hwnd, instProc] (VkInstance instance) -> VkSurfaceKHR { + static PFN_vkCreateWin32SurfaceKHR createWin32SurfaceKHR = nullptr; + if (!createWin32SurfaceKHR) { + createWin32SurfaceKHR = (PFN_vkCreateWin32SurfaceKHR) +@@ -54,6 +54,9 @@ std::unique_ptr<WindowContext> MakeVulka + + return surface; + }; ++ // Allow creating just the shared context, without an associated window. ++ if(hwnd == nullptr) ++ createVkSurface = nullptr; + + auto canPresent = [instProc] (VkInstance instance, VkPhysicalDevice physDev, + uint32_t queueFamilyIndex) { +@@ -71,7 +74,7 @@ std::unique_ptr<WindowContext> MakeVulka + + std::unique_ptr<WindowContext> ctx( + new VulkanWindowContext(params, createVkSurface, canPresent, instProc, devProc)); +- if (!ctx->isValid()) { ++ if (!ctx->isValid() && createVkSurface != nullptr) { + return nullptr; + } + return ctx; +--- ./tools/sk_app/VulkanWindowContext.cpp.sav 2019-11-14 16:46:31.218722399 +0100 ++++ ./tools/sk_app/VulkanWindowContext.cpp 2019-11-15 11:58:46.656455921 +0100 +@@ -24,8 +24,10 @@ #undef CreateSemaphore #endif @@ -20,7 +175,7 @@ index 793c88c158..21164cac67 100644 namespace sk_app { -@@ -49,6 +50,14 @@ VulkanWindowContext::VulkanWindowContext(const DisplayParams& params, +@@ -49,6 +51,14 @@ VulkanWindowContext::VulkanWindowContext } void VulkanWindowContext::initializeContext() { @@ -35,7 +190,7 @@ index 793c88c158..21164cac67 100644 // any config code here (particularly for msaa)? PFN_vkGetInstanceProcAddr getInstanceProc = fGetInstanceProcAddr; -@@ -62,24 +71,25 @@ void VulkanWindowContext::initializeContext() { +@@ -62,24 +72,25 @@ void VulkanWindowContext::initializeCont }; GrVkBackendContext backendContext; GrVkExtensions extensions; @@ -71,7 +226,7 @@ index 793c88c158..21164cac67 100644 PFN_vkGetPhysicalDeviceProperties localGetPhysicalDeviceProperties = reinterpret_cast<PFN_vkGetPhysicalDeviceProperties>( -@@ -87,21 +97,31 @@ void VulkanWindowContext::initializeContext() { +@@ -87,21 +98,31 @@ void VulkanWindowContext::initializeCont backendContext.fInstance, VK_NULL_HANDLE)); if (!localGetPhysicalDeviceProperties) { @@ -108,7 +263,7 @@ index 793c88c158..21164cac67 100644 GET_PROC(DestroySurfaceKHR); GET_PROC(GetPhysicalDeviceSurfaceSupportKHR); GET_PROC(GetPhysicalDeviceSurfaceCapabilitiesKHR); -@@ -109,7 +129,6 @@ void VulkanWindowContext::initializeContext() { +@@ -109,7 +130,6 @@ void VulkanWindowContext::initializeCont GET_PROC(GetPhysicalDeviceSurfacePresentModesKHR); GET_DEV_PROC(DeviceWaitIdle); GET_DEV_PROC(QueueWaitIdle); @@ -116,12 +271,15 @@ index 793c88c158..21164cac67 100644 GET_DEV_PROC(CreateSwapchainKHR); GET_DEV_PROC(DestroySwapchainKHR); GET_DEV_PROC(GetSwapchainImagesKHR); -@@ -117,46 +136,40 @@ void VulkanWindowContext::initializeContext() { +@@ -117,46 +137,44 @@ void VulkanWindowContext::initializeCont GET_DEV_PROC(QueuePresentKHR); GET_DEV_PROC(GetDeviceQueue); - fContext = GrContext::MakeVulkan(backendContext, fDisplayParams.fGrContextOptions); -- ++ // No actual window, used just to create the shared GrContext. ++ if(fCreateVkSurfaceFn == nullptr) ++ return; + - fSurface = fCreateVkSurfaceFn(fInstance); + fSurface = fCreateVkSurfaceFn(fShared->fInstance); if (VK_NULL_HANDLE == fSurface) { @@ -130,6 +288,9 @@ index 793c88c158..21164cac67 100644 return; } ++ // create presentQueue ++ fGetDeviceQueue(fShared->fDevice, fShared->fPresentQueueIndex, 0, &fPresentQueue); ++ VkBool32 supported; - VkResult res = fGetPhysicalDeviceSurfaceSupportKHR(fPhysicalDevice, fPresentQueueIndex, + VkResult res = fGetPhysicalDeviceSurfaceSupportKHR(fShared->fPhysicalDevice, fShared->fPresentQueueIndex, @@ -145,11 +306,10 @@ index 793c88c158..21164cac67 100644 - sk_gpu_test::FreeVulkanFeaturesStructs(&features); return; } - - // create presentQueue +- +- // create presentQueue - fGetDeviceQueue(fDevice, fPresentQueueIndex, 0, &fPresentQueue); - sk_gpu_test::FreeVulkanFeaturesStructs(&features); -+ fGetDeviceQueue(fShared->fDevice, fShared->fPresentQueueIndex, 0, &fPresentQueue); } bool VulkanWindowContext::createSwapchain(int width, int height, @@ -168,7 +328,7 @@ index 793c88c158..21164cac67 100644 nullptr); if (VK_SUCCESS != res) { return false; -@@ -164,14 +177,14 @@ bool VulkanWindowContext::createSwapchain(int width, int height, +@@ -164,14 +182,14 @@ bool VulkanWindowContext::createSwapchai SkAutoMalloc surfaceFormatAlloc(surfaceFormatCount * sizeof(VkSurfaceFormatKHR)); VkSurfaceFormatKHR* surfaceFormats = (VkSurfaceFormatKHR*)surfaceFormatAlloc.get(); @@ -185,7 +345,7 @@ index 793c88c158..21164cac67 100644 nullptr); if (VK_SUCCESS != res) { return false; -@@ -179,7 +192,7 @@ bool VulkanWindowContext::createSwapchain(int width, int height, +@@ -179,7 +197,7 @@ bool VulkanWindowContext::createSwapchai SkAutoMalloc presentModeAlloc(presentModeCount * sizeof(VkPresentModeKHR)); VkPresentModeKHR* presentModes = (VkPresentModeKHR*)presentModeAlloc.get(); @@ -194,7 +354,7 @@ index 793c88c158..21164cac67 100644 presentModes); if (VK_SUCCESS != res) { return false; -@@ -286,8 +299,8 @@ bool VulkanWindowContext::createSwapchain(int width, int height, +@@ -286,8 +304,8 @@ bool VulkanWindowContext::createSwapchai swapchainCreateInfo.imageArrayLayers = 1; swapchainCreateInfo.imageUsage = usageFlags; @@ -205,7 +365,7 @@ index 793c88c158..21164cac67 100644 swapchainCreateInfo.imageSharingMode = VK_SHARING_MODE_CONCURRENT; swapchainCreateInfo.queueFamilyIndexCount = 2; swapchainCreateInfo.pQueueFamilyIndices = queueFamilies; -@@ -303,18 +316,18 @@ bool VulkanWindowContext::createSwapchain(int width, int height, +@@ -303,18 +321,18 @@ bool VulkanWindowContext::createSwapchai swapchainCreateInfo.clipped = true; swapchainCreateInfo.oldSwapchain = fSwapchain; @@ -227,7 +387,7 @@ index 793c88c158..21164cac67 100644 } this->createBuffers(swapchainCreateInfo.imageFormat, colorType); -@@ -323,10 +336,10 @@ bool VulkanWindowContext::createSwapchain(int width, int height, +@@ -323,10 +341,10 @@ bool VulkanWindowContext::createSwapchai } void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType) { @@ -240,7 +400,7 @@ index 793c88c158..21164cac67 100644 // set up initial image layouts and create surfaces fImageLayouts = new VkImageLayout[fImageCount]; -@@ -341,7 +354,7 @@ void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType) +@@ -341,7 +359,7 @@ void VulkanWindowContext::createBuffers( info.fImageTiling = VK_IMAGE_TILING_OPTIMAL; info.fFormat = format; info.fLevelCount = 1; @@ -249,7 +409,7 @@ index 793c88c158..21164cac67 100644 if (fSampleCount == 1) { GrBackendRenderTarget backendRT(fWidth, fHeight, fSampleCount, info); -@@ -372,8 +385,8 @@ void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType) +@@ -372,8 +390,8 @@ void VulkanWindowContext::createBuffers( fBackbuffers = new BackbufferInfo[fImageCount + 1]; for (uint32_t i = 0; i < fImageCount + 1; ++i) { fBackbuffers[i].fImageIndex = -1; @@ -260,7 +420,7 @@ index 793c88c158..21164cac67 100644 nullptr, &fBackbuffers[i].fRenderSemaphore)); } fCurrentBackbufferIndex = fImageCount; -@@ -384,8 +397,8 @@ void VulkanWindowContext::destroyBuffers() { +@@ -384,8 +402,8 @@ void VulkanWindowContext::destroyBuffers if (fBackbuffers) { for (uint32_t i = 0; i < fImageCount + 1; ++i) { fBackbuffers[i].fImageIndex = -1; @@ -271,7 +431,7 @@ index 793c88c158..21164cac67 100644 fBackbuffers[i].fRenderSemaphore, nullptr)); } -@@ -410,41 +423,55 @@ VulkanWindowContext::~VulkanWindowContext() { +@@ -410,41 +428,55 @@ VulkanWindowContext::~VulkanWindowContex void VulkanWindowContext::destroyContext() { if (this->isValid()) { fQueueWaitIdle(fPresentQueue); @@ -340,7 +500,7 @@ index 793c88c158..21164cac67 100644 } VulkanWindowContext::BackbufferInfo* VulkanWindowContext::getAvailableBackbuffer() { -@@ -470,34 +497,34 @@ sk_sp<SkSurface> VulkanWindowContext::getBackbufferSurface() { +@@ -470,34 +502,34 @@ sk_sp<SkSurface> VulkanWindowContext::ge semaphoreInfo.pNext = nullptr; semaphoreInfo.flags = 0; VkSemaphore semaphore; @@ -381,119 +541,10 @@ index 793c88c158..21164cac67 100644 return nullptr; } } -@@ -541,4 +568,6 @@ void VulkanWindowContext::swapBuffers() { +@@ -541,4 +573,6 @@ void VulkanWindowContext::swapBuffers() fQueuePresentKHR(fPresentQueue, &presentInfo); } +SK_API sk_sp<VulkanWindowContext::Shared> VulkanWindowContext::fGlobalShared; + } //namespace sk_app -diff --git a/tools/sk_app/VulkanWindowContext.h b/tools/sk_app/VulkanWindowContext.h -index 2db9e79ae6..7950dc159b 100644 ---- a/tools/sk_app/VulkanWindowContext.h -+++ b/tools/sk_app/VulkanWindowContext.h -@@ -1,4 +1,3 @@ -- - /* - * Copyright 2016 Google Inc. - * -@@ -23,14 +22,30 @@ class GrRenderTarget; - - namespace sk_app { - --class VulkanWindowContext : public WindowContext { -+class SK_API VulkanWindowContext : public WindowContext { -+ struct Shared; - public: - ~VulkanWindowContext() override; - -+ class SharedGrContext { -+ public: -+ SharedGrContext() {} -+ GrContext* getGrContext() { return shared ? shared->fContext.get() : nullptr; } -+ ~SharedGrContext() { shared.reset(); checkDestroyShared(); } -+ bool operator!() const { return !shared; } -+ void reset() { shared.reset(); } -+ private: -+ friend class VulkanWindowContext; -+ SharedGrContext(sk_sp<Shared>& sh ) : shared( sh ) {} -+ sk_sp<Shared> shared; -+ }; -+ -+ static SharedGrContext getSharedGrContext() { return SharedGrContext( fGlobalShared ); } -+ - sk_sp<SkSurface> getBackbufferSurface() override; - void swapBuffers() override; - -- bool isValid() override { return fDevice != VK_NULL_HANDLE; } -+ bool isValid() override { return fShared && fShared->fDevice != VK_NULL_HANDLE; } - - void resize(int w, int h) override { - this->createSwapchain(w, h, fDisplayParams); -@@ -53,6 +68,7 @@ public: - private: - void initializeContext(); - void destroyContext(); -+ static void checkDestroyShared(); - - struct BackbufferInfo { - uint32_t fImageIndex; // image this is associated with -@@ -64,11 +80,6 @@ private: - void createBuffers(VkFormat format, SkColorType colorType); - void destroyBuffers(); - -- VkInstance fInstance = VK_NULL_HANDLE; -- VkPhysicalDevice fPhysicalDevice = VK_NULL_HANDLE; -- VkDevice fDevice = VK_NULL_HANDLE; -- VkDebugReportCallbackEXT fDebugCallback = VK_NULL_HANDLE; -- - // Create functions - CreateVkSurfaceFn fCreateVkSurfaceFn; - CanPresentFn fCanPresentFn; -@@ -90,20 +101,41 @@ private: - PFN_vkAcquireNextImageKHR fAcquireNextImageKHR = nullptr; - PFN_vkQueuePresentKHR fQueuePresentKHR = nullptr; - -- PFN_vkDestroyInstance fDestroyInstance = nullptr; - PFN_vkDeviceWaitIdle fDeviceWaitIdle = nullptr; -- PFN_vkDestroyDebugReportCallbackEXT fDestroyDebugReportCallbackEXT = nullptr; - PFN_vkQueueWaitIdle fQueueWaitIdle = nullptr; -- PFN_vkDestroyDevice fDestroyDevice = nullptr; - PFN_vkGetDeviceQueue fGetDeviceQueue = nullptr; - -+ // We need to use just one GrContext, so share all the relevant data. -+ struct Shared : public SkRefCnt -+ { -+ PFN_vkDestroyInstance fDestroyInstance = nullptr; -+ PFN_vkDestroyDevice fDestroyDevice = nullptr; -+ PFN_vkDestroyDebugReportCallbackEXT fDestroyDebugReportCallbackEXT = nullptr; -+ -+ VkInstance fInstance = VK_NULL_HANDLE; -+ VkPhysicalDevice fPhysicalDevice = VK_NULL_HANDLE; -+ VkDevice fDevice = VK_NULL_HANDLE; -+ VkDebugReportCallbackEXT fDebugCallback = VK_NULL_HANDLE; -+ - sk_sp<const GrVkInterface> fInterface; - -- VkSurfaceKHR fSurface; -- VkSwapchainKHR fSwapchain; -+ // Original code had this as a function-local variable, but that seems wrong. -+ // It should exist as long as the context exists. -+ VkPhysicalDeviceFeatures2 features; -+ - uint32_t fGraphicsQueueIndex; - VkQueue fGraphicsQueue; - uint32_t fPresentQueueIndex; -+ -+ sk_sp<GrContext> fContext; -+ }; -+ -+ sk_sp<Shared> fShared; -+ -+ static sk_sp<Shared> fGlobalShared; -+ -+ VkSurfaceKHR fSurface; -+ VkSwapchainKHR fSwapchain; - VkQueue fPresentQueue; - - uint32_t fImageCount; diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index 7c41e98a91b5..0036b2d89f7a 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -211,6 +211,8 @@ protected: void destroySurface(); // Reimplemented for X11. virtual bool avoidRecreateByResize() const { return false; } + void createWindowSurface(); + virtual void createWindowContext() = 0; void createOffscreenSurface(); void privateDrawAlphaRect(long nX, long nY, long nWidth, long nHeight, double nTransparency, @@ -256,11 +258,12 @@ protected: SalGraphics& mParent; /// Pointer to the SalFrame or SalVirtualDevice SalGeometryProvider* mProvider; + std::unique_ptr<sk_app::WindowContext> mWindowContext; + sk_app::VulkanWindowContext::SharedGrContext mOffscreenGrContext; // The Skia surface that is target of all the rendering. sk_sp<SkSurface> mSurface; bool mIsGPU; // whether the surface is GPU-backed // Keep reference to shared GrContext. - sk_app::VulkanWindowContext::SharedGrContext mOffscreenGrContext; vcl::Region mClipRegion; Color mLineColor; Color mFillColor; diff --git a/vcl/inc/skia/x11/gdiimpl.hxx b/vcl/inc/skia/x11/gdiimpl.hxx index 1dc5064e6667..4d88740b8ba9 100644 --- a/vcl/inc/skia/x11/gdiimpl.hxx +++ b/vcl/inc/skia/x11/gdiimpl.hxx @@ -16,11 +16,6 @@ #include <unx/x11/x11gdiimpl.h> #include <skia/gdiimpl.hxx> -namespace sk_app -{ -class WindowContext; -} - class VCL_PLUGIN_PUBLIC X11SkiaSalGraphicsImpl : public SkiaSalGraphicsImpl, public X11GraphicsImpl { private: @@ -35,12 +30,9 @@ public: virtual void freeResources() override; protected: - virtual void createSurface() override; + virtual void createWindowContext() override; virtual void performFlush() override; virtual bool avoidRecreateByResize() const override; - -private: - std::unique_ptr<sk_app::WindowContext> mWindowContext; }; #endif // INCLUDED_VCL_INC_SKIA_X11_GDIIMPL_HXX diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index caed9fb33431..948c85df72e4 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -31,7 +31,6 @@ #include <SkRegion.h> #include <SkDashPathEffect.h> #include <GrBackendSurface.h> -#include <GrContextFactory.h> #include <basegfx/polygon/b2dpolygontools.hxx> @@ -182,6 +181,7 @@ SkiaSalGraphicsImpl::SkiaSalGraphicsImpl(SalGraphics& rParent, SalGeometryProvid SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl() { assert(!mSurface); + assert(!mWindowContext); assert(!mOffscreenGrContext); } @@ -191,6 +191,17 @@ void SkiaSalGraphicsImpl::recreateSurface() { destroySurface(); createSurface(); +} + +void SkiaSalGraphicsImpl::createSurface() +{ + if (isOffscreen()) + createOffscreenSurface(); + else + createWindowSurface(); +#ifdef DBG_UTIL + prefillSurface(); +#endif mSurface->getCanvas()->save(); // see SetClipRegion() mClipRegion = vcl::Region(tools::Rectangle(0, 0, GetWidth(), GetHeight())); @@ -199,20 +210,35 @@ void SkiaSalGraphicsImpl::recreateSurface() mFlush->SetPriority(TaskPriority::POST_PAINT); } -void SkiaSalGraphicsImpl::createSurface() +void SkiaSalGraphicsImpl::createWindowSurface() { - // Create raster surface. Subclasses will create GPU-backed surfaces as appropriate. - mSurface = SkSurface::MakeRasterN32Premul(GetWidth(), GetHeight()); - mIsGPU = false; -#ifdef DBG_UTIL - prefillSurface(); -#endif + assert(!isOffscreen()); + assert(!mSurface); + assert(!mWindowContext); + createWindowContext(); + if (mWindowContext) + mSurface = mWindowContext->getBackbufferSurface(); + if (!mSurface) + { + switch (SkiaHelper::renderMethodToUse()) + { + case SkiaHelper::RenderVulkan: + SAL_WARN("vcl.skia", "cannot create Vulkan GPU window surface, disabling Vulkan"); + // fall back to raster + SkiaHelper::disableRenderMethod(SkiaHelper::RenderVulkan); + destroySurface(); // destroys also WindowContext + return createWindowSurface(); // try again + case SkiaHelper::RenderRaster: + abort(); // this should not really happen + } + } } void SkiaSalGraphicsImpl::createOffscreenSurface() { assert(isOffscreen()); - destroySurface(); + assert(!mSurface); + assert(!mWindowContext); switch (SkiaHelper::renderMethodToUse()) { case SkiaHelper::RenderVulkan: @@ -228,11 +254,16 @@ void SkiaSalGraphicsImpl::createOffscreenSurface() static bool isUnitTest = (getenv("LO_TESTNAME") != nullptr); if (isUnitTest) { - static vcl::DeleteOnDeinit<sk_gpu_test::GrContextFactory> factory( - new sk_gpu_test::GrContextFactory); - // The factory owns the context. - grContext - = factory.get()->get(sk_gpu_test::GrContextFactory::kVulkan_ContextType); + // Create temporary WindowContext with no window. That will fail, + // but it will initialize the shared GrContext. + createWindowContext(); + // Keep a reference. + sk_app::VulkanWindowContext::SharedGrContext context + = sk_app::VulkanWindowContext::getSharedGrContext(); + // Destroy the temporary WindowContext. + destroySurface(); + mOffscreenGrContext = context; + grContext = mOffscreenGrContext.getGrContext(); } } if (grContext) @@ -241,7 +272,7 @@ void SkiaSalGraphicsImpl::createOffscreenSurface() grContext, SkBudgeted::kNo, SkImageInfo::MakeN32Premul(GetWidth(), GetHeight())); mIsGPU = true; - assert(mSurface.get()); + assert(mSurface); #ifdef DBG_UTIL prefillSurface(); #endif @@ -254,7 +285,10 @@ void SkiaSalGraphicsImpl::createOffscreenSurface() default: break; } - return SkiaSalGraphicsImpl::createSurface(); // create a raster one + // Create raster surface. Subclasses will create GPU-backed surfaces as appropriate. + mSurface = SkSurface::MakeRasterN32Premul(GetWidth(), GetHeight()); + assert(mSurface); + mIsGPU = false; } void SkiaSalGraphicsImpl::destroySurface() @@ -266,6 +300,7 @@ void SkiaSalGraphicsImpl::destroySurface() // if this fails, something forgot to use SkAutoCanvasRestore assert(mSurface->getCanvas()->getTotalMatrix().isIdentity()); } + // TODO Is this still needed? // If we use e.g. Vulkan, we must destroy the surface before the context, // otherwise destroying the surface will reference the context. This is // handled by calling destroySurface() before destroying the context. @@ -276,6 +311,7 @@ void SkiaSalGraphicsImpl::destroySurface() if (mSurface) mSurface->flush(); mSurface.reset(); + mWindowContext.reset(); mIsGPU = false; mOffscreenGrContext.reset(); } diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx index e679fa8dbb2d..3f706399f136 100644 --- a/vcl/skia/x11/gdiimpl.cxx +++ b/vcl/skia/x11/gdiimpl.cxx @@ -38,11 +38,8 @@ void X11SkiaSalGraphicsImpl::Init() SkiaSalGraphicsImpl::Init(); } -void X11SkiaSalGraphicsImpl::createSurface() +void X11SkiaSalGraphicsImpl::createWindowContext() { - if (isOffscreen()) - return createOffscreenSurface(); - destroySurface(); sk_app::DisplayParams displayParams; // Use a macro to hide an unreachable code warning. // TODO The Skia Xlib code actually requires the non-native color type to work properly. @@ -53,7 +50,10 @@ void X11SkiaSalGraphicsImpl::createSurface() sk_app::window_context_factory::XlibWindowInfo winInfo; winInfo.fDisplay = mX11Parent.GetXDisplay(); winInfo.fWindow = mX11Parent.GetDrawable(); - assert(winInfo.fDisplay && winInfo.fWindow != None); + assert(winInfo.fDisplay); + // Allow window being None if offscreen, this is used to temporarily create GrContext + // for an offscreen surface. + assert(winInfo.fWindow != None || isOffscreen()); winInfo.fFBConfig = nullptr; // not used winInfo.fVisualInfo = const_cast<SalVisual*>(&mX11Parent.GetVisual()); #ifdef DBG_UTIL @@ -70,28 +70,14 @@ void X11SkiaSalGraphicsImpl::createSurface() case SkiaHelper::RenderRaster: mWindowContext = sk_app::window_context_factory::MakeRasterForXlib(winInfo, displayParams); - assert(SkToBool(mWindowContext)); - mSurface = mWindowContext->getBackbufferSurface(); - assert(mSurface.get()); mIsGPU = false; break; case SkiaHelper::RenderVulkan: mWindowContext = sk_app::window_context_factory::MakeVulkanForXlib(winInfo, displayParams); - if (mWindowContext) - mSurface = mWindowContext->getBackbufferSurface(); - if (!mSurface) - { - SAL_WARN("vcl.skia", "cannot create Vulkan GPU surface, disabling Vulkan"); - SkiaHelper::disableRenderMethod(SkiaHelper::RenderVulkan); - return createSurface(); // try again - } mIsGPU = true; break; } -#ifdef DBG_UTIL - prefillSurface(); -#endif } bool X11SkiaSalGraphicsImpl::avoidRecreateByResize() const commit 9a3d08463033a1e38b75ab6da5fc43bee6528dd0 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Wed Nov 13 17:24:42 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:59 2019 +0100 add invert() tests to visualbackendtest They already exist and are used by the unittest. And the TrackFrame test actually appears to expect incorrect results (or otherwise pretty much all backends implement the operation incorrectly). Change-Id: I26867a2d1b0f01b5e836131932b422cb8823fb5b diff --git a/vcl/backendtest/VisualBackendTest.cxx b/vcl/backendtest/VisualBackendTest.cxx index a1134df5f0da..9c19c2b66abc 100644 --- a/vcl/backendtest/VisualBackendTest.cxx +++ b/vcl/backendtest/VisualBackendTest.cxx @@ -88,7 +88,7 @@ class VisualBackendTestWindow : public WorkWindow private: Timer maUpdateTimer; std::vector<std::chrono::high_resolution_clock::time_point> mTimePoints; - static constexpr unsigned char gnNumberOfTests = 7; + static constexpr unsigned char gnNumberOfTests = 8; unsigned char mnTest; bool mbAnimate; ScopedVclPtr<VirtualDevice> mpVDev; @@ -389,6 +389,43 @@ public: } } + static void testInvert(vcl::RenderContext& rRenderContext, int nWidth, int nHeight) + { + tools::Rectangle aRectangle; + size_t index = 0; + + std::vector<tools::Rectangle> aRegions = setupRegions(2, 2, nWidth, nHeight); + + aRectangle = aRegions[index++]; + { + vcl::test::OutputDeviceTestRect aOutDevTest; + Bitmap aBitmap = aOutDevTest.setupInvert_NONE(); + assertAndSetBackground(vcl::test::OutputDeviceTestRect::checkInvertRectangle(aBitmap), aRectangle, rRenderContext); + drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext); + } + aRectangle = aRegions[index++]; + { + vcl::test::OutputDeviceTestRect aOutDevTest; + Bitmap aBitmap = aOutDevTest.setupInvert_N50(); + assertAndSetBackground(vcl::test::OutputDeviceTestRect::checkInvertN50Rectangle(aBitmap), aRectangle, rRenderContext); + drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext); + } + aRectangle = aRegions[index++]; + { + vcl::test::OutputDeviceTestRect aOutDevTest; + Bitmap aBitmap = aOutDevTest.setupInvert_TrackFrame(); + assertAndSetBackground(vcl::test::OutputDeviceTestRect::checkInvertTrackFrameRectangle(aBitmap), aRectangle, rRenderContext); + drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext); + } + aRectangle = aRegions[index++]; + { + vcl::test::OutputDeviceTestAnotherOutDev aOutDevTest; + Bitmap aBitmap = aOutDevTest.setupXOR(); + assertAndSetBackground(vcl::test::OutputDeviceTestAnotherOutDev::checkXOR(aBitmap), aRectangle, rRenderContext); + drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext); + } + } + virtual void Paint(vcl::RenderContext& rRenderContext, const tools::Rectangle& /*rRect*/) override { if (mnTest % gnNumberOfTests == gnNumberOfTests - 1) @@ -485,7 +522,11 @@ public: } else if (mnTest % gnNumberOfTests == 5) { - std::vector<tools::Rectangle> aRegions = setupRegions(3, 2, nWidth, nHeight); + testInvert(rRenderContext, nWidth, nHeight); + } + else if (mnTest % gnNumberOfTests == 6) + { + std::vector<tools::Rectangle> aRegions = setupRegions(3, 1, nWidth, nHeight); aRectangle = aRegions[index++]; { @@ -496,13 +537,6 @@ public: } aRectangle = aRegions[index++]; { - vcl::test::OutputDeviceTestAnotherOutDev aOutDevTest; - Bitmap aBitmap = aOutDevTest.setupXOR(); - assertAndSetBackground(vcl::test::OutputDeviceTestAnotherOutDev::checkXOR(aBitmap), aRectangle, rRenderContext); - drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext); - } - aRectangle = aRegions[index++]; - { vcl::test::OutputDeviceTestGradient aOutDevTest; Bitmap aBitmap = aOutDevTest.setupLinearGradient(); drawBitmapScaledAndCentered(aRectangle, aBitmap, rRenderContext); diff --git a/vcl/qa/cppunit/BackendTest.cxx b/vcl/qa/cppunit/BackendTest.cxx index 01a81c0f17f3..6dfda5d02d35 100644 --- a/vcl/qa/cppunit/BackendTest.cxx +++ b/vcl/qa/cppunit/BackendTest.cxx @@ -465,7 +465,8 @@ public: CPPUNIT_TEST(testDrawInvertWithRectangle); CPPUNIT_TEST(testDrawInvertN50WithRectangle); - // CPPUNIT_TEST(testDrawInvertTrackFrameWithRectangle); TODO SKIA + // AFAIK this test (itself) is broken. + // CPPUNIT_TEST(testDrawInvertTrackFrameWithRectangle); CPPUNIT_TEST(testDrawBezierWithPolylineB2D); CPPUNIT_TEST(testDrawBezierAAWithPolylineB2D); commit bd0609e0e16b69775167c41c1e6d44f3e6cfe997 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Thu Nov 14 11:26:58 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:59 2019 +0100 do not use Xlib for accessing something that is not an X window Change-Id: I8986064e581fdb9876068ae3b9736b9716554fb6 diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx index 7eb02a51169e..e679fa8dbb2d 100644 --- a/vcl/skia/x11/gdiimpl.cxx +++ b/vcl/skia/x11/gdiimpl.cxx @@ -96,7 +96,7 @@ void X11SkiaSalGraphicsImpl::createSurface() bool X11SkiaSalGraphicsImpl::avoidRecreateByResize() const { - if (!mSurface) + if (!mSurface || isOffscreen()) return false; // Skia's WindowContext uses actual dimensions of the X window, which due to X11 being // asynchronous may be temporarily different from what VCL thinks are the dimensions. commit d614f62df56b1f762886405b6721d02ca92f7f61 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Wed Nov 13 13:58:58 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:58 2019 +0100 remove no-longer-needed Skia workaround for GPU offscreen drawing Change-Id: I044a9a31af71c4c624f08a0813bc59472f4c728a diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index cb2beb2667a0..caed9fb33431 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -739,11 +739,6 @@ void SkiaSalGraphicsImpl::copyBits(const SalTwoRect& rPosAry, SalGraphics* pSrcG assert(dynamic_cast<SkiaSalGraphicsImpl*>(pSrcGraphics->GetImpl())); src = static_cast<SkiaSalGraphicsImpl*>(pSrcGraphics->GetImpl()); src->checkSurface(); - // TODO Without this flush() Skia asserts if both src and destination are - // GPU-backed SkSurface that come from different GrContext (e.g. when - // src comes from SkiaVulkanGrContext and target is a window). I don't - // know if it's a Skia bug or our GrContext usage is incorrect. - src->mSurface->flush(); } else src = this; commit 7519c285a3d6c612180fed04da38384c5c77f207 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Wed Nov 13 13:27:57 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:57 2019 +0100 make Skia VCL backend fall back to raster if vulkan doesn't work Change-Id: Ic446f6f85e5ebc2e50cb51a3ed1e732b8976a193 diff --git a/external/skia/share-grcontext.patch.1 b/external/skia/share-grcontext.patch.1 index 0492bd948240..357c3a885880 100644 --- a/external/skia/share-grcontext.patch.1 +++ b/external/skia/share-grcontext.patch.1 @@ -426,7 +426,7 @@ index 2db9e79ae6..7950dc159b 100644 void swapBuffers() override; - bool isValid() override { return fDevice != VK_NULL_HANDLE; } -+ bool isValid() override { return fShared->fDevice != VK_NULL_HANDLE; } ++ bool isValid() override { return fShared && fShared->fDevice != VK_NULL_HANDLE; } void resize(int w, int h) override { this->createSwapchain(w, h, fDisplayParams); diff --git a/include/vcl/skia/SkiaHelper.hxx b/include/vcl/skia/SkiaHelper.hxx index 77e9fab5797a..d27cffd650f3 100644 --- a/include/vcl/skia/SkiaHelper.hxx +++ b/include/vcl/skia/SkiaHelper.hxx @@ -12,6 +12,8 @@ #include <vcl/dllapi.h> +#include <config_features.h> + // All member functions static and VCL_DLLPUBLIC. Basically a glorified namespace. struct VCL_DLLPUBLIC SkiaHelper { @@ -19,6 +21,17 @@ struct VCL_DLLPUBLIC SkiaHelper public: static bool isVCLSkiaEnabled(); + +#if HAVE_FEATURE_SKIA + // Which Skia backend to use. + enum RenderMethod + { + RenderRaster, + RenderVulkan + }; + static RenderMethod renderMethodToUse(); + static void disableRenderMethod(RenderMethod method); +#endif }; #endif diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index 195b5d877eed..7c41e98a91b5 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -241,14 +241,6 @@ protected: SkScalar toSkX(long x) const { return mIsGPU ? x + 0.5 : x; } SkScalar toSkY(long y) const { return mIsGPU ? y + 0.5 : y; } - // Which Skia backend to use. - enum RenderMethod - { - RenderRaster, - RenderVulkan - }; - static RenderMethod renderMethodToUse(); - #ifdef DBG_UTIL void prefillSurface(); #endif diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx index bc643bd546b1..810e8f9b0b11 100644 --- a/vcl/skia/SkiaHelper.cxx +++ b/vcl/skia/SkiaHelper.cxx @@ -13,8 +13,6 @@ #include <desktop/crashreport.hxx> #include <officecfg/Office/Common.hxx> -#include <config_features.h> - #if !HAVE_FEATURE_SKIA bool SkiaHelper::isVCLSkiaEnabled() { return false; } @@ -86,6 +84,37 @@ bool SkiaHelper::isVCLSkiaEnabled() return bRet; } +static SkiaHelper::RenderMethod methodToUse = SkiaHelper::RenderRaster; + +static bool initRenderMethodToUse() +{ + if (const char* env = getenv("SAL_SKIA")) + { + if (strcmp(env, "raster") == 0) + { + methodToUse = SkiaHelper::RenderRaster; + return true; + } + } + methodToUse = SkiaHelper::RenderVulkan; + return true; +} + +SkiaHelper::RenderMethod SkiaHelper::renderMethodToUse() +{ + static bool methodToUseInited = initRenderMethodToUse(); + (void)methodToUseInited; // Used just to ensure thread-safe one-time init. + return methodToUse; +} + +void SkiaHelper::disableRenderMethod(RenderMethod method) +{ + if (renderMethodToUse() != method) + return; + // Choose a fallback, right now always raster. + methodToUse = RenderRaster; +} + #endif // HAVE_FEATURE_SKIA /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index ac47d254cc03..cb2beb2667a0 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -24,6 +24,7 @@ #include <vcl/idle.hxx> #include <vcl/svapp.hxx> #include <vcl/lazydelete.hxx> +#include <vcl/skia/SkiaHelper.hxx> #include <SkCanvas.h> #include <SkPath.h> @@ -168,20 +169,6 @@ public: } }; -SkiaSalGraphicsImpl::RenderMethod SkiaSalGraphicsImpl::renderMethodToUse() -{ - static RenderMethod method = [] { - if (const char* env = getenv("SAL_SKIA")) - { - if (strcmp(env, "raster") == 0) - return RenderRaster; - } - return RenderVulkan; - }(); - - return method; -} - SkiaSalGraphicsImpl::SkiaSalGraphicsImpl(SalGraphics& rParent, SalGeometryProvider* pProvider) : mParent(rParent) , mProvider(pProvider) @@ -226,9 +213,9 @@ void SkiaSalGraphicsImpl::createOffscreenSurface() { assert(isOffscreen()); destroySurface(); - switch (renderMethodToUse()) + switch (SkiaHelper::renderMethodToUse()) { - case RenderVulkan: + case SkiaHelper::RenderVulkan: { mOffscreenGrContext = sk_app::VulkanWindowContext::getSharedGrContext(); GrContext* grContext = mOffscreenGrContext.getGrContext(); @@ -260,7 +247,8 @@ void SkiaSalGraphicsImpl::createOffscreenSurface() #endif return; } - SAL_WARN("vcl.skia", "cannot create Vulkan GPU offscreen surface"); + SAL_WARN("vcl.skia", "cannot create Vulkan offscreen GPU surface, disabling Vulkan"); + SkiaHelper::disableRenderMethod(SkiaHelper::RenderVulkan); break; } default: diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx index 8f30c3486dac..cba8bed29355 100644 --- a/vcl/skia/win/gdiimpl.cxx +++ b/vcl/skia/win/gdiimpl.cxx @@ -9,12 +9,13 @@ #include <skia/win/gdiimpl.hxx> -#include <tools/sk_app/win/WindowContextFactory_win.h> -#include <tools/sk_app/WindowContext.h> #include <win/saldata.hxx> +#include <vcl/skia/SkiaHelper.hxx> #include <SkColorFilter.h> #include <SkPixelRef.h> +#include <tools/sk_app/win/WindowContextFactory_win.h> +#include <tools/sk_app/WindowContext.h> WinSkiaSalGraphicsImpl::WinSkiaSalGraphicsImpl(WinSalGraphics& rGraphics, SalGeometryProvider* mpProvider) @@ -52,22 +53,30 @@ void WinSkiaSalGraphicsImpl::createSurface() // valid here, but better check. assert(GetWidth() != 0 && GetHeight() != 0); sk_app::DisplayParams displayParams; - switch (renderMethodToUse()) + switch (SkiaHelper::renderMethodToUse()) { - case RenderRaster: + case SkiaHelper::RenderRaster: mWindowContext = sk_app::window_context_factory::MakeRasterForWin(mWinParent.gethWnd(), displayParams); + assert(SkToBool(mWindowContext)); + mSurface = mWindowContext->getBackbufferSurface(); + assert(mSurface.get()); mIsGPU = false; break; - case RenderVulkan: + case SkiaHelper::RenderVulkan: mWindowContext = sk_app::window_context_factory::MakeVulkanForWin(mWinParent.gethWnd(), displayParams); + if (mWindowContext) + mSurface = mWindowContext->getBackbufferSurface(); + if (!mSurface) + { + SAL_WARN("vcl.skia", "cannot create Vulkan GPU surface, disabling Vulkan"); + SkiaHelper::disableRenderMethod(SkiaHelper::RenderVulkan); + return createSurface(); // try again + } mIsGPU = true; break; } - assert(SkToBool(mWindowContext)); // TODO - mSurface = mWindowContext->getBackbufferSurface(); - assert(mSurface.get()); #ifdef DBG_UTIL prefillSurface(); #endif diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx index ef381c5e56d3..7eb02a51169e 100644 --- a/vcl/skia/x11/gdiimpl.cxx +++ b/vcl/skia/x11/gdiimpl.cxx @@ -21,6 +21,8 @@ #include <tools/sk_app/unix/WindowContextFactory_unix.h> #include <tools/sk_app/WindowContext.h> +#include <vcl/skia/SkiaHelper.hxx> + X11SkiaSalGraphicsImpl::X11SkiaSalGraphicsImpl(X11SalGraphics& rParent) : SkiaSalGraphicsImpl(rParent, rParent.GetGeometryProvider()) , mX11Parent(rParent) @@ -63,22 +65,30 @@ void X11SkiaSalGraphicsImpl::createSurface() #endif winInfo.fWidth = GetWidth(); winInfo.fHeight = GetHeight(); - switch (renderMethodToUse()) + switch (SkiaHelper::renderMethodToUse()) { - case RenderRaster: + case SkiaHelper::RenderRaster: mWindowContext = sk_app::window_context_factory::MakeRasterForXlib(winInfo, displayParams); + assert(SkToBool(mWindowContext)); + mSurface = mWindowContext->getBackbufferSurface(); + assert(mSurface.get()); mIsGPU = false; break; - case RenderVulkan: + case SkiaHelper::RenderVulkan: mWindowContext = sk_app::window_context_factory::MakeVulkanForXlib(winInfo, displayParams); + if (mWindowContext) + mSurface = mWindowContext->getBackbufferSurface(); + if (!mSurface) + { + SAL_WARN("vcl.skia", "cannot create Vulkan GPU surface, disabling Vulkan"); + SkiaHelper::disableRenderMethod(SkiaHelper::RenderVulkan); + return createSurface(); // try again + } mIsGPU = true; break; } - assert(SkToBool(mWindowContext)); // TODO - mSurface = mWindowContext->getBackbufferSurface(); - assert(mSurface.get()); #ifdef DBG_UTIL prefillSurface(); #endif commit 9da976bb42cd5e20680b790ddda9d90237840584 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Tue Nov 12 16:10:50 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:57 2019 +0100 fix Skia offscreen GPU-backed drawing The previous approach of using multiple GrContext instances apparently does not work, it's not possible to do drawing operations that involve objects using two different GrContext's. So patch Skia to use just one GrContext for our needs. See vcl/skia/README for details. Change-Id: I2bd3d3c618bf7f8ff45b2f37cbd086d2289940aa diff --git a/external/skia/README b/external/skia/README index 767f12e91936..95be1561d4fb 100644 --- a/external/skia/README +++ b/external/skia/README @@ -17,3 +17,9 @@ tar cvJf skia-mXX-$id.tar.xz skia (where XX refers to the branch version) And review differences for BUILD.gn and relevant files in gn/ . + + +GrContext sharing +================= + +For details about the share-grcontext patch, see vcl/skia/README. diff --git a/external/skia/UnpackedTarball_skia.mk b/external/skia/UnpackedTarball_skia.mk index e9905f5abfa7..c3988042a012 100644 --- a/external/skia/UnpackedTarball_skia.mk +++ b/external/skia/UnpackedTarball_skia.mk @@ -12,7 +12,13 @@ $(eval $(call gb_UnpackedTarball_UnpackedTarball,skia)) $(eval $(call gb_UnpackedTarball_set_tarball,skia,$(SKIA_TARBALL))) # TODO -skia_patches := lerp.patch fix-pch.patch fix-ddi.patch make-api-visible.patch.1 fix-shader-locale.patch.1 +skia_patches := \ + lerp.patch \ + fix-pch.patch \ + fix-ddi.patch \ + make-api-visible.patch.1 \ + fix-shader-locale.patch.1 \ + share-grcontext.patch.1 $(eval $(call gb_UnpackedTarball_set_patchlevel,skia,1)) diff --git a/external/skia/share-grcontext.patch.1 b/external/skia/share-grcontext.patch.1 new file mode 100644 index 000000000000..0492bd948240 --- /dev/null +++ b/external/skia/share-grcontext.patch.1 @@ -0,0 +1,499 @@ +diff --git a/tools/sk_app/VulkanWindowContext.cpp b/tools/sk_app/VulkanWindowContext.cpp +index 793c88c158..21164cac67 100644 +--- a/tools/sk_app/VulkanWindowContext.cpp ++++ b/tools/sk_app/VulkanWindowContext.cpp +@@ -1,4 +1,3 @@ +- + /* + * Copyright 2015 Google Inc. + * +@@ -24,8 +23,10 @@ + #undef CreateSemaphore + #endif + +-#define GET_PROC(F) f ## F = (PFN_vk ## F) fGetInstanceProcAddr(fInstance, "vk" #F) +-#define GET_DEV_PROC(F) f ## F = (PFN_vk ## F) fGetDeviceProcAddr(fDevice, "vk" #F) ++#define GET_PROC(F) f ## F = (PFN_vk ## F) fGetInstanceProcAddr(fShared->fInstance, "vk" #F) ++#define GET_DEV_PROC(F) f ## F = (PFN_vk ## F) fGetDeviceProcAddr(fShared->fDevice, "vk" #F) ++#define GET_PROC_GLOBAL(F) fGlobalShared->f ## F = (PFN_vk ## F) fGetInstanceProcAddr(fGlobalShared->fInstance, "vk" #F) ++#define GET_DEV_PROC_GLOBAL(F) fGlobalShared->f ## F = (PFN_vk ## F) fGetDeviceProcAddr(fGlobalShared->fDevice, "vk" #F) + + namespace sk_app { + +@@ -49,6 +50,14 @@ VulkanWindowContext::VulkanWindowContext(const DisplayParams& params, + } + + void VulkanWindowContext::initializeContext() { ++ fShared = fGlobalShared; ++ if( !fShared ) ++ { ++ // TODO do we need a mutex? ++ ++ fGlobalShared = sk_make_sp<Shared>(); ++ Shared* d = fGlobalShared.get(); // shorter variable name ++ + // any config code here (particularly for msaa)? + + PFN_vkGetInstanceProcAddr getInstanceProc = fGetInstanceProcAddr; +@@ -62,24 +71,25 @@ void VulkanWindowContext::initializeContext() { + }; + GrVkBackendContext backendContext; + GrVkExtensions extensions; +- VkPhysicalDeviceFeatures2 features; +- if (!sk_gpu_test::CreateVkBackendContext(getProc, &backendContext, &extensions, &features, +- &fDebugCallback, &fPresentQueueIndex, fCanPresentFn)) { +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); ++ if (!sk_gpu_test::CreateVkBackendContext(getProc, &backendContext, &extensions, &d->features, ++ &d->fDebugCallback, &d->fPresentQueueIndex, fCanPresentFn)) { ++ sk_gpu_test::FreeVulkanFeaturesStructs(&d->features); ++ fGlobalShared.reset(); + return; + } + + if (!extensions.hasExtension(VK_KHR_SURFACE_EXTENSION_NAME, 25) || + !extensions.hasExtension(VK_KHR_SWAPCHAIN_EXTENSION_NAME, 68)) { +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); ++ sk_gpu_test::FreeVulkanFeaturesStructs(&d->features); ++ fGlobalShared.reset(); + return; + } + +- fInstance = backendContext.fInstance; +- fPhysicalDevice = backendContext.fPhysicalDevice; +- fDevice = backendContext.fDevice; +- fGraphicsQueueIndex = backendContext.fGraphicsQueueIndex; +- fGraphicsQueue = backendContext.fQueue; ++ d->fInstance = backendContext.fInstance; ++ d->fPhysicalDevice = backendContext.fPhysicalDevice; ++ d->fDevice = backendContext.fDevice; ++ d->fGraphicsQueueIndex = backendContext.fGraphicsQueueIndex; ++ d->fGraphicsQueue = backendContext.fQueue; + + PFN_vkGetPhysicalDeviceProperties localGetPhysicalDeviceProperties = + reinterpret_cast<PFN_vkGetPhysicalDeviceProperties>( +@@ -87,21 +97,31 @@ void VulkanWindowContext::initializeContext() { + backendContext.fInstance, + VK_NULL_HANDLE)); + if (!localGetPhysicalDeviceProperties) { +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); ++ sk_gpu_test::FreeVulkanFeaturesStructs(&d->features); ++ fGlobalShared.reset(); + return; + } + VkPhysicalDeviceProperties physDeviceProperties; + localGetPhysicalDeviceProperties(backendContext.fPhysicalDevice, &physDeviceProperties); + uint32_t physDevVersion = physDeviceProperties.apiVersion; + +- fInterface.reset(new GrVkInterface(backendContext.fGetProc, fInstance, fDevice, ++ d->fInterface.reset(new GrVkInterface(backendContext.fGetProc, d->fInstance, d->fDevice, + backendContext.fInstanceVersion, physDevVersion, + &extensions)); + +- GET_PROC(DestroyInstance); +- if (fDebugCallback != VK_NULL_HANDLE) { +- GET_PROC(DestroyDebugReportCallbackEXT); ++ d->fContext = GrContext::MakeVulkan(backendContext, fDisplayParams.fGrContextOptions); ++ ++ GET_PROC_GLOBAL(DestroyInstance); ++ GET_DEV_PROC_GLOBAL(DestroyDevice); ++ if (fGlobalShared->fDebugCallback != VK_NULL_HANDLE) { ++ GET_PROC_GLOBAL(DestroyDebugReportCallbackEXT); + } ++ ++ fShared = fGlobalShared; ++ } // if( !fShared ) ++ ++ fContext = fShared->fContext; ++ + GET_PROC(DestroySurfaceKHR); + GET_PROC(GetPhysicalDeviceSurfaceSupportKHR); + GET_PROC(GetPhysicalDeviceSurfaceCapabilitiesKHR); +@@ -109,7 +129,6 @@ void VulkanWindowContext::initializeContext() { + GET_PROC(GetPhysicalDeviceSurfacePresentModesKHR); + GET_DEV_PROC(DeviceWaitIdle); + GET_DEV_PROC(QueueWaitIdle); +- GET_DEV_PROC(DestroyDevice); + GET_DEV_PROC(CreateSwapchainKHR); + GET_DEV_PROC(DestroySwapchainKHR); + GET_DEV_PROC(GetSwapchainImagesKHR); +@@ -117,46 +136,40 @@ void VulkanWindowContext::initializeContext() { + GET_DEV_PROC(QueuePresentKHR); + GET_DEV_PROC(GetDeviceQueue); + +- fContext = GrContext::MakeVulkan(backendContext, fDisplayParams.fGrContextOptions); +- +- fSurface = fCreateVkSurfaceFn(fInstance); ++ fSurface = fCreateVkSurfaceFn(fShared->fInstance); + if (VK_NULL_HANDLE == fSurface) { + this->destroyContext(); +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); + return; + } + + VkBool32 supported; +- VkResult res = fGetPhysicalDeviceSurfaceSupportKHR(fPhysicalDevice, fPresentQueueIndex, ++ VkResult res = fGetPhysicalDeviceSurfaceSupportKHR(fShared->fPhysicalDevice, fShared->fPresentQueueIndex, + fSurface, &supported); + if (VK_SUCCESS != res) { + this->destroyContext(); +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); + return; + } + + if (!this->createSwapchain(-1, -1, fDisplayParams)) { + this->destroyContext(); +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); + return; + } + + // create presentQueue +- fGetDeviceQueue(fDevice, fPresentQueueIndex, 0, &fPresentQueue); +- sk_gpu_test::FreeVulkanFeaturesStructs(&features); ++ fGetDeviceQueue(fShared->fDevice, fShared->fPresentQueueIndex, 0, &fPresentQueue); + } + + bool VulkanWindowContext::createSwapchain(int width, int height, + const DisplayParams& params) { + // check for capabilities + VkSurfaceCapabilitiesKHR caps; +- VkResult res = fGetPhysicalDeviceSurfaceCapabilitiesKHR(fPhysicalDevice, fSurface, &caps); ++ VkResult res = fGetPhysicalDeviceSurfaceCapabilitiesKHR(fShared->fPhysicalDevice, fSurface, &caps); + if (VK_SUCCESS != res) { + return false; + } + + uint32_t surfaceFormatCount; +- res = fGetPhysicalDeviceSurfaceFormatsKHR(fPhysicalDevice, fSurface, &surfaceFormatCount, ++ res = fGetPhysicalDeviceSurfaceFormatsKHR(fShared->fPhysicalDevice, fSurface, &surfaceFormatCount, + nullptr); + if (VK_SUCCESS != res) { + return false; +@@ -164,14 +177,14 @@ bool VulkanWindowContext::createSwapchain(int width, int height, + + SkAutoMalloc surfaceFormatAlloc(surfaceFormatCount * sizeof(VkSurfaceFormatKHR)); + VkSurfaceFormatKHR* surfaceFormats = (VkSurfaceFormatKHR*)surfaceFormatAlloc.get(); +- res = fGetPhysicalDeviceSurfaceFormatsKHR(fPhysicalDevice, fSurface, &surfaceFormatCount, ++ res = fGetPhysicalDeviceSurfaceFormatsKHR(fShared->fPhysicalDevice, fSurface, &surfaceFormatCount, + surfaceFormats); + if (VK_SUCCESS != res) { + return false; + } + + uint32_t presentModeCount; +- res = fGetPhysicalDeviceSurfacePresentModesKHR(fPhysicalDevice, fSurface, &presentModeCount, ++ res = fGetPhysicalDeviceSurfacePresentModesKHR(fShared->fPhysicalDevice, fSurface, &presentModeCount, + nullptr); + if (VK_SUCCESS != res) { + return false; +@@ -179,7 +192,7 @@ bool VulkanWindowContext::createSwapchain(int width, int height, + + SkAutoMalloc presentModeAlloc(presentModeCount * sizeof(VkPresentModeKHR)); + VkPresentModeKHR* presentModes = (VkPresentModeKHR*)presentModeAlloc.get(); +- res = fGetPhysicalDeviceSurfacePresentModesKHR(fPhysicalDevice, fSurface, &presentModeCount, ++ res = fGetPhysicalDeviceSurfacePresentModesKHR(fShared->fPhysicalDevice, fSurface, &presentModeCount, + presentModes); + if (VK_SUCCESS != res) { + return false; +@@ -286,8 +299,8 @@ bool VulkanWindowContext::createSwapchain(int width, int height, + swapchainCreateInfo.imageArrayLayers = 1; + swapchainCreateInfo.imageUsage = usageFlags; + +- uint32_t queueFamilies[] = { fGraphicsQueueIndex, fPresentQueueIndex }; +- if (fGraphicsQueueIndex != fPresentQueueIndex) { ++ uint32_t queueFamilies[] = { fShared->fGraphicsQueueIndex, fShared->fPresentQueueIndex }; ++ if (fShared->fGraphicsQueueIndex != fShared->fPresentQueueIndex) { + swapchainCreateInfo.imageSharingMode = VK_SHARING_MODE_CONCURRENT; + swapchainCreateInfo.queueFamilyIndexCount = 2; + swapchainCreateInfo.pQueueFamilyIndices = queueFamilies; +@@ -303,18 +316,18 @@ bool VulkanWindowContext::createSwapchain(int width, int height, + swapchainCreateInfo.clipped = true; + swapchainCreateInfo.oldSwapchain = fSwapchain; + +- res = fCreateSwapchainKHR(fDevice, &swapchainCreateInfo, nullptr, &fSwapchain); ++ res = fCreateSwapchainKHR(fShared->fDevice, &swapchainCreateInfo, nullptr, &fSwapchain); + if (VK_SUCCESS != res) { + return false; + } + + // destroy the old swapchain + if (swapchainCreateInfo.oldSwapchain != VK_NULL_HANDLE) { +- fDeviceWaitIdle(fDevice); ++ fDeviceWaitIdle(fShared->fDevice); + + this->destroyBuffers(); + +- fDestroySwapchainKHR(fDevice, swapchainCreateInfo.oldSwapchain, nullptr); ++ fDestroySwapchainKHR(fShared->fDevice, swapchainCreateInfo.oldSwapchain, nullptr); + } + + this->createBuffers(swapchainCreateInfo.imageFormat, colorType); +@@ -323,10 +336,10 @@ bool VulkanWindowContext::createSwapchain(int width, int height, + } + + void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType) { +- fGetSwapchainImagesKHR(fDevice, fSwapchain, &fImageCount, nullptr); ++ fGetSwapchainImagesKHR(fShared->fDevice, fSwapchain, &fImageCount, nullptr); + SkASSERT(fImageCount); + fImages = new VkImage[fImageCount]; +- fGetSwapchainImagesKHR(fDevice, fSwapchain, &fImageCount, fImages); ++ fGetSwapchainImagesKHR(fShared->fDevice, fSwapchain, &fImageCount, fImages); + + // set up initial image layouts and create surfaces + fImageLayouts = new VkImageLayout[fImageCount]; +@@ -341,7 +354,7 @@ void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType) + info.fImageTiling = VK_IMAGE_TILING_OPTIMAL; + info.fFormat = format; + info.fLevelCount = 1; +- info.fCurrentQueueFamily = fPresentQueueIndex; ++ info.fCurrentQueueFamily = fShared->fPresentQueueIndex; + + if (fSampleCount == 1) { + GrBackendRenderTarget backendRT(fWidth, fHeight, fSampleCount, info); +@@ -372,8 +385,8 @@ void VulkanWindowContext::createBuffers(VkFormat format, SkColorType colorType) + fBackbuffers = new BackbufferInfo[fImageCount + 1]; + for (uint32_t i = 0; i < fImageCount + 1; ++i) { + fBackbuffers[i].fImageIndex = -1; +- GR_VK_CALL_ERRCHECK(fInterface, +- CreateSemaphore(fDevice, &semaphoreInfo, ++ GR_VK_CALL_ERRCHECK(fShared->fInterface, ++ CreateSemaphore(fShared->fDevice, &semaphoreInfo, + nullptr, &fBackbuffers[i].fRenderSemaphore)); + } + fCurrentBackbufferIndex = fImageCount; +@@ -384,8 +397,8 @@ void VulkanWindowContext::destroyBuffers() { + if (fBackbuffers) { + for (uint32_t i = 0; i < fImageCount + 1; ++i) { + fBackbuffers[i].fImageIndex = -1; +- GR_VK_CALL(fInterface, +- DestroySemaphore(fDevice, ++ GR_VK_CALL(fShared->fInterface, ++ DestroySemaphore(fShared->fDevice, + fBackbuffers[i].fRenderSemaphore, + nullptr)); + } +@@ -410,41 +423,55 @@ VulkanWindowContext::~VulkanWindowContext() { + void VulkanWindowContext::destroyContext() { + if (this->isValid()) { + fQueueWaitIdle(fPresentQueue); +- fDeviceWaitIdle(fDevice); ++ fDeviceWaitIdle(fShared->fDevice); + + this->destroyBuffers(); + + if (VK_NULL_HANDLE != fSwapchain) { +- fDestroySwapchainKHR(fDevice, fSwapchain, nullptr); ++ fDestroySwapchainKHR(fShared->fDevice, fSwapchain, nullptr); + fSwapchain = VK_NULL_HANDLE; + } + + if (VK_NULL_HANDLE != fSurface) { +- fDestroySurfaceKHR(fInstance, fSurface, nullptr); ++ fDestroySurfaceKHR(fShared->fInstance, fSurface, nullptr); + fSurface = VK_NULL_HANDLE; + } + } + + fContext.reset(); +- fInterface.reset(); ++ fShared.reset(); ++ ++ checkDestroyShared(); ++} + +- if (VK_NULL_HANDLE != fDevice) { +- fDestroyDevice(fDevice, nullptr); +- fDevice = VK_NULL_HANDLE; ++void VulkanWindowContext::checkDestroyShared() ++{ ++ if(!fGlobalShared || !fGlobalShared->unique()) // TODO mutex? ++ return; ++ SkASSERT(fGlobalShared->fContext->unique()); ++ fGlobalShared->fContext.reset(); ++ fGlobalShared->fInterface.reset(); ++ ++ if (VK_NULL_HANDLE != fGlobalShared->fDevice) { ++ fGlobalShared->fDestroyDevice(fGlobalShared->fDevice, nullptr); ++ fGlobalShared->fDevice = VK_NULL_HANDLE; + } + + #ifdef SK_ENABLE_VK_LAYERS +- if (fDebugCallback != VK_NULL_HANDLE) { +- fDestroyDebugReportCallbackEXT(fInstance, fDebugCallback, nullptr); ++ if (fGlobalShared->fDebugCallback != VK_NULL_HANDLE) { ++ fGlobalShared->fDestroyDebugReportCallbackEXT(fGlobalShared->fInstance, fDebugCallback, nullptr); + } + #endif + +- fPhysicalDevice = VK_NULL_HANDLE; ++ fGlobalShared->fPhysicalDevice = VK_NULL_HANDLE; + +- if (VK_NULL_HANDLE != fInstance) { +- fDestroyInstance(fInstance, nullptr); +- fInstance = VK_NULL_HANDLE; ++ if (VK_NULL_HANDLE != fGlobalShared->fInstance) { ++ fGlobalShared->fDestroyInstance(fGlobalShared->fInstance, nullptr); ++ fGlobalShared->fInstance = VK_NULL_HANDLE; + } ++ ++ sk_gpu_test::FreeVulkanFeaturesStructs(&fGlobalShared->features); ++ fGlobalShared.reset(); + } + + VulkanWindowContext::BackbufferInfo* VulkanWindowContext::getAvailableBackbuffer() { +@@ -470,34 +497,34 @@ sk_sp<SkSurface> VulkanWindowContext::getBackbufferSurface() { + semaphoreInfo.pNext = nullptr; + semaphoreInfo.flags = 0; + VkSemaphore semaphore; +- GR_VK_CALL_ERRCHECK(fInterface, CreateSemaphore(fDevice, &semaphoreInfo, ++ GR_VK_CALL_ERRCHECK(fShared->fInterface, CreateSemaphore(fShared->fDevice, &semaphoreInfo, + nullptr, &semaphore)); + + // acquire the image +- VkResult res = fAcquireNextImageKHR(fDevice, fSwapchain, UINT64_MAX, ++ VkResult res = fAcquireNextImageKHR(fShared->fDevice, fSwapchain, UINT64_MAX, + semaphore, VK_NULL_HANDLE, + &backbuffer->fImageIndex); + if (VK_ERROR_SURFACE_LOST_KHR == res) { + // need to figure out how to create a new vkSurface without the platformData* + // maybe use attach somehow? but need a Window +- GR_VK_CALL(fInterface, DestroySemaphore(fDevice, semaphore, nullptr)); ++ GR_VK_CALL(fShared->fInterface, DestroySemaphore(fShared->fDevice, semaphore, nullptr)); + return nullptr; + } + if (VK_ERROR_OUT_OF_DATE_KHR == res) { + // tear swapchain down and try again + if (!this->createSwapchain(-1, -1, fDisplayParams)) { +- GR_VK_CALL(fInterface, DestroySemaphore(fDevice, semaphore, nullptr)); ++ GR_VK_CALL(fShared->fInterface, DestroySemaphore(fShared->fDevice, semaphore, nullptr)); + return nullptr; + } + backbuffer = this->getAvailableBackbuffer(); + + // acquire the image +- res = fAcquireNextImageKHR(fDevice, fSwapchain, UINT64_MAX, ++ res = fAcquireNextImageKHR(fShared->fDevice, fSwapchain, UINT64_MAX, + semaphore, VK_NULL_HANDLE, + &backbuffer->fImageIndex); + + if (VK_SUCCESS != res) { +- GR_VK_CALL(fInterface, DestroySemaphore(fDevice, semaphore, nullptr)); ++ GR_VK_CALL(fShared->fInterface, DestroySemaphore(fShared->fDevice, semaphore, nullptr)); + return nullptr; + } + } +@@ -541,4 +568,6 @@ void VulkanWindowContext::swapBuffers() { + fQueuePresentKHR(fPresentQueue, &presentInfo); + } + ++SK_API sk_sp<VulkanWindowContext::Shared> VulkanWindowContext::fGlobalShared; ++ + } //namespace sk_app +diff --git a/tools/sk_app/VulkanWindowContext.h b/tools/sk_app/VulkanWindowContext.h +index 2db9e79ae6..7950dc159b 100644 +--- a/tools/sk_app/VulkanWindowContext.h ++++ b/tools/sk_app/VulkanWindowContext.h +@@ -1,4 +1,3 @@ +- + /* + * Copyright 2016 Google Inc. + * +@@ -23,14 +22,30 @@ class GrRenderTarget; + + namespace sk_app { + +-class VulkanWindowContext : public WindowContext { ++class SK_API VulkanWindowContext : public WindowContext { ++ struct Shared; + public: + ~VulkanWindowContext() override; + ++ class SharedGrContext { ++ public: ++ SharedGrContext() {} ++ GrContext* getGrContext() { return shared ? shared->fContext.get() : nullptr; } ++ ~SharedGrContext() { shared.reset(); checkDestroyShared(); } ++ bool operator!() const { return !shared; } ++ void reset() { shared.reset(); } ++ private: ++ friend class VulkanWindowContext; ++ SharedGrContext(sk_sp<Shared>& sh ) : shared( sh ) {} ++ sk_sp<Shared> shared; ++ }; ++ ++ static SharedGrContext getSharedGrContext() { return SharedGrContext( fGlobalShared ); } ++ + sk_sp<SkSurface> getBackbufferSurface() override; + void swapBuffers() override; + +- bool isValid() override { return fDevice != VK_NULL_HANDLE; } ++ bool isValid() override { return fShared->fDevice != VK_NULL_HANDLE; } + + void resize(int w, int h) override { + this->createSwapchain(w, h, fDisplayParams); +@@ -53,6 +68,7 @@ public: + private: + void initializeContext(); + void destroyContext(); ++ static void checkDestroyShared(); + + struct BackbufferInfo { + uint32_t fImageIndex; // image this is associated with +@@ -64,11 +80,6 @@ private: + void createBuffers(VkFormat format, SkColorType colorType); + void destroyBuffers(); + +- VkInstance fInstance = VK_NULL_HANDLE; +- VkPhysicalDevice fPhysicalDevice = VK_NULL_HANDLE; +- VkDevice fDevice = VK_NULL_HANDLE; +- VkDebugReportCallbackEXT fDebugCallback = VK_NULL_HANDLE; +- + // Create functions + CreateVkSurfaceFn fCreateVkSurfaceFn; + CanPresentFn fCanPresentFn; +@@ -90,20 +101,41 @@ private: + PFN_vkAcquireNextImageKHR fAcquireNextImageKHR = nullptr; + PFN_vkQueuePresentKHR fQueuePresentKHR = nullptr; + +- PFN_vkDestroyInstance fDestroyInstance = nullptr; + PFN_vkDeviceWaitIdle fDeviceWaitIdle = nullptr; +- PFN_vkDestroyDebugReportCallbackEXT fDestroyDebugReportCallbackEXT = nullptr; + PFN_vkQueueWaitIdle fQueueWaitIdle = nullptr; +- PFN_vkDestroyDevice fDestroyDevice = nullptr; + PFN_vkGetDeviceQueue fGetDeviceQueue = nullptr; + ++ // We need to use just one GrContext, so share all the relevant data. ++ struct Shared : public SkRefCnt ++ { ++ PFN_vkDestroyInstance fDestroyInstance = nullptr; ++ PFN_vkDestroyDevice fDestroyDevice = nullptr; ++ PFN_vkDestroyDebugReportCallbackEXT fDestroyDebugReportCallbackEXT = nullptr; ++ ++ VkInstance fInstance = VK_NULL_HANDLE; ++ VkPhysicalDevice fPhysicalDevice = VK_NULL_HANDLE; ++ VkDevice fDevice = VK_NULL_HANDLE; ++ VkDebugReportCallbackEXT fDebugCallback = VK_NULL_HANDLE; ++ + sk_sp<const GrVkInterface> fInterface; + +- VkSurfaceKHR fSurface; +- VkSwapchainKHR fSwapchain; ++ // Original code had this as a function-local variable, but that seems wrong. ++ // It should exist as long as the context exists. ++ VkPhysicalDeviceFeatures2 features; ++ + uint32_t fGraphicsQueueIndex; + VkQueue fGraphicsQueue; + uint32_t fPresentQueueIndex; ++ ++ sk_sp<GrContext> fContext; ++ }; ++ ++ sk_sp<Shared> fShared; ++ ++ static sk_sp<Shared> fGlobalShared; ++ ++ VkSurfaceKHR fSurface; ++ VkSwapchainKHR fSwapchain; + VkQueue fPresentQueue; + + uint32_t fImageCount; diff --git a/vcl/Library_vcl.mk b/vcl/Library_vcl.mk index 0bb9f8f64198..c33c4e90db43 100644 --- a/vcl/Library_vcl.mk +++ b/vcl/Library_vcl.mk @@ -579,8 +579,7 @@ $(eval $(call gb_Library_add_exception_objects,vcl,\ vcl/skia/SkiaHelper \ $(if $(filter SKIA,$(BUILD_TYPE)), \ vcl/skia/salbmp \ - vcl/skia/gdiimpl \ - vcl/skia/vulkan) \ + vcl/skia/gdiimpl) \ )) # runtime dependency diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index 04f96cb7bd11..195b5d877eed 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -26,6 +26,7 @@ #include <salgeom.hxx> #include <SkSurface.h> +#include <tools/sk_app/VulkanWindowContext.h> class SkiaFlushIdle; @@ -210,6 +211,7 @@ protected: void destroySurface(); // Reimplemented for X11. virtual bool avoidRecreateByResize() const { return false; } + void createOffscreenSurface(); void privateDrawAlphaRect(long nX, long nY, long nWidth, long nHeight, double nTransparency, bool blockAA = false); @@ -265,6 +267,8 @@ protected: // The Skia surface that is target of all the rendering. sk_sp<SkSurface> mSurface; bool mIsGPU; // whether the surface is GPU-backed + // Keep reference to shared GrContext. + sk_app::VulkanWindowContext::SharedGrContext mOffscreenGrContext; vcl::Region mClipRegion; Color mLineColor; Color mFillColor; diff --git a/vcl/inc/skia/vulkan.hxx b/vcl/inc/skia/vulkan.hxx deleted file mode 100644 index 6c8e3c7bfa44..000000000000 --- a/vcl/inc/skia/vulkan.hxx +++ /dev/null @@ -1,29 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* - * This file is part of the LibreOffice project. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - */ - -#ifndef INCLUDED_VCL_INC_SKIA_VULKAN_HXX -#define INCLUDED_VCL_INC_SKIA_VULKAN_HXX - -#include <GrContext.h> - -#include <vcl/dllapi.h> - -// Create and handle GrContext for Vulkan drawing to offscreen surfaces. -// Skia already provides WindowContext class that does this for surfaces -// used for drawing to windows, but it does not seem to provide a simple -// way to get GrContext without a window. -class VCL_PLUGIN_PUBLIC SkiaVulkanGrContext -{ -public: - static GrContext* getGrContext(); -}; - -#endif // INCLUDED_VCL_INC_SKIA_VULKAN_HXX - -/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/skia/README b/vcl/skia/README index 793c16e8dd39..f1248d90c3ea 100644 --- a/vcl/skia/README +++ b/vcl/skia/README @@ -17,3 +17,19 @@ Skia supports several methods to draw: - Vulkan - Vulkan-based GPU drawing, this is the default There are more (OpenGL, Metal on Mac, etc.), but (as of now) they are not supported by VCL. + +GrContext sharing: +================== + +We use Skia's sk_app::WindowContext class for creating surfaces for windows, that class +takes care of the internals. But of offscreen drawing, we need an instance of class +GrContext. There is sk_app::WindowContext::getGrContext(), but each instance creates +its own GrContext, and apparently it does not work to mix them. Which means that +for offscreen drawing we would need to know which window (and only that window) +the contents will be eventually painted to, which is not possible (it may not even +be known at the time). + +To solve this problem we patch sk_app::WindowContext to create just one GrContext object +and share it between instances. Additionally, using sk_app::WindowContext::SharedGrContext +it is possible to share it also for offscreen drawing, including keeping proper reference +count. diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index 776e44216c45..ac47d254cc03 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -23,12 +23,14 @@ #include <skia/salbmp.hxx> #include <vcl/idle.hxx> #include <vcl/svapp.hxx> +#include <vcl/lazydelete.hxx> #include <SkCanvas.h> #include <SkPath.h> #include <SkRegion.h> #include <SkDashPathEffect.h> #include <GrBackendSurface.h> +#include <GrContextFactory.h> #include <basegfx/polygon/b2dpolygontools.hxx> @@ -190,7 +192,11 @@ SkiaSalGraphicsImpl::SkiaSalGraphicsImpl(SalGraphics& rParent, SalGeometryProvid { } -SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl() {} +SkiaSalGraphicsImpl::~SkiaSalGraphicsImpl() +{ + assert(!mSurface); + assert(!mOffscreenGrContext); +} void SkiaSalGraphicsImpl::Init() {} @@ -208,8 +214,7 @@ void SkiaSalGraphicsImpl::recreateSurface() void SkiaSalGraphicsImpl::createSurface() { - // Create surface for offscreen graphics. Subclasses will create GPU-backed - // surfaces as appropriate. + // Create raster surface. Subclasses will create GPU-backed surfaces as appropriate. mSurface = SkSurface::MakeRasterN32Premul(GetWidth(), GetHeight()); mIsGPU = false; #ifdef DBG_UTIL @@ -217,6 +222,53 @@ void SkiaSalGraphicsImpl::createSurface() #endif } +void SkiaSalGraphicsImpl::createOffscreenSurface() +{ + assert(isOffscreen()); + destroySurface(); + switch (renderMethodToUse()) + { + case RenderVulkan: + { + mOffscreenGrContext = sk_app::VulkanWindowContext::getSharedGrContext(); + GrContext* grContext = mOffscreenGrContext.getGrContext(); + // We may not get a GrContext if called before any onscreen window is created, + // but that happens very early, so this should be rare and insignificant. + // Unittests are an exception, they usually do not create any windows, + // so in that case do create GrContext that has no window associated. + if (!grContext) + { + static bool isUnitTest = (getenv("LO_TESTNAME") != nullptr); + if (isUnitTest) + { + static vcl::DeleteOnDeinit<sk_gpu_test::GrContextFactory> factory( + new sk_gpu_test::GrContextFactory); + // The factory owns the context. + grContext + = factory.get()->get(sk_gpu_test::GrContextFactory::kVulkan_ContextType); + } + } + if (grContext) + { + mSurface = SkSurface::MakeRenderTarget( + grContext, SkBudgeted::kNo, + SkImageInfo::MakeN32Premul(GetWidth(), GetHeight())); + mIsGPU = true; + assert(mSurface.get()); +#ifdef DBG_UTIL + prefillSurface(); +#endif + return; + } + SAL_WARN("vcl.skia", "cannot create Vulkan GPU offscreen surface"); + break; + } + default: + break; + } + return SkiaSalGraphicsImpl::createSurface(); // create a raster one +} + void SkiaSalGraphicsImpl::destroySurface() { if (mSurface) @@ -237,6 +289,7 @@ void SkiaSalGraphicsImpl::destroySurface() mSurface->flush(); mSurface.reset(); mIsGPU = false; + mOffscreenGrContext.reset(); } void SkiaSalGraphicsImpl::DeInit() { destroySurface(); } diff --git a/vcl/skia/vulkan.cxx b/vcl/skia/vulkan.cxx deleted file mode 100644 index c5c9093739fc..000000000000 --- a/vcl/skia/vulkan.cxx +++ /dev/null @@ -1,39 +0,0 @@ -/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ -/* - * This file is part of the LibreOffice project. - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. - * - * Some of this code is based on Skia source code, covered by the following - * license notice (see readlicense_oo for the full license): - * - * Copyright 2016 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - * - */ - -#include <skia/vulkan.hxx> - -#include <GrContextFactory.h> - -#include <vcl/lazydelete.hxx> - -static GrContext* createGrContext() -{ - static vcl::DeleteOnDeinit<sk_gpu_test::GrContextFactory> factory( - new sk_gpu_test::GrContextFactory); - // The factory owns the context. - return factory.get()->get(sk_gpu_test::GrContextFactory::kVulkan_ContextType); -} - -GrContext* SkiaVulkanGrContext::getGrContext() -{ - static GrContext* context = createGrContext(); - return context; -} - -/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/skia/win/gdiimpl.cxx b/vcl/skia/win/gdiimpl.cxx index e9db555c25c0..8f30c3486dac 100644 --- a/vcl/skia/win/gdiimpl.cxx +++ b/vcl/skia/win/gdiimpl.cxx @@ -12,7 +12,6 @@ #include <tools/sk_app/win/WindowContextFactory_win.h> #include <tools/sk_app/WindowContext.h> #include <win/saldata.hxx> -#include <skia/vulkan.hxx> #include <SkColorFilter.h> #include <SkPixelRef.h> @@ -45,26 +44,9 @@ void WinSkiaSalGraphicsImpl::Init() void WinSkiaSalGraphicsImpl::createSurface() { - destroySurface(); if (isOffscreen()) - { - switch (renderMethodToUse()) - { - case RenderVulkan: - mSurface = SkSurface::MakeRenderTarget( - SkiaVulkanGrContext::getGrContext(), SkBudgeted::kNo, - SkImageInfo::MakeN32Premul(GetWidth(), GetHeight())); - mIsGPU = true; - assert(mSurface.get()); -#ifdef DBG_UTIL - prefillSurface(); -#endif - return; - default: - break; - } - return SkiaSalGraphicsImpl::createSurface(); - } + return createOffscreenSurface(); + destroySurface(); // When created, Init() gets called with size (0,0), which is invalid size // for Skia. Creating the actual surface is delayed, so the size should be always // valid here, but better check. diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx index e349074e70e1..ef381c5e56d3 100644 --- a/vcl/skia/x11/gdiimpl.cxx +++ b/vcl/skia/x11/gdiimpl.cxx @@ -21,8 +21,6 @@ #include <tools/sk_app/unix/WindowContextFactory_unix.h> #include <tools/sk_app/WindowContext.h> -#include <skia/vulkan.hxx> - X11SkiaSalGraphicsImpl::X11SkiaSalGraphicsImpl(X11SalGraphics& rParent) : SkiaSalGraphicsImpl(rParent, rParent.GetGeometryProvider()) , mX11Parent(rParent) @@ -40,29 +38,12 @@ void X11SkiaSalGraphicsImpl::Init() void X11SkiaSalGraphicsImpl::createSurface() { - destroySurface(); if (isOffscreen()) - { - switch (renderMethodToUse()) - { - case RenderVulkan: - mSurface = SkSurface::MakeRenderTarget( - SkiaVulkanGrContext::getGrContext(), SkBudgeted::kNo, - SkImageInfo::MakeN32Premul(GetWidth(), GetHeight())); - mIsGPU = true; - assert(mSurface.get()); -#ifdef DBG_UTIL - prefillSurface(); -#endif - return; - default: - break; - } - return SkiaSalGraphicsImpl::createSurface(); - } + return createOffscreenSurface(); + destroySurface(); sk_app::DisplayParams displayParams; - // TODO The Skia Xlib code actually requires the non-native color type to work properly. // Use a macro to hide an unreachable code warning. + // TODO The Skia Xlib code actually requires the non-native color type to work properly. #define GET_FORMAT \ kN32_SkColorType == kBGRA_8888_SkColorType ? kRGBA_8888_SkColorType : kBGRA_8888_SkColorType displayParams.fColorType = GET_FORMAT; @@ -73,6 +54,13 @@ void X11SkiaSalGraphicsImpl::createSurface() assert(winInfo.fDisplay && winInfo.fWindow != None); winInfo.fFBConfig = nullptr; // not used winInfo.fVisualInfo = const_cast<SalVisual*>(&mX11Parent.GetVisual()); +#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; +#endif winInfo.fWidth = GetWidth(); winInfo.fHeight = GetHeight(); switch (renderMethodToUse()) commit 5b5f7f33536dc4690b031dbe09b80092a254252e Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Tue Nov 12 15:50:00 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:56 2019 +0100 clean up properly in VCL backend tests VclPtr is a smart pointer, but it does not clean up automatically, ScopedVclPtr does. Change-Id: If792111cdd489b1743a1bcf060b56c52a4aa79d5 diff --git a/vcl/inc/test/outputdevice.hxx b/vcl/inc/test/outputdevice.hxx index 520436f66fea..f2f4784d086c 100644 --- a/vcl/inc/test/outputdevice.hxx +++ b/vcl/inc/test/outputdevice.hxx @@ -34,7 +34,7 @@ class VCL_DLLPUBLIC OutputDeviceTestCommon { protected: - VclPtr<VirtualDevice> mpVirtualDevice; + ScopedVclPtr<VirtualDevice> mpVirtualDevice; tools::Rectangle maVDRectangle; static const Color constBackgroundColor; commit 6041dbabab8d0b98dbff275023d4f7332e33b7c7 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Tue Nov 12 10:57:30 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:55 2019 +0100 avoid repeated SkSurface recreating because of X11 being asynchronous Sometimes VCL and X11 (and thus Skia) will have a different idea about what the size of a window is. Check for the mismatch and avoid recreating if it wouldn't do anything. Change-Id: Icf3ebba9589cc6f12612e5f280840346cb0edaeb diff --git a/vcl/inc/skia/gdiimpl.hxx b/vcl/inc/skia/gdiimpl.hxx index 8225e76d27ff..04f96cb7bd11 100644 --- a/vcl/inc/skia/gdiimpl.hxx +++ b/vcl/inc/skia/gdiimpl.hxx @@ -208,6 +208,8 @@ protected: void checkSurface(); void recreateSurface(); void destroySurface(); + // Reimplemented for X11. + virtual bool avoidRecreateByResize() const { return false; } void privateDrawAlphaRect(long nX, long nY, long nWidth, long nHeight, double nTransparency, bool blockAA = false); diff --git a/vcl/inc/skia/x11/gdiimpl.hxx b/vcl/inc/skia/x11/gdiimpl.hxx index 1c40aa6b2c28..1dc5064e6667 100644 --- a/vcl/inc/skia/x11/gdiimpl.hxx +++ b/vcl/inc/skia/x11/gdiimpl.hxx @@ -37,6 +37,7 @@ public: protected: virtual void createSurface() override; virtual void performFlush() override; + virtual bool avoidRecreateByResize() const override; private: std::unique_ptr<sk_app::WindowContext> mWindowContext; diff --git a/vcl/skia/gdiimpl.cxx b/vcl/skia/gdiimpl.cxx index 6215317b18ee..776e44216c45 100644 --- a/vcl/skia/gdiimpl.cxx +++ b/vcl/skia/gdiimpl.cxx @@ -267,11 +267,14 @@ void SkiaSalGraphicsImpl::checkSurface() } else if (GetWidth() != mSurface->width() || GetHeight() != mSurface->height()) { - Size oldSize(mSurface->width(), mSurface->height()); - recreateSurface(); - SAL_INFO("vcl.skia", "recreate(" << this << "): old " << oldSize << " new " - << Size(mSurface->width(), mSurface->height()) - << " requested " << Size(GetWidth(), GetHeight())); + if (!avoidRecreateByResize()) + { + Size oldSize(mSurface->width(), mSurface->height()); + recreateSurface(); + SAL_INFO("vcl.skia", "recreate(" << this << "): old " << oldSize << " new " + << Size(mSurface->width(), mSurface->height()) + << " requested " << Size(GetWidth(), GetHeight())); + } } } diff --git a/vcl/skia/x11/gdiimpl.cxx b/vcl/skia/x11/gdiimpl.cxx index 72fc311f7aaa..e349074e70e1 100644 --- a/vcl/skia/x11/gdiimpl.cxx +++ b/vcl/skia/x11/gdiimpl.cxx @@ -73,10 +73,6 @@ void X11SkiaSalGraphicsImpl::createSurface() assert(winInfo.fDisplay && winInfo.fWindow != None); winInfo.fFBConfig = nullptr; // not used winInfo.fVisualInfo = const_cast<SalVisual*>(&mX11Parent.GetVisual()); - // TODO Vulkan does not use these dimensions, instead it uses dimensions of the actual - // drawable, which may lead to repeated createSurface() calls from checkSurface() - // if the window is being resized and VCL already knows the new size but Vulkan doesn't. - // Avoid this somehow. winInfo.fWidth = GetWidth(); winInfo.fHeight = GetHeight(); switch (renderMethodToUse()) @@ -100,6 +96,24 @@ void X11SkiaSalGraphicsImpl::createSurface() #endif } +bool X11SkiaSalGraphicsImpl::avoidRecreateByResize() const +{ + if (!mSurface) + return false; + // Skia's WindowContext uses actual dimensions of the X window, which due to X11 being + // asynchronous may be temporarily different from what VCL thinks are the dimensions. + // That can lead to us repeatedly calling recreateSurface() because of "incorrect" + // size, and we otherwise need to check for size changes, because VCL does not inform us. + // Avoid the problem here by checking the size of the X window and bail out if Skia + // would just return the same size as it is now. + Window r; + int x, y; + unsigned int w, h, border, depth; + XGetGeometry(mX11Parent.GetXDisplay(), mX11Parent.GetDrawable(), &r, &x, &y, &w, &h, &border, + &depth); + return mSurface->width() == int(w) && mSurface->height() == int(h); +} + void X11SkiaSalGraphicsImpl::DeInit() { SkiaSalGraphicsImpl::DeInit(); commit 18d92b83fbf8927e70f0962218009a7ba8748f79 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Fri Nov 1 13:44:20 2019 +0100 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Mon Nov 18 12:47:54 2019 +0100 use different line and fill color in vcl backendtest Having them the same can hide problems with them fixed up incorrectly. And it also shows that drawPolygon() with line color unset does not draw the right-most and bottom-most line, which is what all underlying graphics systems do, so the test is kind of wrong and I've added a compensation to make it visually correct (and match the checked expected result). Change-Id: I333f41210232c74ba55bd5c92ef5fda917ce3e59 diff --git a/vcl/backendtest/VisualBackendTest.cxx b/vcl/backendtest/VisualBackendTest.cxx index c501eb09879f..a1134df5f0da 100644 --- a/vcl/backendtest/VisualBackendTest.cxx +++ b/vcl/backendtest/VisualBackendTest.cxx @@ -88,7 +88,7 @@ class VisualBackendTestWindow : public WorkWindow private: Timer maUpdateTimer; std::vector<std::chrono::high_resolution_clock::time_point> mTimePoints; ... etc. - the rest is truncated _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits