rougier / svg-tag-mode

A minor mode for Emacs that replace keywords with nice SVG labels
GNU General Public License v3.0
496 stars 28 forks source link

Fix calculations and hard-code less #14

Closed tarsius closed 1 year ago

tarsius commented 3 years ago

Closes #12.

All but the last commit perform some cleanup. The last commit makes the big changes. I could have tried to split that into multiple commits, but don't think the work involved in that would be worth it.

There were four major issues:

  1. image-compute-scaling-factor was not taken into account. (While that says "image", the same factor applies to faces.)
  2. Calls to face-attribute did not use the inherit property, which lead to lots of duplication, complications and probably some bugs.
  3. Likely in order to deal with issues caused by (1) and (2) fonts were hard-coded, hiding issues and increasing the likelihood this breaks completely for many users (who don't have the font installed).
  4. The calculations seemed a bit obfuscated at times. ;)

I'll probably be adding svg-tag--make to forge (under a different name of course ;) and use it for labels. Or maybe even for refnames in magit. That's why I looked into svg-tag-mode in the first place!

tarsius commented 3 years ago

Uh, I should not that I have not tested this much. I drafed this code and then let it sit back when I brought up the issue; and how I have cleaned it up a bit because of the activity in #12.

jamesnvc commented 3 years ago

This seems to work for me!

EDIT: Although to make the examples work, I do need to manually set a different :height on the faces to make them be the correct size.

jamesnvc commented 3 years ago

I think I also have a fairly weird display set up...I have xorg conf file setting the DPI to 144, but also Xft.dpi: 180 in my ~/.Xresources; fair enough if Emacs is confused then, I certainly am.

rougier commented 3 years ago

Thank you! Impressive, I've a lot to learn . I'll test it on my side but if no complaints, I'll merge.

jamesnvc commented 3 years ago

I'm very curious how the drawing works internally...I continue to see that omitting the :font-size in the call to svg-text makes the text show as the correct size (in conjunction with changing the text-y to (- txt-height 5) instead of font-size), while normally it's way too small and in the top-left corner. I don't know if it's worth trying to figure out what calculation is needed -- maybe it's just my weird setup that necessitates this -- or just set a much-larger font size for the svg face.

rougier commented 3 years ago

Could post some screenshots showing the problem?

jamesnvc commented 3 years ago

Opening example-2.el & eval-ing with emacs -Q: screenshot-2021-04-30_15-50-49

Commenting out the :font-size line in svg-tag--make and repeating: screenshot-2021-04-30_15-59-25

Commenting out :font-size and changing (text-y font-size) to (text-y (- txt-height 5)) in svg-tag--make: screenshot-2021-04-30_16-00-24

rougier commented 3 years ago

What does (frame-monitor-attributes) returns ?

tarsius commented 3 years ago

Well, it seems my calculations and/or there are values I am not taking into account are off too. :/

So I very much welcome it if other people also try to figure out why it doesn't work for them.

For debugging purposes I recommend editing svg-tag--make to return svg instead of (svg-image svg ...) and pretty-print the value.

jamesnvc commented 3 years ago

What does (frame-monitor-attributes) returns ?

