bug#40845: SVG rendering issues

[Pip Cet <pipcet@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]

On Sat, Apr 25, 2020 at 12:20 PM Clément Pit-Claudel
<cpitclaudel@gmail.com> wrote:
>
> Hi all,
>
> As discussed on the mailing list, a number of issues currently exist with our SVG rendering implementation.  I have tried to summarize the ones I'm aware of in the following example.
>
> (with-current-buffer (get-buffer-create "*svg bugs*")
>   (erase-buffer)
>   (require 'face-remap)
>   (setq text-scale-mode-amount 10)
>   (text-scale-mode)
>   (let ((svg (svg-create 16 16)))
>     (svg-ellipse svg 8 8 4 4)
>     (insert "Text: ")
>     (print (svg-image svg :ascent 100))
>     (insert-image (svg-image svg :ascent 100))
>     (insert-image (svg-image svg :scale 5.0 :ascent 'center :foreground "red" :background "darkgreen"))
>     (add-text-properties
>      (point-min) (point-max)
>      '(face (:foreground "orange" :background "purple")
>             mouse-face '(:foreground "purple" :background "orange"))))
>   (pop-to-buffer (current-buffer)))
>
> The issues:
>
> 1. Manually scaling an image, as is done for the second image, doesn't re-render the svg: is scales the bitmap-rendered version of it, causing blurriness.
> 2. The SVG images don't inherit the background of the current face; instead, they inherit the background of the default face.
> 3. The SVG images don't inherit the foreground of the current face; instead, they use a black foreground.
> 4. The :foreground keyword has no effect on svg images.
> 5. The images are not scaled with the text: changing text-scale-mode-amount doesn't change the size of the images.

I would like to add

6. When the cursor is over an SVG image, it is displayed with a box
around it rather than with inverted video as characters are.

I've played around a while ago in an attempt to fix these issues, and
be able to define "character-like" SVG glyphs. My approach was to add
a display spec of the form (gen FN), which calls FN with the current
face parameters as arguments whenever redisplaying the spec, then uses
the return value (an image spec) as the actual spec to be displayed.

It's probably quite slow, but (as of June last) it works, even when
displaying the same buffer twice. Calling elisp from the redisplay
engine is, of course, something we don't really want to have more of,
but we'd do so anyway for a `when' spec (in fact, an alternative
approach to get this working with old Emacsen was to abuse a `when'
spec to set the image spec correctly).

The attached patch, IIRC, is the main part. As you can see, it leaves
most of the work to the Lisp side of things. It can fix all issues
mentioned above by modifying the SVG as required.

Also, IIRC, 3 is an rsvg issue.

[-- Attachment #2: gen.diff --]
[-- Type: text/x-patch, Size: 3137 bytes --]

commit 0af1850485f3ad1519e52de2b1694efa922917f4
Author: Pip Cet <pipcet@gmail.com>
Date:   Sat Jun 22 07:53:28 2019 +0000

    snapshot

diff --git a/src/callint.c b/src/callint.c
index 88a3c348d0..6f2e5d5066 100644
--- a/src/callint.c
+++ b/src/callint.c
@@ -824,6 +824,7 @@ syms_of_callint (void)
   DEFSYM (Qlet, "let");
   DEFSYM (Qif, "if");
   DEFSYM (Qwhen, "when");
+  DEFSYM (Qgen, "gen");
   DEFSYM (Qletx, "let*");
   DEFSYM (Qsave_excursion, "save-excursion");
   DEFSYM (Qprogn, "progn");
diff --git a/src/xdisp.c b/src/xdisp.c
index 5699aac61b..717fb30ec3 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2728,7 +2728,6 @@ safe_call2 (Lisp_Object fn, Lisp_Object arg1, Lisp_Object arg2)
   return safe_call (3, fn, arg1, arg2);
 }
 
-
 \f
 /***********************************************************************
 			      Debugging
@@ -4893,6 +4892,7 @@ handle_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
 #endif
       && !EQ (XCAR (spec), Qspace)
       && !EQ (XCAR (spec), Qwhen)
+      && !EQ (XCAR (spec), Qgen)
       && !EQ (XCAR (spec), Qslice)
       && !EQ (XCAR (spec), Qspace_width)
       && !EQ (XCAR (spec), Qheight)
@@ -4991,6 +4991,8 @@ display_prop_end (struct it *it, Lisp_Object object, struct text_pos start_pos)
    Value is non-zero if something was found which replaces the display
    of buffer or string text.  */
 
+extern Lisp_Object *lface_id_to_name;
+
 static int
 handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
 			    Lisp_Object overlay, struct text_pos *position,
@@ -5002,6 +5004,46 @@ handle_single_display_spec (struct it *it, Lisp_Object spec, Lisp_Object object,
   struct text_pos start_pos = *position;
   void *itdata = NULL;
 
+  if (it != NULL &&
+      CONSP (spec) &&
+      EQ (XCAR (spec), Qgen))
+    {
+      spec = XCDR (spec);
+      if (!CONSP (spec))
+	return 0;
+      Lisp_Object gen = XCAR (spec);
+      struct face *face = FACE_FROM_ID (it->f, it->face_id);
+      Lisp_Object lface = Qnil;
+      Lisp_Object props[] = {
+	QCtype,
+	QCfamily,
+	QCfoundry,
+	QCwidth,
+	QCheight,
+	QCweight,
+	QCslant,
+	QCunderline,
+	QCinverse_video,
+	QCforeground,
+	QCbackground,
+	QCstipple,
+	QCoverline,
+	QCstrike_through,
+	QCbox,
+	QCfont,
+	QCinherit,
+	QCfontset,
+	QCdistant_foreground,
+      };
+      for (int i = 0; i < LFACE_VECTOR_SIZE; i++)
+	lface = Fcons (Fcons (props[i], face->lface[i]),
+		       lface);
+      Lisp_Object font = Qnil;
+      XSETFONT (font, face->font);
+      lface = Fcons (Fcons (QCfont, font), lface);
+      spec = safe_call1 (gen, lface);
+    }
+
   /* If SPEC is a list of the form `(when FORM . VALUE)', evaluate FORM.
      If the result is non-nil, use VALUE instead of SPEC.  */
   form = Qt;
diff --git a/src/xfaces.c b/src/xfaces.c
index 012cc96470..47453c2b31 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -294,7 +294,7 @@ #define FACE_CACHE_BUCKETS_SIZE 1001
 
 /* A vector mapping Lisp face Id's to face names.  */
 
-static Lisp_Object *lface_id_to_name;
+Lisp_Object *lface_id_to_name;
 static ptrdiff_t lface_id_to_name_size;
 
 #ifdef HAVE_WINDOW_SYSTEM