>From e3ef587f86e3fe0fba1de80c070d495e06148c1b Mon Sep 17 00:00:00 2001 From: Alan Third Date: Sun, 3 May 2020 17:09:31 +0100 Subject: [PATCH] Set basic SVG attributes (bug#40845) * test/manual/image-transforms-tests.el: Replace hard-coded colors with defaults. * src/dispextern.h (struct image): * src/image.c (search_image_cache): (xbm_load_image): (xbm_load): (pbm_load): Rename from frame to face where relevant. (svg_load_image): Parse the image to find out the size, then wrap it in another SVG to set a new size and colors, etc. (lookup_image): Use the face colors instead of the frame colors. (search_image_cache): Add ability to ignore the face colors. (uncache_image): Uncache all copies of the image that share the spec, even if the face colors don't match. --- lisp/net/shr.el | 17 --- src/dispextern.h | 6 +- src/gtkutil.c | 2 +- src/image.c | 205 ++++++++++++++++++++------ src/nsmenu.m | 2 +- src/xdisp.c | 6 +- test/manual/image-transforms-tests.el | 50 +++---- 7 files changed, 189 insertions(+), 99 deletions(-) diff --git a/lisp/net/shr.el b/lisp/net/shr.el index ddd8112721..24595301b7 100644 --- a/lisp/net/shr.el +++ b/lisp/net/shr.el @@ -1209,25 +1209,8 @@ shr-parse-image-data ;; that are non-ASCII. (shr-dom-to-xml (libxml-parse-xml-region (point) (point-max)) 'utf-8))) - ;; SVG images often do not have a specified foreground/background - ;; color, so wrap them in styles. - (when (and (display-images-p) - (eq content-type 'image/svg+xml)) - (setq data (svg--wrap-svg data))) (list data content-type))) -(defun svg--wrap-svg (data) - "Add a default foreground colour to SVG images." - (let ((size (image-size (create-image data nil t :scaling 1) t))) - (with-temp-buffer - (insert - (format - "" - (face-foreground 'default) - (car size) (cdr size) - (base64-encode-string data t))) - (buffer-string)))) - (defun shr-image-displayer (content-function) "Return a function to display an image. CONTENT-FUNCTION is a function to retrieve an image for a cid url that diff --git a/src/dispextern.h b/src/dispextern.h index 311867a0c8..88bbb71fb9 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -3056,9 +3056,9 @@ reset_mouse_highlight (Mouse_HLInfo *hlinfo) if necessary. */ unsigned long background; - /* Foreground and background colors of the frame on which the image + /* Foreground and background colors of the face on which the image is created. */ - unsigned long frame_foreground, frame_background; + unsigned long face_foreground, face_background; /* True if this image has a `transparent' background -- that is, is uses an image mask. The accessor macro for this is @@ -3475,7 +3475,7 @@ #define TRY_WINDOW_IGNORE_FONTS_CHANGE (1 << 1) void mark_image_cache (struct image_cache *); bool valid_image_p (Lisp_Object); void prepare_image_for_display (struct frame *, struct image *); -ptrdiff_t lookup_image (struct frame *, Lisp_Object); +ptrdiff_t lookup_image (struct frame *, Lisp_Object, int face_id); #if defined HAVE_X_WINDOWS || defined USE_CAIRO || defined HAVE_NS #define RGB_PIXEL_COLOR unsigned long diff --git a/src/gtkutil.c b/src/gtkutil.c index 1fe160acca..fafd94c0f7 100644 --- a/src/gtkutil.c +++ b/src/gtkutil.c @@ -5113,7 +5113,7 @@ update_frame_tool_bar (struct frame *f) else idx = -1; - img_id = lookup_image (f, image); + img_id = lookup_image (f, image, -1); img = IMAGE_FROM_ID (f, img_id); prepare_image_for_display (f, img); diff --git a/src/image.c b/src/image.c index 123de54ba2..297278ec9a 100644 --- a/src/image.c +++ b/src/image.c @@ -1081,7 +1081,7 @@ DEFUN ("image-size", Fimage_size, Simage_size, 1, 3, 0, if (valid_image_p (spec)) { struct frame *f = decode_window_system_frame (frame); - ptrdiff_t id = lookup_image (f, spec); + ptrdiff_t id = lookup_image (f, spec, -1); struct image *img = IMAGE_FROM_ID (f, id); int width = img->width + 2 * img->hmargin; int height = img->height + 2 * img->vmargin; @@ -1111,7 +1111,7 @@ DEFUN ("image-mask-p", Fimage_mask_p, Simage_mask_p, 1, 2, 0, if (valid_image_p (spec)) { struct frame *f = decode_window_system_frame (frame); - ptrdiff_t id = lookup_image (f, spec); + ptrdiff_t id = lookup_image (f, spec, -1); struct image *img = IMAGE_FROM_ID (f, id); if (img->mask) mask = Qt; @@ -1134,7 +1134,7 @@ DEFUN ("image-metadata", Fimage_metadata, Simage_metadata, 1, 2, 0, if (valid_image_p (spec)) { struct frame *f = decode_window_system_frame (frame); - ptrdiff_t id = lookup_image (f, spec); + ptrdiff_t id = lookup_image (f, spec, -1); struct image *img = IMAGE_FROM_ID (f, id); ext = img->lisp_data; } @@ -1611,7 +1611,9 @@ equal_lists (Lisp_Object a, Lisp_Object b) /* Find an image matching SPEC in the cache, and return it. If no image is found, return NULL. */ static struct image * -search_image_cache (struct frame *f, Lisp_Object spec, EMACS_UINT hash) +search_image_cache (struct frame *f, Lisp_Object spec, EMACS_UINT hash, + unsigned long foreground, unsigned long background, + bool ignore_colors) { struct image *img; struct image_cache *c = FRAME_IMAGE_CACHE (f); @@ -1634,8 +1636,8 @@ search_image_cache (struct frame *f, Lisp_Object spec, EMACS_UINT hash) for (img = c->buckets[i]; img; img = img->next) if (img->hash == hash && equal_lists (img->spec, spec) - && img->frame_foreground == FRAME_FOREGROUND_PIXEL (f) - && img->frame_background == FRAME_BACKGROUND_PIXEL (f)) + && (ignore_colors || (img->face_foreground == foreground + && img->face_background == background))) break; return img; } @@ -1646,8 +1648,13 @@ search_image_cache (struct frame *f, Lisp_Object spec, EMACS_UINT hash) static void uncache_image (struct frame *f, Lisp_Object spec) { - struct image *img = search_image_cache (f, spec, sxhash (spec)); - if (img) + struct image *img; + + /* Because the background colors are based on the current face, we + can have multiple copies of an image with the same spec. We want + to remove them all to ensure the user doesn't see an old version + of the image when the face changes. */ + while ((img = search_image_cache (f, spec, sxhash (spec), 0, 0, true))) { free_image (f, img); /* As display glyphs may still be referring to the image ID, we @@ -2133,7 +2140,17 @@ image_set_transform (struct frame *f, struct image *img) /* Determine size. */ int width, height; - compute_image_size (img->width, img->height, img->spec, &width, &height); + +#ifdef HAVE_RSVG + /* SVGs are pre-scaled to the correct size. */ + if (EQ (image_spec_value (img->spec, QCtype, NULL), Qsvg)) + { + width = img->width; + height = img->height; + } + else +#endif + compute_image_size (img->width, img->height, img->spec, &width, &height); /* Determine rotation. */ double rotation = 0.0; @@ -2312,11 +2329,16 @@ image_set_transform (struct frame *f, struct image *img) SPEC must be a valid Lisp image specification (see valid_image_p). */ ptrdiff_t -lookup_image (struct frame *f, Lisp_Object spec) +lookup_image (struct frame *f, Lisp_Object spec, int face_id) { struct image *img; EMACS_UINT hash; + struct face *face = (face_id >= 0) ? FACE_FROM_ID (f, face_id) + : FACE_FROM_ID_OR_NULL (f, DEFAULT_FACE_ID); + unsigned long foreground = FACE_COLOR_TO_PIXEL (face->foreground, f); + unsigned long background = FACE_COLOR_TO_PIXEL (face->background, f); + /* F must be a window-system frame, and SPEC must be a valid image specification. */ eassert (FRAME_WINDOW_P (f)); @@ -2324,7 +2346,7 @@ lookup_image (struct frame *f, Lisp_Object spec) /* Look up SPEC in the hash table of the image cache. */ hash = sxhash (spec); - img = search_image_cache (f, spec, hash); + img = search_image_cache (f, spec, hash, foreground, background, true); if (img && img->load_failed_p) { free_image (f, img); @@ -2337,9 +2359,9 @@ lookup_image (struct frame *f, Lisp_Object spec) block_input (); img = make_image (spec, hash); cache_image (f, img); + img->face_foreground = foreground; + img->face_background = background; img->load_failed_p = ! img->type->load (f, img); - img->frame_foreground = FRAME_FOREGROUND_PIXEL (f); - img->frame_background = FRAME_BACKGROUND_PIXEL (f); /* If we can't load the image, and we don't have a width and height, use some arbitrary width and height so that we can @@ -2393,8 +2415,7 @@ lookup_image (struct frame *f, Lisp_Object spec) if (!NILP (bg)) { img->background - = image_alloc_image_color (f, img, bg, - FRAME_BACKGROUND_PIXEL (f)); + = image_alloc_image_color (f, img, bg, background); img->background_valid = 1; } } @@ -3667,8 +3688,8 @@ xbm_load_image (struct frame *f, struct image *img, char *contents, char *end) &data, 0); if (rc) { - unsigned long foreground = FRAME_FOREGROUND_PIXEL (f); - unsigned long background = FRAME_BACKGROUND_PIXEL (f); + unsigned long foreground = img->face_foreground; + unsigned long background = img->face_background; bool non_default_colors = 0; Lisp_Object value; @@ -3764,8 +3785,8 @@ xbm_load (struct frame *f, struct image *img) { struct image_keyword fmt[XBM_LAST]; Lisp_Object data; - unsigned long foreground = FRAME_FOREGROUND_PIXEL (f); - unsigned long background = FRAME_BACKGROUND_PIXEL (f); + unsigned long foreground = img->face_foreground; + unsigned long background = img->face_background; bool non_default_colors = 0; char *bits; bool parsed_p; @@ -6125,8 +6146,8 @@ pbm_load (struct frame *f, struct image *img) unsigned char c = 0; int g; struct image_keyword fmt[PBM_LAST]; - unsigned long fg = FRAME_FOREGROUND_PIXEL (f); - unsigned long bg = FRAME_BACKGROUND_PIXEL (f); + unsigned long fg = img->face_foreground; + unsigned long bg = img->face_background; /* Parse the image specification. */ memcpy (fmt, pbm_format, sizeof fmt); parse_image_spec (img->spec, fmt, PBM_LAST, Qpbm); @@ -9433,6 +9454,7 @@ DEFUN ("imagemagick-types", Fimagemagick_types, Simagemagick_types, 0, 0, 0, SVG_ALGORITHM, SVG_HEURISTIC_MASK, SVG_MASK, + SVG_FOREGROUND, SVG_BACKGROUND, SVG_LAST }; @@ -9451,6 +9473,7 @@ DEFUN ("imagemagick-types", Fimagemagick_types, Simagemagick_types, 0, 0, 0, {":conversion", IMAGE_DONT_CHECK_VALUE_TYPE, 0}, {":heuristic-mask", IMAGE_DONT_CHECK_VALUE_TYPE, 0}, {":mask", IMAGE_DONT_CHECK_VALUE_TYPE, 0}, + {":foreground", IMAGE_STRING_OR_NIL_VALUE, 0}, {":background", IMAGE_STRING_OR_NIL_VALUE, 0} }; @@ -9715,6 +9738,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents, int height; const guint8 *pixels; int rowstride; + char *wrapped_contents = NULL; + ptrdiff_t wrapped_size; #if ! GLIB_CHECK_VERSION (2, 36, 0) /* g_type_init is a glib function that must be called prior to @@ -9722,6 +9747,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents, g_type_init (); #endif + /* Parse the unmodified SVG data so we can get it's initial size. */ + #if LIBRSVG_CHECK_VERSION (2, 32, 0) GInputStream *input_stream = g_memory_input_stream_new_from_data (contents, size, NULL); @@ -9750,6 +9777,107 @@ svg_load_image (struct frame *f, struct image *img, char *contents, rsvg_handle_write (rsvg_handle, (unsigned char *) contents, size, &err); if (err) goto rsvg_error; + /* The parsing is complete, rsvg_handle is ready to used, close it + for further writes. */ + rsvg_handle_close (rsvg_handle, &err); + if (err) goto rsvg_error; +#endif + + /* Get the image dimensions. */ + rsvg_handle_get_dimensions (rsvg_handle, &dimension_data); + + /* We are now done with the unmodified data. */ + g_object_unref (rsvg_handle); + + /* Calculate the final image size. */ + compute_image_size (dimension_data.width, dimension_data.height, + img->spec, &width, &height); + + /* Wrap the SVG data in another SVG. This allows us to set the + width and height, as well as modify the foreground and background + colors. */ + { + Lisp_Object value; + unsigned long foreground = img->face_foreground; + unsigned long background = img->face_background; + + Lisp_Object encoded_contents = Fbase64_encode_string + (make_unibyte_string (contents, size), Qt); + + /* The wrapper sets the foreground color, width and height, and + viewBox must contain the dimensions of the original image. It + also draws a rectangle over the whole space, set to the + background color, before including the original image. This + acts to set the background color, instead of leaving it + transparent. */ + const char *wrapper = + ""; + + /* FIXME: I've added 64 in the hope it will cover the size of the + width and height strings and things. */ + int buffer_size = SBYTES (encoded_contents) + strlen (wrapper) + 64; + + value = image_spec_value (img->spec, QCforeground, NULL); + if (!NILP (value)) + { + foreground = image_alloc_image_color (f, img, value, img->face_foreground); + } + value = image_spec_value (img->spec, QCbackground, NULL); + if (!NILP (value)) + { + background = image_alloc_image_color (f, img, value, img->face_background); + img->background = background; + img->background_valid = 1; + } + + wrapped_contents = malloc (buffer_size); + + if (!wrapped_contents + || buffer_size <= snprintf (wrapped_contents, buffer_size, wrapper, + foreground & 0xFFFFFF, width, height, + dimension_data.width, dimension_data.height, + background & 0xFFFFFF, SSDATA (encoded_contents))) + goto rsvg_error; + + wrapped_size = strlen (wrapped_contents); + } + + /* Now we parse the wrapped version. */ + +#if LIBRSVG_CHECK_VERSION (2, 32, 0) + input_stream = g_memory_input_stream_new_from_data (wrapped_contents, wrapped_size, NULL); + base_file = filename ? g_file_new_for_path (filename) : NULL; + rsvg_handle = rsvg_handle_new_from_stream_sync (input_stream, base_file, + RSVG_HANDLE_FLAGS_NONE, + NULL, &err); + if (base_file) + g_object_unref (base_file); + g_object_unref (input_stream); + + /* Check rsvg_handle too, to avoid librsvg 2.40.13 bug (Bug#36773#26). */ + if (!rsvg_handle || err) goto rsvg_error; +#else + /* Make a handle to a new rsvg object. */ + rsvg_handle = rsvg_handle_new (); + eassume (rsvg_handle); + + /* Set base_uri for properly handling referenced images (via 'href'). + See rsvg bug 596114 - "image refs are relative to curdir, not .svg file" + . */ + if (filename) + rsvg_handle_set_base_uri (rsvg_handle, filename); + + /* Parse the contents argument and fill in the rsvg_handle. */ + rsvg_handle_write (rsvg_handle, (unsigned char *) wrapped_contents, wrapped_size, &err); + if (err) goto rsvg_error; + /* The parsing is complete, rsvg_handle is ready to used, close it for further writes. */ rsvg_handle_close (rsvg_handle, &err); @@ -9768,6 +9896,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents, pixbuf = rsvg_handle_get_pixbuf (rsvg_handle); if (!pixbuf) goto rsvg_error; g_object_unref (rsvg_handle); + free (wrapped_contents); /* Extract some meta data from the svg handle. */ width = gdk_pixbuf_get_width (pixbuf); @@ -9792,25 +9921,6 @@ svg_load_image (struct frame *f, struct image *img, char *contents, init_color_table (); - /* Handle alpha channel by combining the image with a background - color. */ - Emacs_Color background; - Lisp_Object specified_bg = image_spec_value (img->spec, QCbackground, NULL); - if (!STRINGP (specified_bg) - || !FRAME_TERMINAL (f)->defined_color_hook (f, - SSDATA (specified_bg), - &background, - false, - false)) - FRAME_TERMINAL (f)->query_frame_background_color (f, &background); - - /* SVG pixmaps specify transparency in the last byte, so right - shift 8 bits to get rid of it, since emacs doesn't support - transparency. */ - background.red >>= 8; - background.green >>= 8; - background.blue >>= 8; - /* This loop handles opacity values, since Emacs assumes non-transparent images. Each pixel must be "flattened" by calculating the resulting color, given the transparency of the @@ -9822,16 +9932,11 @@ svg_load_image (struct frame *f, struct image *img, char *contents, int red = *pixels++; int green = *pixels++; int blue = *pixels++; - int opacity = *pixels++; - red = ((red * opacity) - + (background.red * ((1 << 8) - opacity))); - green = ((green * opacity) - + (background.green * ((1 << 8) - opacity))); - blue = ((blue * opacity) - + (background.blue * ((1 << 8) - opacity))); + /* Skip opacity. */ + pixels++; - PUT_PIXEL (ximg, x, y, lookup_rgb_color (f, red, green, blue)); + PUT_PIXEL (ximg, x, y, lookup_rgb_color (f, red << 8, green << 8, blue << 8)); } pixels += rowstride - 4 * width; @@ -9861,6 +9966,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents, rsvg_error: if (rsvg_handle) g_object_unref (rsvg_handle); + if (wrapped_contents) + free (wrapped_contents); /* FIXME: Use error->message so the user knows what is the actual problem with the image. */ image_error ("Error parsing SVG image `%s'", img->spec); @@ -10159,7 +10266,7 @@ DEFUN ("lookup-image", Flookup_image, Slookup_image, 1, 1, 0, ptrdiff_t id = -1; if (valid_image_p (spec)) - id = lookup_image (SELECTED_FRAME (), spec); + id = lookup_image (SELECTED_FRAME (), spec, -1); debug_print (spec); return make_fixnum (id); diff --git a/src/nsmenu.m b/src/nsmenu.m index b7e4cbd565..e313fc03f4 100644 --- a/src/nsmenu.m +++ b/src/nsmenu.m @@ -1092,7 +1092,7 @@ - (Lisp_Object)runMenuAt: (NSPoint)p forFrame: (struct frame *)f continue; } - img_id = lookup_image (f, image); + img_id = lookup_image (f, image, -1); img = IMAGE_FROM_ID (f, img_id); prepare_image_for_display (f, img); diff --git a/src/xdisp.c b/src/xdisp.c index ad03ac4605..764d5345f2 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -5696,7 +5696,7 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object, else { it->what = IT_IMAGE; - it->image_id = lookup_image (it->f, value); + it->image_id = lookup_image (it->f, value, it->face_id); it->position = start_pos; it->object = NILP (object) ? it->w->contents : object; it->method = GET_FROM_IMAGE; @@ -22434,7 +22434,7 @@ push_prefix_prop (struct it *it, Lisp_Object prop) else if (IMAGEP (prop)) { it->what = IT_IMAGE; - it->image_id = lookup_image (it->f, prop); + it->image_id = lookup_image (it->f, prop, it->face_id); it->method = GET_FROM_IMAGE; } #endif /* HAVE_WINDOW_SYSTEM */ @@ -27335,7 +27335,7 @@ calc_pixel_width_or_height (double *res, struct it *it, Lisp_Object prop, if (FRAME_WINDOW_P (it->f) && valid_image_p (prop)) { - ptrdiff_t id = lookup_image (it->f, prop); + ptrdiff_t id = lookup_image (it->f, prop, it->face_id); struct image *img = IMAGE_FROM_ID (it->f, id); return OK_PIXELS (width_p ? img->width : img->height); diff --git a/test/manual/image-transforms-tests.el b/test/manual/image-transforms-tests.el index 0ebd5c7a19..02607e6367 100644 --- a/test/manual/image-transforms-tests.el +++ b/test/manual/image-transforms-tests.el @@ -48,24 +48,24 @@ test-cropping (let ((image "") (top-left "") (middle "") (bottom-right "")) (insert-header "Test Crop: cropping an image (only works with ImageMagick)") (insert-test "all params" top-left image '(:crop (10 10 0 0))) @@ -77,23 +77,23 @@ test-cropping (defun test-scaling () (let ((image "") (large "") (small "")) (insert-header "Test Scaling: resize an image (pixelization may occur)") (insert-test "1x" image image '(:scale 1)) @@ -107,27 +107,27 @@ test-scaling (defun test-scaling-rotation () (let ((image "") (x2-90 "") (x2--90 "") (x0.5-180 "")) (insert-header "Test Scaling and Rotation: resize and rotate an image (pixelization may occur)") (insert-test "1x, 0 degrees" image image '(:scale 1 :rotation 0)) -- 2.26.1