From 4056b31465e72e0a9e262639444fc5a3249f548d Mon Sep 17 00:00:00 2001 From: Patrick Luby Date: Wed, 25 Sep 2024 19:32:55 -0400 Subject: tdf#163152 don't convert image's sRGB colorspace With Skia/Raster or Skia disabled, converting the image's colorspace to match the window's colorspace causes more than an expected amount of color saturation so let the window's underlying CGContext handle any necessary colorspace conversion in CGContextDrawImage(). With Skia/Metal, this bug is caused by the CAMetalLayer being set to the same colorspace as its matching window. So set the CAMetalLayer's colorspace to sRGB so that, like with Skia/Raster and Skia disabled, any colorspace conversion is handled when the CAMetalLayer is drawn to the window. Change-Id: Ifa2abe46d34bfcf5acd478fffd346603f869157b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173962 Tested-by: Jenkins Reviewed-by: Patrick Luby (cherry picked from commit e4ab68142c7bc4e04ffe429567dda974b86985a7) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/173923 Reviewed-by: Adolfo Jayme Barrientos --- external/skia/UnpackedTarball_skia.mk | 2 +- external/skia/macosmetal.patch.0 | 120 ++++++++++++++++++++++++++++++++++ external/skia/tdf147342.patch.0 | 110 ------------------------------- 3 files changed, 121 insertions(+), 111 deletions(-) create mode 100644 external/skia/macosmetal.patch.0 delete mode 100644 external/skia/tdf147342.patch.0 (limited to 'external') diff --git a/external/skia/UnpackedTarball_skia.mk b/external/skia/UnpackedTarball_skia.mk index b985d8aa9744..bbc51141cda6 100644 --- a/external/skia/UnpackedTarball_skia.mk +++ b/external/skia/UnpackedTarball_skia.mk @@ -33,7 +33,7 @@ skia_patches := \ windows-libraries-system32.patch.1 \ allow-no-es2restrictions.patch.1 \ vk_mem_alloc.patch.1 \ - tdf147342.patch.0 \ + macosmetal.patch.0 \ redefinition-of-op.patch.0 \ 0001-Added-missing-include-cstdio.patch \ fix-SkDebugf-link-error.patch.1 \ diff --git a/external/skia/macosmetal.patch.0 b/external/skia/macosmetal.patch.0 new file mode 100644 index 000000000000..3da9fc693fe9 --- /dev/null +++ b/external/skia/macosmetal.patch.0 @@ -0,0 +1,120 @@ +--- tools/sk_app/mac/WindowContextFactory_mac.h 2022-02-16 06:03:39.000000000 -0500 ++++ tools/sk_app/mac/WindowContextFactory_mac.h 2023-01-25 08:09:00.000000000 -0500 +@@ -19,15 +19,8 @@ + + struct DisplayParams; + +-static inline CGFloat GetBackingScaleFactor(NSView* view) { +- #ifdef SK_BUILD_FOR_IOS +- UIScreen* screen = view.window.screen ?: [UIScreen mainScreen]; +- return screen.nativeScale; +- #else +- NSScreen* screen = view.window.screen ?: [NSScreen mainScreen]; +- return screen.backingScaleFactor; +- #endif +-} ++SK_API CGFloat GetBackingScaleFactor(NSView* view); ++SK_API void ResetBackingScaleFactor(); + + namespace window_context_factory { + +--- /dev/null 2023-01-25 09:20:55.000000000 -0500 ++++ tools/sk_app/mac/WindowContextFactory_mac.mm 2023-01-25 09:21:22.000000000 -0500 +@@ -0,0 +1,57 @@ ++/* ++ * Use of this source code is governed by a BSD-style license that can be ++ * found in the LICENSE file. ++ */ ++ ++#include "tools/sk_app/mac/WindowContextFactory_mac.h" ++ ++namespace sk_app { ++ ++static bool bWindowScaling = false; ++static float fWindowScale = 1.0f; ++ ++CGFloat GetBackingScaleFactor(NSView* view) { ++ #ifdef SK_BUILD_FOR_IOS ++ UIScreen* screen = view.window.screen ?: [UIScreen mainScreen]; ++ return screen.nativeScale; ++ #else ++ // Related: tdf#147342 This should always be an exact copy of the ++ // sal::aqua::getWindowScaling() function in the following file: ++ // vcl/osx/salgdiutils.cxx ++ (void)view; ++ ++ if (!bWindowScaling) ++ { ++ NSArray *aScreens = [NSScreen screens]; ++ if (aScreens) ++ { ++ for (NSScreen *aScreen : aScreens) ++ { ++ float fScale = [aScreen backingScaleFactor]; ++ if (fScale > fWindowScale) ++ fWindowScale = fScale; ++ } ++ bWindowScaling = true; ++ } ++ if( const char* env = getenv("SAL_FORCE_HIDPI_SCALING")) ++ { ++ fWindowScale = atof(env); ++ bWindowScaling = true; ++ } ++ } ++ return fWindowScale; ++ #endif ++} ++ ++void ResetBackingScaleFactor() { ++ #ifndef SK_BUILD_FOR_IOS ++ // Related: tdf#147342 Force recalculation of the window scaling but keep ++ // the previous window scaling as the minimum so that we don't lose the ++ // resolution in cached images if a HiDPI monitor is disconnected and ++ // then reconnected. ++ bWindowScaling = false; ++ GetBackingScaleFactor(nil); ++ #endif ++} ++ ++} // namespace sk_app +--- tools/sk_app/mac/MetalWindowContext_mac.mm 2024-08-31 15:49:57 ++++ tools/sk_app/mac/MetalWindowContext_mac.mm 2024-09-25 20:09:32 +@@ -11,6 +11,8 @@ + #import + #import + ++#include ++ + using sk_app::DisplayParams; + using sk_app::window_context_factory::MacWindowInfo; + using sk_app::MetalWindowContext; +@@ -66,8 +68,7 @@ + fMetalLayer.autoresizingMask = kCALayerHeightSizable | kCALayerWidthSizable; + fMetalLayer.contentsGravity = kCAGravityTopLeft; + fMetalLayer.magnificationFilter = kCAFilterNearest; +- NSColorSpace* cs = fMainView.window.colorSpace; +- fMetalLayer.colorspace = cs.CGColorSpace; ++ fMetalLayer.colorspace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); + + fMainView.layer = fMetalLayer; + fMainView.wantsLayer = YES; +@@ -86,6 +87,18 @@ + fMetalLayer.drawableSize = backingSize; + fMetalLayer.contentsScale = backingScaleFactor; + ++ // Related: tdf#147342 Copy layer's colorspace to window's colorspace ++ // This method is now called when the window's backing properties have ++ // changed so copy any colorspace changes. ++ fMetalLayer.colorspace = CGColorSpaceCreateWithName(kCGColorSpaceSRGB); ++ ++ // Related tdf#145988 Reset layer's pixel format to MTLPixelFormatBGRA8Unorm ++ // Skia initally sets the layer's pixel format to be BGRA8888 but macOS ++ // may change the layer's pixel format when a window has moved to a screen ++ // with 30-bit color depth so reset it back to BGRA8888. ++ SAL_WARN_IF(fMetalLayer.pixelFormat != MTLPixelFormatBGRA8Unorm, "vcl.skia.metal", "CAMetalLayer pixel format is " << fMetalLayer.pixelFormat << " but should be " << MTLPixelFormatBGRA8Unorm << " (MTLPixelFormatBGRA8Unorm)"); ++ fMetalLayer.pixelFormat = MTLPixelFormatBGRA8Unorm; ++ + fWidth = backingSize.width; + fHeight = backingSize.height; + } diff --git a/external/skia/tdf147342.patch.0 b/external/skia/tdf147342.patch.0 deleted file mode 100644 index 3b50038c07ac..000000000000 --- a/external/skia/tdf147342.patch.0 +++ /dev/null @@ -1,110 +0,0 @@ ---- tools/sk_app/mac/WindowContextFactory_mac.h 2022-02-16 06:03:39.000000000 -0500 -+++ tools/sk_app/mac/WindowContextFactory_mac.h 2023-01-25 08:09:00.000000000 -0500 -@@ -19,15 +19,8 @@ - - struct DisplayParams; - --static inline CGFloat GetBackingScaleFactor(NSView* view) { -- #ifdef SK_BUILD_FOR_IOS -- UIScreen* screen = view.window.screen ?: [UIScreen mainScreen]; -- return screen.nativeScale; -- #else -- NSScreen* screen = view.window.screen ?: [NSScreen mainScreen]; -- return screen.backingScaleFactor; -- #endif --} -+SK_API CGFloat GetBackingScaleFactor(NSView* view); -+SK_API void ResetBackingScaleFactor(); - - namespace window_context_factory { - ---- tools/sk_app/mac/MetalWindowContext_mac.mm 2021-11-25 10:39:27.000000000 -0500 -+++ tools/sk_app/mac/MetalWindowContext_mac.mm 2023-01-28 14:55:57.000000000 -0500 -@@ -11,6 +11,8 @@ - #import - #import - -+#include -+ - using sk_app::DisplayParams; - using sk_app::window_context_factory::MacWindowInfo; - using sk_app::MetalWindowContext; -@@ -87,6 +89,18 @@ - fMetalLayer.drawableSize = backingSize; - fMetalLayer.contentsScale = backingScaleFactor; - -+ // Related: tdf#147342 Copy layer's colorspace to window's colorspace -+ // This method is now called when the window's backing properties have -+ // changed so copy any colorspace changes. -+ NSColorSpace* cs = fMainView.window.colorSpace; -+ fMetalLayer.colorspace = cs.CGColorSpace; -+ // Related tdf#145988 Reset layer's pixel format to MTLPixelFormatBGRA8Unorm -+ // Skia initally sets the layer's pixel format to be BGRA8888 but macOS -+ // may change the layer's pixel format when a window has moved to a screen -+ // with 30-bit color depth so reset it back to BGRA8888. -+ SAL_WARN_IF(fMetalLayer.pixelFormat != MTLPixelFormatBGRA8Unorm, "vcl.skia.metal", "CAMetalLayer pixel format is " << fMetalLayer.pixelFormat << " but should be " << MTLPixelFormatBGRA8Unorm << " (MTLPixelFormatBGRA8Unorm)"); -+ fMetalLayer.pixelFormat = MTLPixelFormatBGRA8Unorm; -+ - fWidth = backingSize.width; - fHeight = backingSize.height; - } ---- /dev/null 2023-01-25 09:20:55.000000000 -0500 -+++ tools/sk_app/mac/WindowContextFactory_mac.mm 2023-01-25 09:21:22.000000000 -0500 -@@ -0,0 +1,57 @@ -+/* -+ * Use of this source code is governed by a BSD-style license that can be -+ * found in the LICENSE file. -+ */ -+ -+#include "tools/sk_app/mac/WindowContextFactory_mac.h" -+ -+namespace sk_app { -+ -+static bool bWindowScaling = false; -+static float fWindowScale = 1.0f; -+ -+CGFloat GetBackingScaleFactor(NSView* view) { -+ #ifdef SK_BUILD_FOR_IOS -+ UIScreen* screen = view.window.screen ?: [UIScreen mainScreen]; -+ return screen.nativeScale; -+ #else -+ // Related: tdf#147342 This should always be an exact copy of the -+ // sal::aqua::getWindowScaling() function in the following file: -+ // vcl/osx/salgdiutils.cxx -+ (void)view; -+ -+ if (!bWindowScaling) -+ { -+ NSArray *aScreens = [NSScreen screens]; -+ if (aScreens) -+ { -+ for (NSScreen *aScreen : aScreens) -+ { -+ float fScale = [aScreen backingScaleFactor]; -+ if (fScale > fWindowScale) -+ fWindowScale = fScale; -+ } -+ bWindowScaling = true; -+ } -+ if( const char* env = getenv("SAL_FORCE_HIDPI_SCALING")) -+ { -+ fWindowScale = atof(env); -+ bWindowScaling = true; -+ } -+ } -+ return fWindowScale; -+ #endif -+} -+ -+void ResetBackingScaleFactor() { -+ #ifndef SK_BUILD_FOR_IOS -+ // Related: tdf#147342 Force recalculation of the window scaling but keep -+ // the previous window scaling as the minimum so that we don't lose the -+ // resolution in cached images if a HiDPI monitor is disconnected and -+ // then reconnected. -+ bWindowScaling = false; -+ GetBackingScaleFactor(nil); -+ #endif -+} -+ -+} // namespace sk_app -- cgit