((name . "DP-1") (geometry 0 0 3840 2160) (workarea 0 0 3840 2160)
 (mm-size 610 350) 
 (frames #<frame *scratch* - GNU Emacs at gonk 0x56359771c210>) (source . "Gdk"))

Well, it seems my calculations and/or there are values I am not taking into account are off too. :/

It is entirely possible I've just done something so bizarre with my DPI settings that Emacs is doing the wrong thing somewhere...although I do find it suspicious that not passing :font-size to svg-text does the right thing.

The value of svg seems reasonable enough; for example, this is svg-tag-org-todo from example-2.el:


(svg 
 ((width . 60) (height . 23) 
  (version . "1.1") (xmlns . "http://www.w3.org/2000/svg")
  (xmlns:xlink . "http://www.w3.org/1999/xlink"))
 (rect
  ((width . 50) (height . 21)
   (x . 5.0) (y . 0) (rx . 2)
   (fill . "#FFAB91")))
 (rect
  ((width . 49.0) (height . 20.0)
   (x . 5.5) (y . 0.5)
   (rx . 1.5) (fill . "#FFAB91")))
 (text
  ((y . 10.040268456375841) (x . 10.0)
   (fill . "#ffffff") (font-size . 10.040268456375841)
   (font-weight) (font-family . "PragmataPro Liga"))
  "TODO"))
rougier commented 3 years ago

From https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/font-size, it seems the default font-size is medium. Can you try specifying this font size and check if you get the same result as when not specifying the font size?

jamesnvc commented 3 years ago

Hm, "medium" seems to make the text too big:

screenshot-2021-05-03_09-18-10

I did note that setting the font-size to be "1em" appears to be the correct size, albeit misaligned vertically:

screenshot-2021-05-03_09-19-49

rougier commented 3 years ago

Interesting. I'm not sure how Emacs or librsvg computes the 1em. Do you know which svg library your emacs is using? For the vertical alignment, I think it might be font dependent and there nothing much we can dot but adapt manually the offset.

jamesnvc commented 3 years ago

Do you know which svg library your emacs is using?

Looks like librsvg2 2.48.9 (I've built it myself from git).

I'm not sure how Emacs or librsvg computes the 1em

Reading svg_load_image in image.c, it seems like it pretty much just passes the content to rsvg. I'll have a look at rsvg and see if I can figure out what it's doing.

For the vertical alignment, I think it might be font dependent and there nothing much we can dot but adapt manually the offset.

Yeah...might be able to compute something from window-font-height?

jamesnvc commented 3 years ago

Ah, I see that it injects CSS into the SVG that sets a default font-size and font-family from the face used to display the image. Unsure then why it's different from the font-size set on the text element...

jamesnvc commented 3 years ago

Another interesting data point; I tried setting the font size in pt (instead of unsuffixed which will be pixels) and now it renders differently in Firefox and Emacs:

Firefox: screenshot-2021-05-03_12-54-14

Emacs: screenshot-2021-05-03_12-54-33

svg:

<svg width="60" height="23" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
  <rect width="50" height="21" x="5.0" y="0" rx="2" fill="#FFAB91"></rect>
  <rect width="49.0" height="20.0" x="5.5" y="0.5" rx="1.5" fill="#FFAB91"></rect>
  <text y="10.053691275167786pt" x="10.0pt" fill="#ffffff" font-size="10.053691275167786pt" font-weight="nil" font-family="PragmataPro Liga">
    TODO
  </text>
</svg>
rougier commented 3 years ago

librsvg seems to have suffered from dpi problems (https://gitlab.gnome.org/GNOME/librsvg/-/issues/427) but I don't know if this is related.

jamesnvc commented 3 years ago

I do see calls to rsvg_handle_set_dpi_x_y which I assume are to address DPI issues, but maybe the text drawing does something different with those...

jamesnvc commented 3 years ago

I see there was an issue in Emacs' svg rendering on high-dpi screens, but it was apparently resolved: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=45124

jamesnvc commented 3 years ago

I have something working by taking a somewhat different approach -- calculating the font size in ems instead of pixels. Setting the text-y still needs some fiddling though...does this work for other folks?

diff --git a/svg-tag-mode.el b/svg-tag-mode.el
index 49cca62..a673aa6 100644
--- a/svg-tag-mode.el
+++ b/svg-tag-mode.el
@@ -141,14 +141,13 @@ (defun svg-tag--make (text face padding margin radius)
          (text-x      (+ tag-x
                          (/ (- tag-width (* (length text) txt-width))
                             2)))
-         (font-size   (* (ceiling
-                          (* (face-attribute face :height nil 'default)
-                             0.1))
-                         (image-compute-scaling-factor 'auto)))
-         (txt-height  (window-font-height))
+         (font-size   (format "%sem"
+                              (/ (face-attribute face :height nil 'default)
+                                 (face-attribute 'default :height nil))))
+         (txt-height  (window-font-height nil face))
          (svg-height  txt-height)
          (tag-height  (- txt-height 2))
-         (text-y      font-size)
+         (text-y      (- txt-height 5))
          (svg         (svg-create svg-width svg-height)))
     (svg-rectangle svg tag-x 0 tag-width tag-height
                    :fill (if box box-color background)

EDIT fixing patch

tarsius commented 3 years ago

Doesn't work for me:

20210511-09:08:46

jamesnvc commented 3 years ago

I confess myself very confused. I would love to see what this looks like on other OSes & DPIs; right now, I'm just not sure where to look.

rougier commented 3 years ago

Another try below. I'm able to extract the ascent and descent that should fix the position of the text but the pixel size is wrong for me. I set the default size to "140" in Emacs but the reading of the pixel size returns me 8.

Can you check the code below on your side (you'll need to adapt font size until we find where we can read this value):

;; See www.gnu.org/software/emacs/manual/html_node/elisp/Low_002dLevel-Font.html
;;
;; Pixel size
;; (elt (query-font (font-at (point-min))) 3)
;;  -> This is wrong for me (return 8 instead of 14)
;;
;; Descent
;; (elt (query-font (font-at (point-min))) 4)
;;
;; Ascent
;; (elt (query-font (font-at (point-min))) 5)
;;
;; Average width
;; (elt (query-font (font-at (point-min))) 7)

(let* ((text       "Hello guys!")

       ;; This should correspond the size of your default face
       (font-size  14)
       ;; (elt (query-font (font-at (point-min))) 3)

       (descent    (elt (query-font (font-at (point-min))) 4))
       (ascent     (elt (query-font (font-at (point-min))) 5))
       (svg-height (+ ascent descent))
       (char-width (elt (query-font (font-at (point-min))) 7))
       (svg-width  (* char-width (length text)))
       (svg        (svg-create svg-width svg-height)))

  (svg-text svg text
            :fill "black"
            :stroke-width 0
            :font-family "Roboto Mono"
            :font-weight "300"
            :font-size   font-size
            :x 0
            :y descent)
  (insert-image (svg-image svg :ascent 'center)))
rougier commented 3 years ago

Also, can you try (/ (display-pixel-width) (/ (display-mm-width) 25.4)) (this should returns the dpi as seen by emacs)

jamesnvc commented 3 years ago

Can you check the code below on your side

When I up the font-size to 20 (where my font is actually set to 9) it looks perfect.

can you try (/ (display-pixel-width) (/ (display-mm-width) 25.4))

That gives me 159.112..., which seems pretty much correct (actual DPI 159)

rougier commented 3 years ago

I suspect that the SVG uses a DPI of 72 instead of actual DPI. In your case, 9 would need to be multiplied by 159/72 ~ 2.2. But more generally, I think this means we need to set the viewBox properly and this should solve all our problem. But I'm not quite sure what is the right value for the viewbox.

rougier commented 3 years ago

Could you test code below and play with font size (text-scale-adjust) to check if it is working properly ?

(let* ((text "Hello guys!")
       (font-dpi   72.272) ;; see https://en.wikipedia.org/wiki/Point_(typography)
       (screen-dpi (/ (display-pixel-width) (/ (display-mm-width) 25.4)))
       (font-size  (elt (query-font (font-at (point-min))) 3))
       (font-size  (truncate (* font-size (/ screen-dpi font-dpi))))
       (family     (face-attribute 'default :family))
       (descent    (elt (query-font (font-at (point-min))) 4))
       (ascent     (elt (query-font (font-at (point-min))) 5))
       (svg-height (+ ascent descent))
       (char-width (elt (query-font (font-at (point-min))) 7))
       (svg-width  (* char-width (length text)))
       (svg        (svg-create svg-width svg-height)))
  (svg-text svg text
            :fill "black"
            :stroke-width 0
            :font-family family
            :font-weight "300"
            :font-size font-size
            :x 0
            :y descent)
  (insert-image (svg-image svg :ascent 'center)))
jamesnvc commented 3 years ago

Could you test code below and play with font size

Hm, it seems slightly too large for me, truncating the exclamation mark, but consistent with different font sizes.

Original: screenshot-2021-05-16_10-24-29

Scaled up a bunch: screenshot-2021-05-16_10-24-40

Scaled down a bunch: screenshot-2021-05-16_10-24-51

jamesnvc commented 3 years ago

Hm, I see though that (elt (query-font (font-at (point-min))) 3) gives 10, but I have my default font set to 9, which seems like that would cause the different size :/

rougier commented 3 years ago

How do you set the initial size of your font in your .emacs?

jamesnvc commented 3 years ago

How do you set the initial size of your font in your .emacs?

(set-frame-font "PragmataPro Liga 9" nil t)

rougier commented 3 years ago

And if you try to customize face default, what is the height ?

jamesnvc commented 3 years ago

And if you try to customize face default, what is the height ?

91 - so I guess this font is just tall enough that it gets rounded up or something? Weird...

rougier commented 3 years ago

Ok, maybe we can use (/ (face-attribute 'default :height) 10) for font-size. When it is an integer, size is expressed in 1/10 of pt. But for me, this value is 14 wich is the right value fot the svg (without dpi conversion).

jamesnvc commented 3 years ago

I just tried using the face-attribute method, but then it doesn't work properly with text-scale-adjust, although it is okay normally.

rougier commented 3 years ago

That is expected. When you scale manually a buffer, it does not change the face.

rougier commented 3 years ago

What is the output of (display-monitor-attributes-list) ?

jamesnvc commented 3 years ago

(((name . "DP-1")
  (geometry 0 0 3840 2160)
  (workarea 0 0 3840 2160)
  (mm-size 610 350)
  (frames #<frame /home/james/org/todo.org 0x55e7ece72890> #<frame *scratch* 0x55e7ede43320> #<frame /home/james/Projects/writing/_drafts/braindump4.org 0x55e7f104d2f8> #<frame /home/james/src/terminusdb-client-python/terminusdb_client/woqlquery/woql_query.py 0x55e7f7a101f0> #<frame  0x55e7f3e23b70> #<frame /home/james/org/Raj_work.org 0x55e801d05008> #<frame *scratch* 0x55e803854ef0> #<frame  0x55e804679f40> #<frame braindump4.org 0x55e804ede448> #<frame *eshell*<4> 0x55e806ef7c10>)
  (source . "Gdk")))
rougier commented 3 years ago

So maybe we can use both value to compute scaling factor. What do you get from these two lines?

(/ (face-attribute 'default :height) 10)             ;; For me 14 (screen pt?)
(elt (query-font (face-attribute 'default :font)) 3) ;; For me 8 (font pt ?)
jamesnvc commented 3 years ago

(/ (face-attribute 'default :height) 10)

9

(elt (query-font (face-attribute 'default :font)) 3)

10

rougier commented 3 years ago

I'm a bit lost. I need to find how emacs convert pt to pixels. But given your dpi and the size you get, your font should be really tiny on screen and from your screenshot, it does not seem to be the case. Do you know if gtk might interfere with the rendering? Like a global scaling factor?

rougier commented 3 years ago

For example: https://wiki.archlinux.org/title/HiDPI#Fractional_scaling

rougier commented 3 years ago

Can you try (on the command line): GDK_SCALE= emacs (from https://github.com/syl20bnr/spacemacs/issues/8770)

jamesnvc commented 3 years ago

Do you know if gtk might interfere with the rendering?

Extremely possible...I think I have some GTK scaling setting, but I set this machine up quite a few years ago.

GDK_SCALE= didn't do anything, but I see I also have org.gnome.desktop.interface text-scaling-factor set. Resetting it didn't seem to do anything, but I'll try restarting and see if that changed anything...

rougier commented 3 years ago

Try https://github.com/wbolster/emacs-gsettings/blob/master/gsettings.el

jamesnvc commented 3 years ago

I have no idea what's happening on my computer now 😅

Changing the gsettings text-scaling-factor didn't seem to affect anything. Somehow after restarting, Emacs font rendering seems the same, the window decoration font is now rendering much thicker. About ready to just nuke my installation and start from scratch...

deb75 commented 3 years ago

From this comment and the following answeer, here is the result of the test : capture

It I tune the font-dpi parameter to 160, then it is nicely displayed : the box size is the same, but the text well fits in.

Also, I can keep the original font-dpi and lower the font-size to 48, I get the same nice result : capture2

rougier commented 3 years ago

What is the height of the default face?

deb75 commented 3 years ago

Hi,

The default font is "bitstream vera sans mono", I do not know how to retrieve its height.

rougier commented 3 years ago

M-: (face-attribute 'default :height)