Fix calculations and hard-code less #14
Conversation
|
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. |
It seems like a good idea to abstract this complication away so one doesn't have to worry about it when dealing with the size calculations.
Fix bugs - Fix the essential size calculation (see "(ceiling ...)") - Use the INHERIT argument of `face-attribute' Also cleanup - Simplify calculations - Use more descriptive variable names Remove variable only needed to paper over fixed bugs - `svg-tag-vertical-offset' - `svg-tag-horizontal-offset' Remove variable `svg-tag-default-line-width'. Instead support box-less tags. Move the calculations and formatting to a new function `svn-tag--make', while `svn-tag-make' now only takes care of passing along values to the former.
|
This seems to work for me! EDIT: Although to make the examples work, I do need to manually set a different |
|
I think I also have a fairly weird display set up...I have xorg conf file setting the DPI to 144, but also |
|
Thank you! Impressive, I've a lot to learn . I'll test it on my side but if no complaints, I'll merge. |
|
I'm very curious how the drawing works internally...I continue to see that omitting the |
|
Could post some screenshots showing the problem? |
|
Opening Commenting out the Commenting out |
|
What does |
|
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 |
((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"))
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 The value of (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")) |
|
From https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/font-size, it seems the default |
|
Hm,
I did note that setting the font-size to be
|
|
Interesting. I'm not sure how Emacs or librsvg computes the 1em. Do you know which svg library your emacs is using? |
Looks like librsvg2 2.48.9 (I've built it myself from git).
Reading
Yeah...might be able to compute something from |
|
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... |
|
Another interesting data point; I tried setting the font size in Firefox: Emacs: 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> |
|
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. |
|
I do see calls to |
|
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 |
|
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 |
|
Doesn't work for me:
|
|
Hi, Thanks for pointing out this. It have tested most of the examples in the demo. It works nice on However, I still have to manually increase the That said, it is not a too annoying issue because the It would be nice if you could provide a more detailed documentation or example of all the functions parameters and what they do. Also, in the last exemple ( Thanks very much for providing this library. Regards |
|
@deb75 Thansk for the report and ideas. I should have included a local repo for icons and have options to adjust font size. Can you open issues on the svg lib repo? In the end, I would like to understand the HiDPI stuff such that no tweaking would be necessary but I ran out of ideas. I need to find a windows machine to test. |
|
Using (font-dpi 96) in the code above seems to work fine here... the image and the default text seems identical. |
|
Thanks for the report. What is the code you're referring exactly (there's too much code in this issue). Also, how did you find the dpi that works ? Is i reported by Emacs somewhere? |
|
That would be your code from #14 (comment) I tried 96 (as that is the default dpi set by most stuff nowadays it seems), after both higher and lower values, but 96 did get me the exact same width in the image inserted as in the same text on the next line in my emacs buffer. It also works with 96 in my other laptop while docked on a 2560x1440 (597mm x 336mm) screen, so 108.9 ppi there... The values emacs reports is (display-pixel-height) => 1440 and (display-mm-height) => 220 (the laptop-screen itself) which gives 166 ppi, which is also the value i have set for the Xresource Xft.dpi and given to xrandr --dpi. Btw, 72.27 seems to only be used in typesetting, I have never encountered it on a computer (exept maybe in some TeX code)... 72 on the other hand, is pretty common, as used in PostScript and PDF amongst other things. On the other hand, if i just use the following code, it works just fine (note that i use the PIXEL-SIZE element of the font-query) |
|
Thansk for the report and sorry for the delay. The fact that the code you provide doesn't need dpi is nice and may solve a lot of hassle. I think it corresponds more or less to what I did for svg-lib. In the end, I wonder where the dpi problem comes from when it occurs. |
|
Hey all, after been testing a bit, I decided to remake What I dont know is if it will work for everyone, I tried with some fonts and worked for me, here is the code for the function: (defun svg-tag--make (text face padding margin radius)
(let* ((foreground (face-attribute face :foreground nil t))
(background (face-attribute face :background nil t))
(box (face-attribute face :box nil t))
(box (if (eq box 'unspecified) nil box))
(border-color (or (plist-get box :color) foreground))
(border-width (or (plist-get box :line-width) 0))
(font-family (face-attribute face :family nil 'default))
(font-weight (alist-get (face-attribute face :weight nil 'default)
svg-tag--font-weights))
(char-width (window-font-width))
(char-height (window-font-height))
(text-width (* char-width (length text)))
(tag-width (+ text-width (* padding 2)))
(box-width (+ tag-width (* border-width 2)))
(box-height (- char-height 2))
(tag-height (- box-height (* border-width 2)))
(tag-x (+ margin border-width))
(tag-y (+ border-width 1))
(svg-width (+ box-width (* margin 2)))
(text-size (* (ceiling
(* (face-attribute face :height nil 'default)
0.1))
(image-compute-scaling-factor 'auto)))
(text-x (+ tag-x padding))
(text-y (+ text-size (/ (- box-height (* (/ text-size 3.0) 4)) 2) border-width))
(svg (svg-create svg-width char-height)))
(svg-rectangle svg margin 1 box-width box-height
:fill (if box border-color background)
:rx radius)
(when box
(svg-rectangle svg
tag-x tag-y tag-width tag-height
:fill background
:rx radius))
(svg-text svg text
:font-family font-family
:font-weight font-weight
:font-size text-size
:fill foreground
:x text-x
:y text-y)
(svg-image svg :scale 1 :ascent 'center)))And here is a ss of how it looks And here with a larger font (from 120 height before to 180 here): Hope this helps Edit: just realized the width is a bit off |
|
Here is how to make the text stay at the center of the box (horizontally), we could make it by default like this or with a bool argument. This is the only thing to change to make it work: (text-x (+ tag-x
padding
(/ (- text-width
(* (/ (float char-width)
char-height)
(* (/ text-size
3.0)
4)
(length text)))
2)))And here a ss: Im now working in fixing the width, so it stays the same as the normal text |
|
Here is the fix for the width: (total-width (+ text-width (* padding 2) (* margin 2) (* border-width 2)))
(svg-width (if (= (mod total-width
char-width)
0)
total-width
(* (+ 1
(truncate (/ total-width
char-width)))
char-width)))
(box-width (- svg-width (* margin 2)))
(tag-width (- box-width (* border-width 2)))And if you want centered text with the above fix: (text-x (+ tag-x
padding
(/ (- tag-width
(* padding 2)
(* (/ (float char-width)
char-height)
(* (/ text-size
3.0)
4)
(length text)))
2)))In the centering of the text, the padding could be better called as a min-padding, since the text will separate from it so it is centered. Here is a ss: Should I make a pr to add the code or do you add it yourself @tarsius ? Since this pr is related to calculations too and I used your code as base. |
|
@aru-hackZ, open a new pr. Also, should this pr be closed, @rougier? I don't plan to work on it anymore. It seems things have moved beyond what we have here. Code-wise that is, it still seems like a place to have a conversation, so maybe it should stay open. |
Then I would like if this pr was merged, since the code I worked on was with yours as its base. |
|
Just include my commit in your pr. 😀 (But of course one could first merge this too. In any case you don't have to wait for this to be merged before opening your pr.) |
|
I think I will wait for the pr to be merged first. |
|
Thank for the code and I'll need to test it a bit further. As or merging, it might be better to merge it in svg-lib which is now part of ELPA (and also have a |
|
Hey @rougier the new calculations I made, as I said, were based on the code tarsius has on his fork, so you would need to first merge his PR, the final code I had (which also aligns the text in the center of the tag) is this: (defun svg-tag--make (text face padding margin radius)
(let* ((foreground (face-attribute face :foreground nil t))
(background (face-attribute face :background nil t))
(box (face-attribute face :box nil t))
(box (if (eq box 'unspecified) nil box))
(border-color (or (plist-get box :color) foreground))
(border-width (or (plist-get box :line-width) 0))
(font-family (face-attribute face :family nil 'default))
(font-weight (alist-get (face-attribute face :weight nil 'default)
svg-tag--font-weights))
(char-width (window-font-width))
(char-height (window-font-height))
(text-width (* char-width (length text)))
(box-height (- char-height 2))
(tag-height (- box-height (* border-width 2)))
(tag-x (+ margin border-width))
(tag-y (+ border-width 1))
(total-width (+ text-width (* padding 2) (* margin 2) (* border-width 2)))
(svg-width (if (= (mod total-width
char-width)
0)
total-width
(* (+ 1
(truncate (/ total-width
char-width)))
char-width)))
(box-width (- svg-width (* margin 2)))
(tag-width (- box-width (* border-width 2)))
(text-size (* (ceiling
(* (face-attribute face :height nil 'default)
0.1))
(image-compute-scaling-factor 'auto)))
(text-x (+ tag-x
padding
(/ (- tag-width
(* padding 2)
(* (/ (float char-width)
char-height)
(* (/ text-size
3.0)
4)
(length text)))
2)))
(text-y (+ text-size (/ (- box-height
(* (/ text-size 3.0)
4))
2)
border-width))
(svg (svg-create svg-width char-height)))
(svg-rectangle svg margin 1 box-width box-height
:fill (if box border-color background)
:rx radius)
(when box
(svg-rectangle svg
tag-x tag-y tag-width tag-height
:fill background
:rx radius))
(svg-text svg text
:font-family font-family
:font-weight font-weight
:font-size text-size
:fill foreground
:x text-x
:y text-y)
(svg-image svg :scale 1 :ascent 'center))) |
|
Thansk. It still doesn't look right on my machine:
|
|
The tag is the 'hello' that seems like it is bold? Then I would like to know your Also since i have a 1920x1080 screen i cant test if it works in bigger screens or with HiDPI so I need more input in thoae areas. |
|
I'm using "Roboto Mono" with light weight. Also, note that the tag is also a bit off (baseline doesn't match regular text). You can check svg-lib.el to see how I compute the ascent. |
|
"Nicolas P. Rougier" ***@***.***> writes:
I'm using "Roboto Mono" with light weight. Also, note that the tag is also a bit off (baseline doesn't
match regular text). You can check svg-lib.el to see how I compute the ascent.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
Is the `tag-char-width` the width of the characters *inside* the tag? If
it is, that's a better way than calculating it with what I did:
```
(* (/ (float char-width)
char-height)
(* (/ text-size
3.0)
4))
```
I will be testing it all tomorrow, because actually I didn't know you
could get that info from `font-info`, and when I understand it I may be
able to adapt my text aligning to the center in your svg-lib mode, which
seems to have better calculations than svg-tags (in the sense of it may
work in more screens)
Edit: my bad, email replies don't support MarkDown
…------
By aru
|
For now at least I have decided against this. Still, it might be interesting for future reference. Also see rougier/svg-tag-mode#14.
For now at least I have decided against this. Still, it might be interesting for future reference. Also see rougier/svg-tag-mode#14.
For now at least I have decided against this. Still, it might be interesting for future reference. Also see rougier/svg-tag-mode#14.
For now at least I have decided against this. Still, it might be interesting for future reference. Also see rougier/svg-tag-mode#14.
For now at least I have decided against this. Still, it might be interesting for future reference. Also see rougier/svg-tag-mode#14.
For now at least I have decided against this. Still, it might be interesting for future reference. Also see rougier/svg-tag-mode#14.
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:
image-compute-scaling-factorwas not taken into account. (While that says "image", the same factor applies to faces.)face-attributedid not use theinheritproperty, which lead to lots of duplication, complications and probably some bugs.I'll probably be adding
svg-tag--maketo forge (under a different name of course ;) and use it for labels. Or maybe even for refnames in magit. That's why I looked intosvg-tag-modein the first place!