https://gitlab.freedesktop.org/cairo/cairo/-/merge_requests/290 From dbaab35b42c559ef3eb8ef0df6277246fd02a03f Mon Sep 17 00:00:00 2001 From: John Ralls Date: Sat, 26 Feb 2022 16:47:22 -0800 Subject: [PATCH 1/3] Conditionally Use Main Display ColorSpace instead of kCGColorSpaceDefaultRGB. The default RGB colorspace must be converted to the GPU's colorspace using CGColorTransformConvertUsingCMSConverter. Profiling has shown this function to consume as much as 48% of a redraw cycle in gdk-quartz. There seems to be no named colorspace that matches the one stored on the display, so we use the one associated with the main display. This has some risks for users with multiple monitors but in testing with my own two-monitor setup, one of which is HDR and the other not, the colorspace was the same for both. There's another issue: The monitor's color space is wrong for generating PNGs, producing somewhat faded colors and causing many tests to fail, so I've resorted to a bit of a hack: If cairo_quartz_surface_create_for_cg_context has been called then we're drawing for a screen and need to use its colorspace, otherwise we use kCGColorSpaceDefaultRGB as usual. Fixes https://gitlab.freedesktop.org/cairo/cairo/-/issues/330 --- src/cairo-mutex-list-private.h | 4 ++++ src/cairo-quartz-surface.c | 42 ++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/cairo-mutex-list-private.h b/src/cairo-mutex-list-private.h index 70d566ebb..eef6a10a1 100644 --- a/src/cairo-mutex-list-private.h +++ b/src/cairo-mutex-list-private.h @@ -68,6 +68,10 @@ CAIRO_MUTEX_DECLARE (_cairo_xcb_connections_mutex) CAIRO_MUTEX_DECLARE (_cairo_gl_context_mutex) #endif +#if CAIRO_HAS_QUARTZ_SURFACE +CAIRO_MUTEX_DECLARE (_cairo_quartz_surface_draw_to_screen_mutex) +#endif + #if !defined (HAS_ATOMIC_OPS) || defined (ATOMIC_OP_NEEDS_MEMORY_BARRIER) CAIRO_MUTEX_DECLARE (_cairo_atomic_mutex) #endif diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c index 5681918c4..d32ada32a 100644 --- a/src/cairo-quartz-surface.c +++ b/src/cairo-quartz-surface.c @@ -64,6 +64,10 @@ #define ND(_x) do {} while(0) #endif +#if ! CAIRO_NO_MUTEX +static bool _draw_to_screen = FALSE; +#endif + #define IS_EMPTY(s) ((s)->extents.width == 0 || (s)->extents.height == 0) /** @@ -166,6 +170,28 @@ static void quartz_ensure_symbols (void) _cairo_quartz_symbol_lookup_done = TRUE; } +static CGColorSpaceRef +_cairo_quartz_create_color_space (CGContextRef context) +{ + CGColorSpaceRef color_space = NULL; + + if (context) + { + color_space = CGBitmapContextGetColorSpace (context); + return CGColorSpaceRetain (color_space); + } +#if !CAIRO_NO_MUTEX + CAIRO_MUTEX_LOCK (_cairo_quartz_surface_draw_to_screen_mutex); + if (_draw_to_screen) + color_space = CGDisplayCopyColorSpace (CGMainDisplayID ()); + CAIRO_MUTEX_UNLOCK (_cairo_quartz_surface_draw_to_screen_mutex); +#endif + if (!color_space) + color_space = CGColorSpaceCreateDeviceRGB (); + + return color_space; +} + CGImageRef CairoQuartzCreateCGImage (cairo_format_t format, unsigned int width, @@ -186,7 +212,7 @@ CairoQuartzCreateCGImage (cairo_format_t format, switch (format) { case CAIRO_FORMAT_ARGB32: if (colorSpace == NULL) - colorSpace = CGColorSpaceCreateDeviceRGB (); + colorSpace = _cairo_quartz_create_color_space (NULL); bitinfo |= kCGImageAlphaPremultipliedFirst; bitsPerComponent = 8; bitsPerPixel = 32; @@ -194,7 +220,7 @@ CairoQuartzCreateCGImage (cairo_format_t format, case CAIRO_FORMAT_RGB24: if (colorSpace == NULL) - colorSpace = CGColorSpaceCreateDeviceRGB (); + colorSpace = _cairo_quartz_create_color_space (NULL); bitinfo |= kCGImageAlphaNoneSkipFirst; bitsPerComponent = 8; bitsPerPixel = 32; @@ -788,6 +814,7 @@ CairoQuartzCreateGradientFunction (const cairo_gradient_pattern_t *gradient, input_value_range, 4, gradient_output_value_ranges, + &gradient_callbacks); } @@ -1234,7 +1261,7 @@ _cairo_quartz_setup_gradient_source (cairo_quartz_drawing_state_t *state, if (unlikely (gradFunc == NULL)) return CAIRO_INT_STATUS_UNSUPPORTED; - rgb = CGColorSpaceCreateDeviceRGB (); + rgb = _cairo_quartz_create_color_space (NULL); if (gradient->base.type == CAIRO_PATTERN_TYPE_LINEAR) { state->shading = CGShadingCreateAxial (rgb, @@ -1485,7 +1512,7 @@ _cairo_quartz_surface_map_to_image (void *abstract_surface, bpp = CGBitmapContextGetBitsPerPixel (surface->cgContext); // let's hope they don't add YUV under us - colorspace = CGBitmapContextGetColorSpace (surface->cgContext); + colorspace = _cairo_quartz_create_color_space (surface->cgContext); color_comps = CGColorSpaceGetNumberOfComponents (colorspace); /* XXX TODO: We can handle many more data formats by @@ -2398,6 +2425,11 @@ cairo_quartz_surface_create_for_cg_context (CGContextRef cgContext, { cairo_quartz_surface_t *surf; +#if !CAIRO_NO_MUTEX + CAIRO_MUTEX_LOCK (_cairo_quartz_surface_draw_to_screen_mutex); + _draw_to_screen = TRUE; + CAIRO_MUTEX_UNLOCK (_cairo_quartz_surface_draw_to_screen_mutex); +#endif surf = _cairo_quartz_surface_create_internal (cgContext, CAIRO_CONTENT_COLOR_ALPHA, width, height); if (likely (!surf->base.status)) @@ -2445,7 +2477,7 @@ cairo_quartz_surface_create (cairo_format_t format, if (format == CAIRO_FORMAT_ARGB32 || format == CAIRO_FORMAT_RGB24) { - cgColorspace = CGColorSpaceCreateDeviceRGB (); + cgColorspace = _cairo_quartz_create_color_space (NULL); bitinfo = kCGBitmapByteOrder32Host; if (format == CAIRO_FORMAT_ARGB32) bitinfo |= kCGImageAlphaPremultipliedFirst; -- GitLab From 94a3792078de492920af2944b25a5e1b6722a8ac Mon Sep 17 00:00:00 2001 From: John Ralls Date: Sat, 26 Feb 2022 17:53:50 -0800 Subject: [PATCH 2/3] [quartz] Use CGContextSetFooColorWithColor Replacing CGContextSetFooRGBColor because that re-sets the CGContext's color space to kCGColorSpaceDefaultRGB and we don't want that. --- src/cairo-quartz-surface.c | 53 ++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c index d32ada32a..ec8cd9b75 100644 --- a/src/cairo-quartz-surface.c +++ b/src/cairo-quartz-surface.c @@ -192,6 +192,25 @@ _cairo_quartz_create_color_space (CGContextRef context) return color_space; } +static CGColorRef +_cairo_quartz_create_cgcolor (CGColorSpaceRef cs, CGFloat red, CGFloat green, + CGFloat blue, CGFloat alpha) +{ + CGFloat colors[4] = { red, green, blue, alpha }; + CGColorRef cgc; + if (!cs) + cs = _cairo_quartz_create_color_space (NULL); + cgc = CGColorCreate (cs, colors); + CGColorSpaceRelease (cs); + return cgc; +} + +static CGColorRef +_cairo_quartz_black (CGColorSpaceRef cs) +{ + return _cairo_quartz_create_cgcolor (cs, 0.0, 0.0, 0.0, 1.0); +} + CGImageRef CairoQuartzCreateCGImage (cairo_format_t format, unsigned int width, @@ -1128,6 +1147,7 @@ _cairo_quartz_setup_pattern_source (cairo_quartz_drawing_state_t *state, CGColorSpaceRef patternSpace; CGPatternRef cgpat = NULL; cairo_int_status_t status; + CGColorRef black = _cairo_quartz_black (_cairo_quartz_create_color_space (state->cgDrawContext)); _cairo_surface_get_extents (&surface->base, &extents); @@ -1183,7 +1203,7 @@ _cairo_quartz_setup_pattern_source (cairo_quartz_drawing_state_t *state, -state->clipRect.origin.y); } - CGContextSetRGBFillColor (state->cgDrawContext, 0, 0, 0, 1); + CGContextSetFillColorWithColor (state->cgDrawContext, black); state->rect = CGRectMake (0, 0, pattern_extents.width, pattern_extents.height); state->action = DO_IMAGE; @@ -1222,6 +1242,7 @@ _cairo_quartz_setup_pattern_source (cairo_quartz_drawing_state_t *state, CGContextSetPatternPhase (state->cgDrawContext, CGSizeMake (0, 0)); CGPatternRelease (cgpat); + CGColorRelease (black); state->action = DO_DIRECT; return CAIRO_STATUS_SUCCESS; @@ -1300,6 +1321,7 @@ _cairo_quartz_setup_state (cairo_quartz_drawing_state_t *state, const cairo_clip_t *clip = composite->clip; cairo_bool_t needs_temp; cairo_status_t status; + CGColorRef black; state->layer = NULL; state->image = NULL; @@ -1332,9 +1354,11 @@ _cairo_quartz_setup_state (cairo_quartz_drawing_state_t *state, state->filter = _cairo_quartz_filter_to_quartz (source->filter); if (op == CAIRO_OPERATOR_CLEAR) { - CGContextSetRGBFillColor (state->cgDrawContext, 0, 0, 0, 1); + CGColorRef black = _cairo_quartz_black (_cairo_quartz_create_color_space (surface->cgContext)); + CGContextSetFillColorWithColor (state->cgDrawContext, black); state->action = DO_DIRECT; + CGColorRelease (black); return CAIRO_STATUS_SUCCESS; } @@ -1371,18 +1395,17 @@ _cairo_quartz_setup_state (cairo_quartz_drawing_state_t *state, if (source->type == CAIRO_PATTERN_TYPE_SOLID) { cairo_solid_pattern_t *solid = (cairo_solid_pattern_t *) source; + CGColorRef color = _cairo_quartz_create_cgcolor (_cairo_quartz_create_color_space (state->cgDrawContext), + solid->color.red, + solid->color.green, + solid->color.blue, + solid->color.alpha); - CGContextSetRGBStrokeColor (state->cgDrawContext, - solid->color.red, - solid->color.green, - solid->color.blue, - solid->color.alpha); - CGContextSetRGBFillColor (state->cgDrawContext, - solid->color.red, - solid->color.green, - solid->color.blue, - solid->color.alpha); + CGContextSetStrokeColorWithColor (state->cgDrawContext, color); + CGContextSetFillColorWithColor (state->cgDrawContext, color); + + CGColorRelease (color); state->action = DO_DIRECT; return CAIRO_STATUS_SUCCESS; } @@ -1436,6 +1459,9 @@ static inline void _cairo_quartz_draw_cgcontext (cairo_quartz_drawing_state_t *state, cairo_operator_t op) { + CGColorSpaceRef cs = _cairo_quartz_create_color_space (state->cgDrawContext); + CGColorRef transparent = _cairo_quartz_create_cgcolor (cs, 0.0, 0.0, 0.0, 0.0); + if (! (op == CAIRO_OPERATOR_SOURCE && state->cgDrawContext == state->cgMaskContext)) return; @@ -1450,8 +1476,9 @@ _cairo_quartz_draw_cgcontext (cairo_quartz_drawing_state_t *state, CGContextAddRect (state->cgDrawContext, state->clipRect); - CGContextSetRGBFillColor (state->cgDrawContext, 0, 0, 0, 0); + CGContextSetFillColorWithColor (state->cgDrawContext, transparent); CGContextEOFillPath (state->cgDrawContext); + CGColorRelease (transparent); } -- GitLab From 79624a15d3c3bfcd089d2d833bd986a1177301f3 Mon Sep 17 00:00:00 2001 From: John Ralls Date: Thu, 7 Apr 2022 10:58:02 -0700 Subject: [PATCH 3/3] Use macOS png creation facilities for creating test PNGs --- boilerplate/cairo-boilerplate-quartz.c | 6 +-- src/cairo-quartz-private.h | 3 ++ src/cairo-quartz-surface.c | 52 ++++++-------------------- 3 files changed, 18 insertions(+), 43 deletions(-) diff --git a/boilerplate/cairo-boilerplate-quartz.c b/boilerplate/cairo-boilerplate-quartz.c index d4ca35383..1a1417c75 100644 --- a/boilerplate/cairo-boilerplate-quartz.c +++ b/boilerplate/cairo-boilerplate-quartz.c @@ -26,7 +26,7 @@ #include "cairo-boilerplate-private.h" -#include +#include static cairo_surface_t * _cairo_boilerplate_quartz_create_surface (const char *name, @@ -56,7 +56,7 @@ static const cairo_boilerplate_target_t targets[] = { cairo_surface_create_similar, NULL, NULL, _cairo_boilerplate_get_image_surface, - cairo_surface_write_to_png, + _cairo_quartz_surface_to_png, NULL, NULL, NULL, TRUE, FALSE, FALSE }, @@ -68,7 +68,7 @@ static const cairo_boilerplate_target_t targets[] = { cairo_surface_create_similar, NULL, NULL, _cairo_boilerplate_get_image_surface, - cairo_surface_write_to_png, + _cairo_quartz_surface_to_png, NULL, NULL, NULL, FALSE, FALSE, FALSE }, diff --git a/src/cairo-quartz-private.h b/src/cairo-quartz-private.h index 609a9052c..08d9a33c1 100644 --- a/src/cairo-quartz-private.h +++ b/src/cairo-quartz-private.h @@ -106,6 +106,9 @@ CairoQuartzCreateCGImage (cairo_format_t format, cairo_private CGFontRef _cairo_quartz_scaled_font_get_cg_font_ref (cairo_scaled_font_t *sfont); +cairo_status_t _cairo_quartz_surface_to_png (cairo_quartz_surface_t *nq, const char *dest); +cairo_private void _cairo_quartz_image_to_png (CGImageRef, const char *dest); + #else # error Cairo was not compiled with support for the quartz backend diff --git a/src/cairo-quartz-surface.c b/src/cairo-quartz-surface.c index ec8cd9b75..13f409339 100644 --- a/src/cairo-quartz-surface.c +++ b/src/cairo-quartz-surface.c @@ -64,10 +64,6 @@ #define ND(_x) do {} while(0) #endif -#if ! CAIRO_NO_MUTEX -static bool _draw_to_screen = FALSE; -#endif - #define IS_EMPTY(s) ((s)->extents.width == 0 || (s)->extents.height == 0) /** @@ -126,15 +122,6 @@ static bool (*CGContextGetAllowsFontSmoothingPtr) (CGContextRef) = NULL; static cairo_bool_t _cairo_quartz_symbol_lookup_done = FALSE; -/* - * Utility functions - */ - -#ifdef QUARTZ_DEBUG -static void quartz_surface_to_png (cairo_quartz_surface_t *nq, const char *dest); -static void quartz_image_to_png (CGImageRef, const char *dest); -#endif - typedef struct { cairo_surface_t base; @@ -180,12 +167,8 @@ _cairo_quartz_create_color_space (CGContextRef context) color_space = CGBitmapContextGetColorSpace (context); return CGColorSpaceRetain (color_space); } -#if !CAIRO_NO_MUTEX - CAIRO_MUTEX_LOCK (_cairo_quartz_surface_draw_to_screen_mutex); - if (_draw_to_screen) - color_space = CGDisplayCopyColorSpace (CGMainDisplayID ()); - CAIRO_MUTEX_UNLOCK (_cairo_quartz_surface_draw_to_screen_mutex); -#endif + color_space = CGDisplayCopyColorSpace (CGMainDisplayID ()); + if (!color_space) color_space = CGColorSpaceCreateDeviceRGB (); @@ -2450,15 +2433,9 @@ cairo_quartz_surface_create_for_cg_context (CGContextRef cgContext, unsigned int width, unsigned int height) { - cairo_quartz_surface_t *surf; - -#if !CAIRO_NO_MUTEX - CAIRO_MUTEX_LOCK (_cairo_quartz_surface_draw_to_screen_mutex); - _draw_to_screen = TRUE; - CAIRO_MUTEX_UNLOCK (_cairo_quartz_surface_draw_to_screen_mutex); -#endif - surf = _cairo_quartz_surface_create_internal (cgContext, CAIRO_CONTENT_COLOR_ALPHA, - width, height); + cairo_quartz_surface_t *surf = + _cairo_quartz_surface_create_internal (cgContext, CAIRO_CONTENT_COLOR_ALPHA, + width, height); if (likely (!surf->base.status)) CGContextRetain (cgContext); @@ -2673,12 +2650,8 @@ _cairo_quartz_surface_snapshot_get_image (cairo_quartz_surface_t *surface) return CGImageRetain (((cairo_quartz_snapshot_t*)snapshot)->image); } -/* Debug stuff */ - -#ifdef QUARTZ_DEBUG - void -quartz_image_to_png (CGImageRef image, const char *dest) +_cairo_quartz_image_to_png (CGImageRef image, const char *dest) { static int sctr = 0; const char* image_name = "quartz-image"; @@ -2691,7 +2664,7 @@ quartz_image_to_png (CGImageRef image, const char *dest) memset (pathbuf, 0, sizeof (pathbuf)); dest = dest ? dest : image_name; - snprintf (pathbuf, sizeof (pathbuf), "%s/Desktop/%s%d.png",getenv ("HOME"), dest, sctr++, ext); + snprintf (pathbuf, sizeof (pathbuf), "%s/Desktop/%s%d.png",getenv ("HOME"), dest, sctr++); path = CFStringCreateWithCString (NULL, pathbuf, kCFStringEncodingUTF8); url = CFURLCreateWithFileSystemPath (NULL, path, kCFURLPOSIXPathStyle, FALSE); image_dest = CGImageDestinationCreateWithURL (url, png_utti, 1, NULL); @@ -2703,21 +2676,20 @@ quartz_image_to_png (CGImageRef image, const char *dest) CFRelease (path); } -void -quartz_surface_to_png (cairo_quartz_surface_t *nq, const char *dest) +cairo_status_t +_cairo_quartz_surface_to_png (cairo_quartz_surface_t *nq, const char *dest) { static int sctr = 0; CGImageRef image; if (nq->base.type != CAIRO_SURFACE_TYPE_QUARTZ) { fprintf (stderr, "** quartz_surface_to_png: surface %p isn't quartz!\n", nq); - return; + return CAIRO_STATUS_SURFACE_TYPE_MISMATCH; } image = CGBitmapContextCreateImage (nq->cgContext); - quartz_image_to_png (image, dest); + _cairo_quartz_image_to_png (image, dest); CGImageRelease (image); + return CAIRO_STATUS_SUCCESS; } - -#endif /* QUARTZ_DEBUG */ -- GitLab