vcl/inc/skia/utils.hxx | 2 vcl/skia/SkiaHelper.cxx | 210 +++++++++++++++++------------------------------- 2 files changed, 77 insertions(+), 135 deletions(-)
New commits: commit 2fe8349ebcd033cd9810b4124638eef652e01f63 Author: Mike Kaganski <[email protected]> AuthorDate: Wed May 14 15:45:57 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Wed May 14 19:06:24 2025 +0200 Simplify SkiaHelper code Reorder functions to avoid some forward declarations; group some env variable checks in a single function to make their interaction clear; use function-local statics for safe initialization; use std::atomic for safety. Also drops a check of SAL_FORCEGL - it's unused now. Change-Id: I45a3bdc076b881bda2366f9d57e41cb7b5a177b9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185304 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/vcl/inc/skia/utils.hxx b/vcl/inc/skia/utils.hxx index c1db9814692b..0617de466744 100644 --- a/vcl/inc/skia/utils.hxx +++ b/vcl/inc/skia/utils.hxx @@ -51,8 +51,6 @@ namespace SkiaHelper // Get the one shared GrDirectContext instance. GrDirectContext* getSharedGrDirectContext(); -void disableRenderMethod(RenderMethod method); - // Create SkSurface, GPU-backed if possible. VCL_DLLPUBLIC sk_sp<SkSurface> createSkSurface(int width, int height, SkColorType type = kN32_SkColorType, diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx index 45cc25451eed..53de25ffcf86 100644 --- a/vcl/skia/SkiaHelper.cxx +++ b/vcl/skia/SkiaHelper.cxx @@ -39,6 +39,7 @@ bool isAlphaMaskBlendingEnabled() { return false; } #include <config_skia.h> #include <osl/file.hxx> #include <tools/stream.hxx> +#include <atomic> #include <list> #include <o3tl/lru_map.hxx> @@ -241,7 +242,47 @@ static void writeSkiaRasterInfo() static std::unique_ptr<skwindow::WindowContext> getTemporaryWindowContext(); #endif -static void checkDeviceDenylisted(bool blockDisable = false) +static RenderMethod initRenderMethodToUse() +{ + if (Application::IsBitmapRendering()) + return RenderRaster; + + if (const char* env = getenv("SAL_SKIA")) + { + if (strcmp(env, "raster") == 0) + return RenderRaster; +#if defined SK_METAL + if (strcmp(env, "metal") == 0) + return RenderMetal; +#elif defined SK_VULKAN + if (strcmp(env, "vulkan") == 0) + return RenderVulkan; +#endif + SAL_WARN("vcl.skia", "Unrecognized value of SAL_SKIA"); + abort(); + } + if (officecfg::Office::Common::VCL::ForceSkiaRaster::get()) + return RenderRaster; +#if defined SK_METAL + return RenderMetal; +#elif defined SK_VULKAN + return RenderVulkan; +#else + return RenderRaster; +#endif +} + +static std::atomic<RenderMethod>& accessRenderMethodToUse() +{ + static std::atomic<RenderMethod> methodToUse = initRenderMethodToUse(); + return methodToUse; +} + +RenderMethod renderMethodToUse() { return accessRenderMethodToUse(); } + +static void forceRasterRenderMethod() { accessRenderMethodToUse() = RenderRaster; } + +static void checkDeviceDenylisted(bool blockDisable) { static bool done = false; if (done) @@ -289,7 +330,7 @@ static void checkDeviceDenylisted(bool blockDisable = false) SAL_INFO("vcl.skia", "Vulkan could not be initialized"); if (denylisted && !blockDisable) { - disableRenderMethod(RenderVulkan); + forceRasterRenderMethod(); useRaster = true; } @@ -321,7 +362,7 @@ static void checkDeviceDenylisted(bool blockDisable = false) if (!blockDisable && !DefaultMTLDeviceIsSupported()) { SAL_INFO("vcl.skia", "Metal default device not supported"); - disableRenderMethod(RenderMetal); + forceRasterRenderMethod(); useRaster = true; } else @@ -334,7 +375,7 @@ static void checkDeviceDenylisted(bool blockDisable = false) else { SAL_INFO("vcl.skia", "Metal could not be initialized"); - disableRenderMethod(RenderMetal); + forceRasterRenderMethod(); useRaster = true; } #else @@ -356,158 +397,71 @@ static void checkDeviceDenylisted(bool blockDisable = false) done = true; } -static bool skiaSupportedByBackend = false; +static std::atomic<bool> skiaSupportedByBackend = false; static bool supportsVCLSkia() { - if (!skiaSupportedByBackend) - { - SAL_INFO("vcl.skia", "Skia not supported by VCL backend, disabling"); - return false; - } - return getenv("SAL_DISABLESKIA") == nullptr; + if (skiaSupportedByBackend) + return true; + SAL_INFO("vcl.skia", "Skia not supported by VCL backend, disabling"); + return false; } -static void initInternal(); - -bool isVCLSkiaEnabled() +static bool initVCLSkiaEnabled() { /** - * The !bSet part should only be called once! Changing the results in the same + * Should only be called once! Changing the results in the same * run will mix Skia and normal rendering. */ - static bool bSet = false; - static bool bEnable = false; - static bool bForceSkia = false; - // allow global disable when testing SystemPrimitiveRenderer since current Skia on Win does not // harmonize with using Direct2D and D2DPixelProcessor2D - static const bool bTestSystemPrimitiveRenderer( - nullptr != std::getenv("TEST_SYSTEM_PRIMITIVE_RENDERER")); - if (bTestSystemPrimitiveRenderer) + if (std::getenv("TEST_SYSTEM_PRIMITIVE_RENDERER") != nullptr) return false; - if (bSet) - { - return bForceSkia || bEnable; - } - /* * There are a number of cases that these environment variables cover: * * SAL_FORCESKIA forces Skia if disabled by UI options or denylisted * * SAL_DISABLESKIA avoids the use of Skia regardless of any option */ - bSet = true; - bForceSkia = !!getenv("SAL_FORCESKIA") || officecfg::Office::Common::VCL::ForceSkia::get(); - bool bRet = false; - bool bSupportsVCLSkia = supportsVCLSkia(); - if (bForceSkia && bSupportsVCLSkia) - { - bRet = true; - initInternal(); - // don't actually block if denylisted, but log it if enabled, and also get the vendor id - checkDeviceDenylisted(true); - } - else if (getenv("SAL_FORCEGL")) - { - // Skia usage is checked before GL usage, so if GL is forced (and Skia is not), do not - // enable Skia in order to allow GL. - bRet = false; - } - else if (bSupportsVCLSkia) + if (supportsVCLSkia() && getenv("SAL_DISABLESKIA") == nullptr) { - static bool bEnableSkiaEnv = !!getenv("SAL_ENABLESKIA"); + const bool bForceSkia = getenv("SAL_FORCESKIA") != nullptr + || officecfg::Office::Common::VCL::ForceSkia::get(); - bEnable = bEnableSkiaEnv; - - if (officecfg::Office::Common::VCL::UseSkia::get()) - bEnable = true; - - // Force disable in safe mode - if (Application::IsSafeModeEnabled()) - bEnable = false; - - if (bEnable) + bRet = bForceSkia; + // If not forced, don't enable in safe mode + if (!bRet && !Application::IsSafeModeEnabled()) { - initInternal(); - checkDeviceDenylisted(); // switch to raster if driver is denylisted + bRet = getenv("SAL_ENABLESKIA") != nullptr + || officecfg::Office::Common::VCL::UseSkia::get(); } - bRet = bEnable; + if (bRet) + { + // Set up all things needed for using Skia. + SkGraphics::Init(); + SkLoOpts::Init(); + // if bForceSkia, don't actually block if denylisted, but log it if enabled, + // and also get the vendor id; otherwise, switch to raster if driver is denylisted + checkDeviceDenylisted(bForceSkia); + WatchdogThread::start(); + } } - if (bRet) - WatchdogThread::start(); - CrashReporter::addKeyValue(u"UseSkia"_ustr, OUString::boolean(bRet), CrashReporter::Write); return bRet; } -bool isAlphaMaskBlendingEnabled() { return false; } - -static RenderMethod methodToUse = RenderRaster; - -static bool initRenderMethodToUse() -{ - if (Application::IsBitmapRendering()) - { - methodToUse = RenderRaster; - return true; - } - - if (const char* env = getenv("SAL_SKIA")) - { - if (strcmp(env, "raster") == 0) - { - methodToUse = RenderRaster; - return true; - } -#ifdef MACOSX - if (strcmp(env, "metal") == 0) - { - methodToUse = RenderMetal; - return true; - } -#else - if (strcmp(env, "vulkan") == 0) - { - methodToUse = RenderVulkan; - return true; - } -#endif - SAL_WARN("vcl.skia", "Unrecognized value of SAL_SKIA"); - abort(); - } - methodToUse = RenderRaster; - if (officecfg::Office::Common::VCL::ForceSkiaRaster::get()) - return true; -#ifdef SK_METAL - methodToUse = RenderMetal; -#endif -#ifdef SK_VULKAN - methodToUse = RenderVulkan; -#endif - return true; -} - -RenderMethod renderMethodToUse() +bool isVCLSkiaEnabled() { - static bool methodToUseInited = initRenderMethodToUse(); - if (methodToUseInited) // Used just to ensure thread-safe one-time init. - return methodToUse; - abort(); + static const bool val = initVCLSkiaEnabled(); + return val; } -void disableRenderMethod(RenderMethod method) -{ - if (renderMethodToUse() != method) - return; - // Choose a fallback, right now always raster. - methodToUse = RenderRaster; -} +bool isAlphaMaskBlendingEnabled() { return false; } // If needed, we'll allocate one extra window context so that we have a valid GrDirectContext // from Vulkan/MetalWindowContext. @@ -561,7 +515,7 @@ GrDirectContext* getSharedGrDirectContext() "Cannot create Vulkan GPU context, falling back to Raster"); SAL_WARN_IF(renderMethodToUse() == RenderMetal, "vcl.skia", "Cannot create Metal GPU context, falling back to Raster"); - disableRenderMethod(renderMethodToUse()); + forceRasterRenderMethod(); return nullptr; } @@ -859,13 +813,6 @@ void setBlenderXor(SkPaint* paint) paint->setBlender(xorBlender); } -static void initInternal() -{ - // Set up all things needed for using Skia. - SkGraphics::Init(); - SkLoOpts::Init(); -} - void cleanup() { sharedWindowContext.reset(); commit 56ed516429ec9320b884341fc811b4bf8fbfc322 Author: Mike Kaganski <[email protected]> AuthorDate: Wed May 14 13:37:35 2025 +0500 Commit: Mike Kaganski <[email protected]> CommitDate: Wed May 14 19:06:18 2025 +0200 Drop unused static variable Change-Id: I6885c7e28553132a4e2cb37dfeee20417e6553e6 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185298 Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/vcl/skia/SkiaHelper.cxx b/vcl/skia/SkiaHelper.cxx index fdff5e464380..45cc25451eed 100644 --- a/vcl/skia/SkiaHelper.cxx +++ b/vcl/skia/SkiaHelper.cxx @@ -148,8 +148,6 @@ static OUString getDenylistFile() return url + "/skia/skia_denylist_vulkan.xml"; } -static uint32_t driverVersion = 0; - static OUString versionAsString(uint32_t version) { return OUString::number(version >> 22) + "." + OUString::number((version >> 12) & 0x3ff) + "." @@ -185,11 +183,10 @@ static bool isVulkanDenylisted(const VkPhysicalDeviceProperties& props) { static const char* const types[] = { "other", "integrated", "discrete", "virtual", "cpu", "??" }; // VkPhysicalDeviceType - driverVersion = props.driverVersion; vendorId = props.vendorID; OUString vendorIdStr = "0x" + OUString::number(props.vendorID, 16); OUString deviceIdStr = "0x" + OUString::number(props.deviceID, 16); - OUString driverVersionString = versionAsString(driverVersion); + OUString driverVersionString = versionAsString(props.driverVersion); OUString apiVersion = versionAsString(props.apiVersion); const char* deviceType = types[std::min<unsigned>(props.deviceType, SAL_N_ELEMENTS(types) - 1)];
