unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44655: 28.0.50; Oversized SVG margin
@ 2020-11-15  8:59 Matsievskiy S.V.
  2020-11-15 17:25 ` Alan Third
  2021-11-05 19:28 ` Paul Eggert
  0 siblings, 2 replies; 22+ messages in thread
From: Matsievskiy S.V. @ 2020-11-15  8:59 UTC (permalink / raw)
  To: 44655

Inserted into overlays SVG images have oversized bottom and right
margins. This bug was
introduced somewhere between 109eb1e7e29455418b40ca00bf5dad3e61e5fc78
and a32fd9f64d06ccd07a9beaf6d6f1283f7a80edac

To reproduce the bug, use the following code:

;; MathJax image
(setq image "<svg xmlns:xlink=\"http://www.w3.org/1999/xlink\"
width=\"2.384ex\" height=\"2.676ex\" style=\"vertical-align:
-0.338ex;\" viewBox=\"0 -1006.6 1026.4 1152.1\" role=\"img\"
focusable=\"false\" xmlns=\"http://www.w3.org/2000/svg\" aria-
labelledby=\"MathJax-SVG-1-Title\">
<title id=\"MathJax-SVG-1-Title\">x^2</title>
<defs aria-hidden=\"true\">
<path stroke-width=\"1\" id=\"E1-MJMATHI-78\" d=\"M52 289Q59 331 106
386T222 442Q257 442 286 424T329 379Q371 442 430 442Q467 442 494 420T522
361Q522 332 508 314T481 292T458 288Q439 288 427 299T415 328Q415 374 465
391Q454 404 425 404Q412 404 406 402Q368 386 350 336Q290 115 290 78Q290
50 306 38T341 26Q378 26 414 59T463 140Q466 150 469 151T485 153H489Q504
153 504 145Q504 144 502 134Q486 77 440 33T333 -11Q263 -11 227 52Q186
-10 133 -10H127Q78 -10 57 16T35 71Q35 103 54 123T99 143Q142 143 142
101Q142 81 130 66T107 46T94 41L91 40Q91 39 97 36T113 29T132 26Q168 26
194 71Q203 87 217 139T245 247T261 313Q266 340 266 352Q266 380 251
392T217 404Q177 404 142 372T93 290Q91 281 88 280T72 278H58Q52 284 52
289Z\"></path>
<path stroke-width=\"1\" id=\"E1-MJMAIN-32\" d=\"M109 429Q82 429 66
447T50 491Q50 562 103 614T235 666Q326 666 387 610T449 465Q449 422 429
383T381 315T301 241Q265 210 201 149L142 93L218 92Q375 92 385 97Q392 99
409 186V189H449V186Q448 183 436 95T421 3V0H50V19V31Q50 38 56 46T86
81Q115 113 136 137Q145 147 170 174T204 211T233 244T261 278T284 308T305
340T320 369T333 401T340 431T343 464Q343 527 309 573T212 619Q179 619 154
602T119 569T109 550Q109 549 114 549Q132 549 151 535T170 489Q170 464 154
447T109 429Z\"></path>
</defs>
<g stroke=\"currentColor\" fill=\"currentColor\" stroke-width=\"0\"
transform=\"matrix(1 0 0 -1 0 0)\" aria-hidden=\"true\">
 <use xlink:href=\"#E1-MJMATHI-78\" x=\"0\" y=\"0\"></use>
 <use transform=\"scale(0.707)\" xlink:href=\"#E1-MJMAIN-32\" x=\"809\"
y=\"583\"></use>
</g>
</svg>")

(setq o (make-overlay 1 3))
(overlay-put o 'display (list (cons 'image (list :type 'svg :data image
:scale 2))))



In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.23, cairo version 1.16.0)
 of 2020-11-14 built on KILLINGMACHINE
Repository revision: 61dca6e92ac972b832e889fbeab9b6131fc896fa
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version
11.0.12008000
System Description: Debian GNU/Linux bullseye/sid

Configured using:
 'configure --with-imagemagick --prefix=/opt/emacs28 --enable-largefile
 --with-jpeg --with-png --with-json --with-threads'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO IMAGEMAGICK SOUND GPM DBUS GSETTINGS
GLIB NOTIFY INOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE HARFBUZZ
M17N_FLT LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES
THREADS LIBSYSTEMD JSON PDUMPER LCMS2

Important settings:
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-
loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-
utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset
image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-
mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit x multi-tty make-network-process
emacs)

Memory information:
((conses 16 52735 5932)
 (symbols 48 6743 1)
 (strings 32 18259 1209)
 (string-bytes 1 607244)
 (vectors 16 11426)
 (vector-slots 8 163389 9435)
 (floats 8 21 47)
 (intervals 56 192 0)
 (buffers 992 10))






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15  8:59 bug#44655: 28.0.50; Oversized SVG margin Matsievskiy S.V.
@ 2020-11-15 17:25 ` Alan Third
  2020-11-15 17:31   ` Eli Zaretskii
  2020-11-15 17:39   ` Matsievskiy S.V.
  2021-11-05 19:28 ` Paul Eggert
  1 sibling, 2 replies; 22+ messages in thread
From: Alan Third @ 2020-11-15 17:25 UTC (permalink / raw)
  To: Matsievskiy S.V.; +Cc: 44655

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

On Sun, Nov 15, 2020 at 11:59:11AM +0300, Matsievskiy S.V. wrote:
> Inserted into overlays SVG images have oversized bottom and right
> margins. This bug was
> introduced somewhere between 109eb1e7e29455418b40ca00bf5dad3e61e5fc78
> and a32fd9f64d06ccd07a9beaf6d6f1283f7a80edac

This is exactly what I expected when I made the last change: someone
would quickly find an SVG that doesn't work.

All I can think of to do is ignore the deprecation of
svg_handle_get_dimensions and continue using it. The release notes for
librsvg 2.46 state it's deprecated but still available.

Patch attached. Are we happy to continue using deprecated functions?

(I was looking through the librsvg bug tracker and it looks as though
they want to move away from depending on Cairo. Perhaps this means
things will improve for us in the future.)
-- 
Alan Third

[-- Attachment #2: 0001-Fix-SVG-display-again-bug-44655.patch --]
[-- Type: text/plain, Size: 2567 bytes --]

From 2408b2579ee9fbe831035b7784133863d91b77ab Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 15 Nov 2020 17:21:03 +0000
Subject: [PATCH] Fix SVG display again (bug#44655)

* src/image.c (svg_load_image): Fall back to a deprecated function if
we can't calculate the size of the image.
---
 src/image.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/src/image.c b/src/image.c
index 3858f3c41f..652605d9c8 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9903,27 +9903,34 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       viewbox_width = viewbox.x + viewbox.width;
       viewbox_height = viewbox.y + viewbox.height;
     }
-#else
-  /* The function used above to get the geometry of the visible area
-     of the SVG are only available in librsvg 2.46 and above, so in
-     certain circumstances this code path can result in some parts of
-     the SVG being cropped.  */
-  RsvgDimensionData dimension_data;
-
-  rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
 
-  viewbox_width = dimension_data.width;
-  viewbox_height = dimension_data.height;
+  if (viewbox_width == 0 || viewbox_height == 0)
 #endif
+  {
+    /* The functions used above to get the geometry of the visible
+       area of the SVG are only available in librsvg 2.46 and above,
+       so in certain circumstances this code path can result in some
+       parts of the SVG being cropped.
+
+       FIXME: The rsvg_handle_get_dimensions has been deprecated since
+       version 2.46 of librsvg, but is still available.  With some SVG
+       files (bug#44655) I can't find any other way to get sensible
+       dimensions from librsvg, so with 2.46 and above this is a last
+       ditch attempt.  Presumably at some point the function will be
+       removed and this will need to be changed.  */
+    RsvgDimensionData dimension_data;
+
+    rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
+
+    viewbox_width = dimension_data.width;
+    viewbox_height = dimension_data.height;
+  }
 
   if (viewbox_width == 0 || viewbox_height == 0)
     {
       /* We do not have any usable dimensions, so make some up.  The
          values below are supposedly the default values most web
          browsers use for SVGs with no set size.  */
-      /* FIXME: At this stage we should perhaps consider rendering the
-         image out to a bitmap and getting the dimensions from
-         that.  */
       viewbox_width = 300;
       viewbox_height = 150;
     }
-- 
2.26.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:25 ` Alan Third
@ 2020-11-15 17:31   ` Eli Zaretskii
  2020-11-15 17:33     ` Alan Third
  2020-11-15 17:39   ` Matsievskiy S.V.
  1 sibling, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2020-11-15 17:31 UTC (permalink / raw)
  To: Alan Third; +Cc: matsievskiysv, 44655

> Date: Sun, 15 Nov 2020 17:25:35 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: 44655@debbugs.gnu.org
> 
> All I can think of to do is ignore the deprecation of
> svg_handle_get_dimensions and continue using it. The release notes for
> librsvg 2.46 state it's deprecated but still available.
> 
> Patch attached. Are we happy to continue using deprecated functions?

Could it cause compilation failure if strict compiler switches are
used?





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:31   ` Eli Zaretskii
@ 2020-11-15 17:33     ` Alan Third
  2020-11-15 17:47       ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Third @ 2020-11-15 17:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: matsievskiysv, 44655

On Sun, Nov 15, 2020 at 07:31:04PM +0200, Eli Zaretskii wrote:
> > Date: Sun, 15 Nov 2020 17:25:35 +0000
> > From: Alan Third <alan@idiocy.org>
> > Cc: 44655@debbugs.gnu.org
> > 
> > All I can think of to do is ignore the deprecation of
> > svg_handle_get_dimensions and continue using it. The release notes for
> > librsvg 2.46 state it's deprecated but still available.
> > 
> > Patch attached. Are we happy to continue using deprecated functions?
> 
> Could it cause compilation failure if strict compiler switches are
> used?

I don't believe so. They say they're still available. What switches
should I try?
-- 
Alan Third





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:25 ` Alan Third
  2020-11-15 17:31   ` Eli Zaretskii
@ 2020-11-15 17:39   ` Matsievskiy S.V.
  2020-11-15 17:52     ` Alan Third
  1 sibling, 1 reply; 22+ messages in thread
From: Matsievskiy S.V. @ 2020-11-15 17:39 UTC (permalink / raw)
  To: Alan Third; +Cc: 44655

Is this the function 
https://developer.gnome.org/rsvg/2.50/RsvgHandle.html#rsvg-handle-get-dimensions
?
Cannot find a deprecation warning anywhere.







^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:33     ` Alan Third
@ 2020-11-15 17:47       ` Eli Zaretskii
  2020-11-15 17:56         ` Alan Third
  2020-11-16 22:34         ` Lars Ingebrigtsen
  0 siblings, 2 replies; 22+ messages in thread
From: Eli Zaretskii @ 2020-11-15 17:47 UTC (permalink / raw)
  To: Alan Third; +Cc: matsievskiysv, 44655

> Date: Sun, 15 Nov 2020 17:33:39 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: matsievskiysv@gmail.com, 44655@debbugs.gnu.org
> 
> > > Patch attached. Are we happy to continue using deprecated functions?
> > 
> > Could it cause compilation failure if strict compiler switches are
> > used?
> 
> I don't believe so. They say they're still available. What switches
> should I try?

I was thinking about -Wdeprecated.  It could cause problem if used
with -Werror.  My question was whether some distributions include
these switches under some forged environments.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:39   ` Matsievskiy S.V.
@ 2020-11-15 17:52     ` Alan Third
  2020-11-18 21:40       ` Matsievskiy S.V.
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Third @ 2020-11-15 17:52 UTC (permalink / raw)
  To: Matsievskiy S.V.; +Cc: 44655

On Sun, Nov 15, 2020 at 08:39:22PM +0300, Matsievskiy S.V. wrote:
> Is this the function 
> https://developer.gnome.org/rsvg/2.50/RsvgHandle.html#rsvg-handle-get-dimensions
> ?
> Cannot find a deprecation warning anywhere.

Yes, it's unclear what's going on, but they've marked the
RsvgDimensionData type as deprecated, and the release notes for 2.46
list the function as deprecated.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:47       ` Eli Zaretskii
@ 2020-11-15 17:56         ` Alan Third
  2020-11-16 22:34         ` Lars Ingebrigtsen
  1 sibling, 0 replies; 22+ messages in thread
From: Alan Third @ 2020-11-15 17:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: matsievskiysv, 44655

On Sun, Nov 15, 2020 at 07:47:05PM +0200, Eli Zaretskii wrote:
> > Date: Sun, 15 Nov 2020 17:33:39 +0000
> > From: Alan Third <alan@idiocy.org>
> > Cc: matsievskiysv@gmail.com, 44655@debbugs.gnu.org
> > 
> > > > Patch attached. Are we happy to continue using deprecated functions?
> > > 
> > > Could it cause compilation failure if strict compiler switches are
> > > used?
> > 
> > I don't believe so. They say they're still available. What switches
> > should I try?
> 
> I was thinking about -Wdeprecated.  It could cause problem if used
> with -Werror.  My question was whether some distributions include
> these switches under some forged environments.

Ah, I don't think I can answer that. Just trying those switches here
results in a build failure, but due to NS code, not librsvg.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:47       ` Eli Zaretskii
  2020-11-15 17:56         ` Alan Third
@ 2020-11-16 22:34         ` Lars Ingebrigtsen
  1 sibling, 0 replies; 22+ messages in thread
From: Lars Ingebrigtsen @ 2020-11-16 22:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Alan Third, matsievskiysv, 44655

Eli Zaretskii <eliz@gnu.org> writes:

> I was thinking about -Wdeprecated.  It could cause problem if used
> with -Werror.  My question was whether some distributions include
> these switches under some forged environments.

I tried adding -Wdeprecated to the Makefile and compiling (after
applying Alan's patches), and I got no warnings on Debian bullseye.

So I say go ahead and push the patch, and we'll see whether that holds
true for other systems, too, but my guess is that it'll be OK across the
board.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15 17:52     ` Alan Third
@ 2020-11-18 21:40       ` Matsievskiy S.V.
  2020-11-18 23:08         ` Alan Third
  0 siblings, 1 reply; 22+ messages in thread
From: Matsievskiy S.V. @ 2020-11-18 21:40 UTC (permalink / raw)
  To: Alan Third; +Cc: 44655

Got clarification from Gnome. rsvg_handle_get_dimensions is not
deprecated:

https://discourse.gnome.org/t/rsvg-rsvg-handle-get-dimensions-deprecation/4821







^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-18 21:40       ` Matsievskiy S.V.
@ 2020-11-18 23:08         ` Alan Third
  2020-11-20  0:18           ` Andy Moreton
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Third @ 2020-11-18 23:08 UTC (permalink / raw)
  To: Matsievskiy S.V.; +Cc: 44655-done

On Thu, Nov 19, 2020 at 12:40:00AM +0300, Matsievskiy S.V. wrote:
> Got clarification from Gnome. rsvg_handle_get_dimensions is not
> deprecated:
> 
> https://discourse.gnome.org/t/rsvg-rsvg-handle-get-dimensions-deprecation/4821

OK, thanks. The patch is pushed to master.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-18 23:08         ` Alan Third
@ 2020-11-20  0:18           ` Andy Moreton
  2020-11-20 15:02             ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Andy Moreton @ 2020-11-20  0:18 UTC (permalink / raw)
  To: 44655

On Wed 18 Nov 2020, Alan Third wrote:

> On Thu, Nov 19, 2020 at 12:40:00AM +0300, Matsievskiy S.V. wrote:
>> Got clarification from Gnome. rsvg_handle_get_dimensions is not
>> deprecated:
>> 
>> https://discourse.gnome.org/t/rsvg-rsvg-handle-get-dimensions-deprecation/4821
>
> OK, thanks. The patch is pushed to master.

This needs another fixup for the Windows build (64bit Mingw64):

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: image.o: in function `svg_load_image':
C:/emacs/git/emacs/master/src/image.c:9916: undefined reference to `rsvg_handle_get_dimensions'

It looks like the ifdefs for Windows in image.c do not do the runtime import of
rsvg_handle_get_dimensions if librvsg version < 2.46.0, but now it is
needed always.

    AndyM






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-20  0:18           ` Andy Moreton
@ 2020-11-20 15:02             ` Eli Zaretskii
  2020-11-20 15:39               ` Andy Moreton
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2020-11-20 15:02 UTC (permalink / raw)
  To: Andy Moreton; +Cc: 44655

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 20 Nov 2020 00:18:38 +0000
> 
> C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: image.o: in function `svg_load_image':
> C:/emacs/git/emacs/master/src/image.c:9916: undefined reference to `rsvg_handle_get_dimensions'
> 
> It looks like the ifdefs for Windows in image.c do not do the runtime import of
> rsvg_handle_get_dimensions if librvsg version < 2.46.0, but now it is
> needed always.

Thanks, I hope I fixed that now.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-20 15:02             ` Eli Zaretskii
@ 2020-11-20 15:39               ` Andy Moreton
  0 siblings, 0 replies; 22+ messages in thread
From: Andy Moreton @ 2020-11-20 15:39 UTC (permalink / raw)
  To: 44655

On Fri 20 Nov 2020, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Fri, 20 Nov 2020 00:18:38 +0000
>> 
>> C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe:
>> image.o: in function `svg_load_image':
>> C:/emacs/git/emacs/master/src/image.c:9916: undefined reference to `rsvg_handle_get_dimensions'
>> 
>> It looks like the ifdefs for Windows in image.c do not do the runtime import of
>> rsvg_handle_get_dimensions if librvsg version < 2.46.0, but now it is
>> needed always.
>
> Thanks, I hope I fixed that now.

Thanks Eli, build is ok again.

    AndyM






^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2020-11-15  8:59 bug#44655: 28.0.50; Oversized SVG margin Matsievskiy S.V.
  2020-11-15 17:25 ` Alan Third
@ 2021-11-05 19:28 ` Paul Eggert
  2021-11-06 12:55   ` Alan Third
  1 sibling, 1 reply; 22+ messages in thread
From: Paul Eggert @ 2021-11-05 19:28 UTC (permalink / raw)
  To: 44655

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

I ran into a problem building Emacs with Fedora 35 (released 3 days ago) 
related to the fix for bug#44655, and installed the attached to try to 
work around the issue (which is due to the deprecation of 
rsvg_handle_get_dimensions in librsvg 2.52.0 (2021-09-15)).

[-- Attachment #2: 0001-rsvg_handle_get_dimensions-is-deprecated-in-2.52.0.patch --]
[-- Type: text/x-patch, Size: 4460 bytes --]

From 133026c362eb27191d992fbf35727550804bfc96 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 5 Nov 2021 11:51:46 -0700
Subject: [PATCH] rsvg_handle_get_dimensions is deprecated in 2.52.0
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In Bug#44655#32 (2020-11-19) it was reported that
rsvg_handle_get_dimensions was not deprecated.  However, it became
deprecated in librsvg 2.52.0 (2021-09-15), and because of this Emacs
builds with --enable-gcc-warnings fail in Fedora 35 (2025-11-02)
with the diagnostic “‘rsvg_handle_get_dimensions’ is deprecated:
Use 'rsvg_handle_get_intrinsic_size_in_pixels' instead
[-Werror=deprecated-declarations]”.
* src/image.c (rsvg_handle_get_dimensions): Define as a DLL
function only if < librsvg 2.46.0, since it’s not used
in 2.46.0 or later.
(svg_load_image): Use rsvg_handle_get_dimensions only if librsvg <
2.46.0, since it isn’t needed if >= 2.46.0.
---
 src/image.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/image.c b/src/image.c
index 102f3a1c3a..da6cbba74d 100644
--- a/src/image.c
+++ b/src/image.c
@@ -9974,14 +9974,15 @@ DEF_DLL_FN (void, rsvg_handle_get_intrinsic_dimensions,
 DEF_DLL_FN (gboolean, rsvg_handle_get_geometry_for_layer,
 	    (RsvgHandle *, const char *, const RsvgRectangle *,
 	     RsvgRectangle *, RsvgRectangle *, GError **));
+#  else
+DEF_DLL_FN (void, rsvg_handle_get_dimensions,
+	    (RsvgHandle *, RsvgDimensionData *));
 #  endif
 
 #  if LIBRSVG_CHECK_VERSION (2, 48, 0)
 DEF_DLL_FN (gboolean, rsvg_handle_set_stylesheet,
 	    (RsvgHandle *, const guint8 *, gsize, GError **));
 #  endif
-DEF_DLL_FN (void, rsvg_handle_get_dimensions,
-	    (RsvgHandle *, RsvgDimensionData *));
 DEF_DLL_FN (GdkPixbuf *, rsvg_handle_get_pixbuf, (RsvgHandle *));
 DEF_DLL_FN (int, gdk_pixbuf_get_width, (const GdkPixbuf *));
 DEF_DLL_FN (int, gdk_pixbuf_get_height, (const GdkPixbuf *));
@@ -10032,11 +10033,12 @@ init_svg_functions (void)
 #if LIBRSVG_CHECK_VERSION (2, 46, 0)
   LOAD_DLL_FN (library, rsvg_handle_get_intrinsic_dimensions);
   LOAD_DLL_FN (library, rsvg_handle_get_geometry_for_layer);
+#else
+  LOAD_DLL_FN (library, rsvg_handle_get_dimensions);
 #endif
 #if LIBRSVG_CHECK_VERSION (2, 48, 0)
   LOAD_DLL_FN (library, rsvg_handle_set_stylesheet);
 #endif
-  LOAD_DLL_FN (library, rsvg_handle_get_dimensions);
   LOAD_DLL_FN (library, rsvg_handle_get_pixbuf);
 
   LOAD_DLL_FN (gdklib, gdk_pixbuf_get_width);
@@ -10074,8 +10076,9 @@ init_svg_functions (void)
 #  if LIBRSVG_CHECK_VERSION (2, 46, 0)
 #   undef rsvg_handle_get_intrinsic_dimensions
 #   undef rsvg_handle_get_geometry_for_layer
+#  else
+#   undef rsvg_handle_get_dimensions
 #  endif
-#  undef rsvg_handle_get_dimensions
 #  if LIBRSVG_CHECK_VERSION (2, 48, 0)
 #   undef rsvg_handle_set_stylesheet
 #  endif
@@ -10110,8 +10113,9 @@ init_svg_functions (void)
 	fn_rsvg_handle_get_intrinsic_dimensions
 #   define rsvg_handle_get_geometry_for_layer	\
 	fn_rsvg_handle_get_geometry_for_layer
+#  else
+#   define rsvg_handle_get_dimensions fn_rsvg_handle_get_dimensions
 #  endif
-#  define rsvg_handle_get_dimensions fn_rsvg_handle_get_dimensions
 #  if LIBRSVG_CHECK_VERSION (2, 48, 0)
 #   define rsvg_handle_set_stylesheet fn_rsvg_handle_set_stylesheet
 #  endif
@@ -10386,21 +10390,13 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       viewbox_width = viewbox.x + viewbox.width;
       viewbox_height = viewbox.y + viewbox.height;
     }
-
-  if (viewbox_width == 0 || viewbox_height == 0)
+#else
+  /* In librsvg before 2.46.0, guess the viewbox from the image dimensions.  */
+  RsvgDimensionData dimension_data;
+  rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
+  viewbox_width = dimension_data.width;
+  viewbox_height = dimension_data.height;
 #endif
-  {
-    /* The functions used above to get the geometry of the visible
-       area of the SVG are only available in librsvg 2.46 and above,
-       so in certain circumstances this code path can result in some
-       parts of the SVG being cropped.  */
-    RsvgDimensionData dimension_data;
-
-    rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
-
-    viewbox_width = dimension_data.width;
-    viewbox_height = dimension_data.height;
-  }
 
   compute_image_size (viewbox_width, viewbox_height, img,
                       &width, &height);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-11-05 19:28 ` Paul Eggert
@ 2021-11-06 12:55   ` Alan Third
  2021-12-03 18:45     ` Paul Eggert
  0 siblings, 1 reply; 22+ messages in thread
From: Alan Third @ 2021-11-06 12:55 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 44655

On Fri, Nov 05, 2021 at 12:28:37PM -0700, Paul Eggert wrote:
> I ran into a problem building Emacs with Fedora 35 (released 3 days ago)
> related to the fix for bug#44655, and installed the attached to try to work
> around the issue (which is due to the deprecation of
> rsvg_handle_get_dimensions in librsvg 2.52.0 (2021-09-15)).

There seems to be a lot of churn in the librsvg API at the moment, and
their documentation isn't keeping up as it still doesn't mark
rsvg_handle_get_dimensions as deprecated.

It appears they've introduced rsvg_handle_get_intrinsic_size_in_pixels
which does the same thing (but better) than my sizing code, but anyone
using librsvg 2.46-2.52, which is probably most people for now, can't
use it and probably don't want to use the old scheme, so I expect
there's no point replacing my code at the moment.

> -  if (viewbox_width == 0 || viewbox_height == 0)
> +#else
> +  /* In librsvg before 2.46.0, guess the viewbox from the image dimensions.  */
> +  RsvgDimensionData dimension_data;
> +  rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
> +  viewbox_width = dimension_data.width;
> +  viewbox_height = dimension_data.height;
>  #endif
> -  {
> -    /* The functions used above to get the geometry of the visible
> -       area of the SVG are only available in librsvg 2.46 and above,
> -       so in certain circumstances this code path can result in some
> -       parts of the SVG being cropped.  */
> -    RsvgDimensionData dimension_data;
> -
> -    rsvg_handle_get_dimensions (rsvg_handle, &dimension_data);
> -
> -    viewbox_width = dimension_data.width;
> -    viewbox_height = dimension_data.height;
> -  }
>  
>    compute_image_size (viewbox_width, viewbox_height, img,
>                        &width, &height);

The fall through is in place because if the image size is defined in
units we don't know (e.g. % or ex) then we end up with no idea what
size it should be.

Perhaps what we should do is move the final "else" section of the
previous code block (where rsvg_handle_get_geometry_for_layer is
called) into its own block which is executed

  if (viewbox_width == 0 || viewbox_height == 0)

instead of only in the case where rsvg_handle_get_intrinsic_dimensions
fails to return any dimensions. That way we should have *some*
dimensions without having to call rsvg_handle_get_dimensions.
-- 
Alan Third





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-11-06 12:55   ` Alan Third
@ 2021-12-03 18:45     ` Paul Eggert
  2021-12-04 10:46       ` Alan Third
  2021-12-04 11:00       ` Arash Esbati
  0 siblings, 2 replies; 22+ messages in thread
From: Paul Eggert @ 2021-12-03 18:45 UTC (permalink / raw)
  To: Alan Third; +Cc: 44655

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

On 11/6/21 05:55, Alan Third wrote:

> There seems to be a lot of churn in the librsvg API at the moment, and
> their documentation isn't keeping up as it still doesn't mark
> rsvg_handle_get_dimensions as deprecated.
> 
> It appears they've introduced rsvg_handle_get_intrinsic_size_in_pixels
> which does the same thing (but better) than my sizing code, but anyone
> using librsvg 2.46-2.52, which is probably most people for now, can't
> use it and probably don't want to use the old scheme, so I expect
> there's no point replacing my code at the moment.

If it improves on your code then let's try using it for bleeding-edge 
librsvg (2.52.0+).


> Perhaps what we should do is move the final "else" section of the
> previous code block (where rsvg_handle_get_geometry_for_layer is
> called) into its own block which is executed
> 
>    if (viewbox_width == 0 || viewbox_height == 0)
> 
> instead of only in the case where rsvg_handle_get_intrinsic_dimensions
> fails to return any dimensions. That way we should have *some*
> dimensions without having to call rsvg_handle_get_dimensions.

I installed a patch into master to do that, along with other patches to 
try using svg_handle_get_intrinsic_size_in_pixels with bleeding-edge 
librsvg, and to catch some potential integer overflow problems I noticed 
while doing all this (see attached).

[-- Attachment #2: 0001-More-robust-svg_load_image-fallback.patch --]
[-- Type: text/x-patch, Size: 1092 bytes --]

From 9a4a14d1e838e85da2a480340e0a9c8109faee45 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 3 Dec 2021 09:47:22 -0800
Subject: [PATCH 1/4] More-robust svg_load_image fallback

Suggested by Alan Third (Bug#44655#56).
* src/image.c (svg_load_image): Fall back on
rsvg_handle_get_geometry_for_layer if the
rsvg_handle_get_intrinsic_dimensions computations yielded unusable
viewbox width and height, instead of falling back only if
rsvg_handle_get_intrinsic_dimensions did not report image width
and height, or did not report a viewbox.
---
 src/image.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/image.c b/src/image.c
index f2597f529d..f13304912c 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10484,6 +10484,9 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
       viewbox_height = viewbox.height;
     }
   else
+    viewbox_width = viewbox_height = 0;
+
+  if (viewbox_width == 0 || viewbox_height == 0)
     {
       /* We haven't found a usable set of sizes, so try working out
          the visible area.  */
-- 
2.32.0


[-- Attachment #3: 0002-Simplify-svg_load_image.patch --]
[-- Type: text/x-patch, Size: 1244 bytes --]

From 24303f19f3fdd9971014b26b2b8ee7043a055fdc Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 3 Dec 2021 09:47:22 -0800
Subject: [PATCH 2/4] Simplify svg_load_image

* src/image.c (svg_load_image): Simplify slightly.
---
 src/image.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/image.c b/src/image.c
index f13304912c..1db2b78ad5 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10469,14 +10469,12 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   else if (has_width && has_viewbox)
     {
       viewbox_width = svg_css_length_to_pixels (iwidth, dpi, img->face_font_size);
-      viewbox_height = svg_css_length_to_pixels (iwidth, dpi, img->face_font_size)
-        * viewbox.height / viewbox.width;
+      viewbox_height = viewbox_width * viewbox.height / viewbox.width;
     }
   else if (has_height && has_viewbox)
     {
       viewbox_height = svg_css_length_to_pixels (iheight, dpi, img->face_font_size);
-      viewbox_width = svg_css_length_to_pixels (iheight, dpi, img->face_font_size)
-        * viewbox.width / viewbox.height;
+      viewbox_width = viewbox_height * viewbox.width / viewbox.height;
     }
   else if (has_viewbox)
     {
-- 
2.32.0


[-- Attachment #4: 0003-Improve-overflow-checking-in-svg_load_image.patch --]
[-- Type: text/x-patch, Size: 6774 bytes --]

From 57eaf033d71d755009ffc1e9b552201143129218 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 3 Dec 2021 09:47:22 -0800
Subject: [PATCH 3/4] Improve overflow checking in svg_load_image
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/image.c: Include math.h, for lrint.
(scale_image_size, compute_image_size): Use ‘double’, not ‘int’
for image size args, since librsvg uses ‘double’ for pixel counts.
(scale_image_size): Use ceil instead of rounding, to avoid
discarding fractional SVG pixels.  Divisor and multiplier are now
double instead of int, for better portability to librsvg
functions with fractional pixel sizes.
(image_get_dimension, compute_image_size, svg_load_image):
Be more careful about ignoring, rejecting or clipping scale
factors or sizes that are out of integer range.
(compute_image_size): Don’t bother to calculate :max-width if
:width is specified, and likewise for :max-height and :height.
---
 src/image.c | 76 +++++++++++++++++++++++++----------------------------
 1 file changed, 36 insertions(+), 40 deletions(-)

diff --git a/src/image.c b/src/image.c
index 1db2b78ad5..14e9944db2 100644
--- a/src/image.c
+++ b/src/image.c
@@ -31,6 +31,7 @@ Copyright (C) 1989, 1992-2021 Free Software Foundation, Inc.
 
 #include <setjmp.h>
 
+#include <math.h>
 #include <stdint.h>
 #include <c-ctype.h>
 #include <flexmember.h>
@@ -2016,14 +2017,16 @@ postprocess_image (struct frame *f, struct image *img)
    safely rounded and clipped to int range.  */
 
 static int
-scale_image_size (int size, size_t divisor, size_t multiplier)
+scale_image_size (int size, double divisor, double multiplier)
 {
   if (divisor != 0)
     {
-      double s = size;
-      double scaled = s * multiplier / divisor + 0.5;
+      double scaled = size * multiplier / divisor;
       if (scaled < INT_MAX)
-	return scaled;
+	{
+	  /* Use ceil, as rounding can discard fractional SVG pixels.  */
+	  return ceil (scaled);
+	}
     }
   return INT_MAX;
 }
@@ -2044,84 +2047,77 @@ image_get_dimension (struct image *img, Lisp_Object symbol)
   if (FIXNATP (value))
     return min (XFIXNAT (value), INT_MAX);
   if (CONSP (value) && NUMBERP (CAR (value)) && EQ (Qem, CDR (value)))
-    return min (img->face_font_size * XFLOATINT (CAR (value)), INT_MAX);
+    return scale_image_size (img->face_font_size, 1, XFLOATINT (CAR (value)));
 
   return -1;
 }
 
 /* Compute the desired size of an image with native size WIDTH x HEIGHT.
-   Use SPEC to deduce the size.  Store the desired size into
+   Use IMG to deduce the size.  Store the desired size into
    *D_WIDTH x *D_HEIGHT.  Store -1 x -1 if the native size is OK.  */
 static void
-compute_image_size (size_t width, size_t height,
+compute_image_size (double width, double height,
 		    struct image *img,
 		    int *d_width, int *d_height)
 {
-  Lisp_Object value;
-  int int_value;
-  int desired_width = -1, desired_height = -1, max_width = -1, max_height = -1;
   double scale = 1;
-
-  value = image_spec_value (img->spec, QCscale, NULL);
+  Lisp_Object value = image_spec_value (img->spec, QCscale, NULL);
   if (NUMBERP (value))
-    scale = XFLOATINT (value);
-
-  int_value = image_get_dimension (img, QCmax_width);
-  if (int_value >= 0)
-    max_width = int_value;
-
-  int_value = image_get_dimension (img, QCmax_height);
-  if (int_value >= 0)
-    max_height = int_value;
+    {
+      double dval = XFLOATINT (value);
+      if (0 <= dval)
+	scale = dval;
+    }
 
   /* If width and/or height is set in the display spec assume we want
      to scale to those values.  If either h or w is unspecified, the
      unspecified should be calculated from the specified to preserve
      aspect ratio.  */
-  int_value = image_get_dimension (img, QCwidth);
-  if (int_value >= 0)
+  int desired_width = image_get_dimension (img, QCwidth), max_width;
+  if (desired_width < 0)
+    max_width = image_get_dimension (img, QCmax_width);
+  else
     {
-      desired_width = int_value * scale;
+      desired_width = scale_image_size (desired_width, 1, scale);
       /* :width overrides :max-width. */
       max_width = -1;
     }
 
-  int_value = image_get_dimension (img, QCheight);
-  if (int_value >= 0)
+  int desired_height = image_get_dimension (img, QCheight), max_height;
+  if (desired_height < 0)
+    max_height = image_get_dimension (img, QCmax_height);
+  else
     {
-      desired_height = int_value * scale;
+      desired_height = scale_image_size (desired_height, 1, scale);
       /* :height overrides :max-height. */
       max_height = -1;
     }
 
   /* If we have both width/height set explicitly, we skip past all the
      aspect ratio-preserving computations below. */
-  if (desired_width != -1 && desired_height != -1)
+  if (0 <= desired_width && 0 <= desired_height)
     goto out;
 
-  width = width * scale;
-  height = height * scale;
-
-  if (desired_width != -1)
+  if (0 <= desired_width)
     /* Width known, calculate height. */
     desired_height = scale_image_size (desired_width, width, height);
-  else if (desired_height != -1)
+  else if (0 <= desired_height)
     /* Height known, calculate width. */
     desired_width = scale_image_size (desired_height, height, width);
   else
     {
-      desired_width = width;
-      desired_height = height;
+      desired_width = scale_image_size (width, 1, scale);
+      desired_height = scale_image_size (height, 1, scale);
     }
 
-  if (max_width != -1 && desired_width > max_width)
+  if (0 <= max_width && max_width < desired_width)
     {
       /* The image is wider than :max-width. */
       desired_width = max_width;
       desired_height = scale_image_size (desired_width, width, height);
     }
 
-  if (max_height != -1 && desired_height > max_height)
+  if (0 <= max_height && max_height < desired_height)
     {
       /* The image is higher than :max-height. */
       desired_height = max_height;
@@ -10484,7 +10480,7 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   else
     viewbox_width = viewbox_height = 0;
 
-  if (viewbox_width == 0 || viewbox_height == 0)
+  if (! (0 < viewbox_width && 0 < viewbox_height))
     {
       /* We haven't found a usable set of sizes, so try working out
          the visible area.  */
@@ -10505,8 +10501,8 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
   compute_image_size (viewbox_width, viewbox_height, img,
                       &width, &height);
 
-  width *= FRAME_SCALE_FACTOR (f);
-  height *= FRAME_SCALE_FACTOR (f);
+  width = scale_image_size (width, 1, FRAME_SCALE_FACTOR (f));
+  height = scale_image_size (height, 1, FRAME_SCALE_FACTOR (f));
 
   if (! check_image_size (f, width, height))
     {
-- 
2.32.0


[-- Attachment #5: 0004-Prefer-rsvg_handle_get_intrinsic_size_in_pixels.patch --]
[-- Type: text/x-patch, Size: 6597 bytes --]

From 89d494e7a26e6031f1ae35e8590793e413b22641 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 3 Dec 2021 09:47:22 -0800
Subject: [PATCH 4/4] Prefer rsvg_handle_get_intrinsic_size_in_pixels

Use rsvg_handle_get_intrinsic_size_in_pixels if available,
as this is simpler and better than what we were doing.
From a comment by by Alan Third (Bug#44655#56).
* src/image.c (init_svg_functions): Arrange for the new function.
(svg_load_image): Prefer the results of
rsvg_handle_get_intrinsic_size_in_pixels if available.
---
 src/image.c | 114 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 74 insertions(+), 40 deletions(-)

diff --git a/src/image.c b/src/image.c
index 14e9944db2..c08ee92287 100644
--- a/src/image.c
+++ b/src/image.c
@@ -10066,6 +10066,10 @@ DEF_DLL_FN (gboolean, rsvg_handle_close, (RsvgHandle *, GError **));
 DEF_DLL_FN (void, rsvg_handle_set_dpi_x_y,
 	    (RsvgHandle * handle, double dpi_x, double dpi_y));
 
+#  if LIBRSVG_CHECK_VERSION (2, 52, 1)
+DEF_DLL_FN (void, rsvg_handle_get_intrinsic_size_in_pixels,
+            (RsvgHandle *, gdouble *, gdouble *));
+#  endif
 #  if LIBRSVG_CHECK_VERSION (2, 46, 0)
 DEF_DLL_FN (void, rsvg_handle_get_intrinsic_dimensions,
             (RsvgHandle *, gboolean *, RsvgLength *, gboolean *,
@@ -10129,6 +10133,9 @@ init_svg_functions (void)
   LOAD_DLL_FN (library, rsvg_handle_close);
 #endif
   LOAD_DLL_FN (library, rsvg_handle_set_dpi_x_y);
+#if LIBRSVG_CHECK_VERSION (2, 52, 1)
+  LOAD_DLL_FN (library, rsvg_handle_get_intrinsic_size_in_pixels);
+#endif
 #if LIBRSVG_CHECK_VERSION (2, 46, 0)
   LOAD_DLL_FN (library, rsvg_handle_get_intrinsic_dimensions);
   LOAD_DLL_FN (library, rsvg_handle_get_geometry_for_layer);
@@ -10172,6 +10179,9 @@ init_svg_functions (void)
 #  undef g_clear_error
 #  undef g_object_unref
 #  undef g_type_init
+#  if LIBRSVG_CHECK_VERSION (2, 52, 1)
+#   undef rsvg_handle_get_intrinsic_size_in_pixels
+#  endif
 #  if LIBRSVG_CHECK_VERSION (2, 46, 0)
 #   undef rsvg_handle_get_intrinsic_dimensions
 #   undef rsvg_handle_get_geometry_for_layer
@@ -10207,6 +10217,10 @@ init_svg_functions (void)
 #  if ! GLIB_CHECK_VERSION (2, 36, 0)
 #   define g_type_init fn_g_type_init
 #  endif
+#  if LIBRSVG_CHECK_VERSION (2, 52, 1)
+#   define rsvg_handle_get_intrinsic_size_in_pixels \
+	fn_rsvg_handle_get_intrinsic_size_in_pixels
+#  endif
 #  if LIBRSVG_CHECK_VERSION (2, 46, 0)
 #   define rsvg_handle_get_intrinsic_dimensions \
 	fn_rsvg_handle_get_intrinsic_dimensions
@@ -10444,51 +10458,71 @@ svg_load_image (struct frame *f, struct image *img, char *contents,
 
   /* Get the image dimensions.  */
 #if LIBRSVG_CHECK_VERSION (2, 46, 0)
-  RsvgRectangle zero_rect, viewbox, out_logical_rect;
-
-  /* Try the intrinsic dimensions first.  */
-  gboolean has_width, has_height, has_viewbox;
-  RsvgLength iwidth, iheight;
-  double dpi = FRAME_DISPLAY_INFO (f)->resx;
-
-  rsvg_handle_get_intrinsic_dimensions (rsvg_handle,
-                                        &has_width, &iwidth,
-                                        &has_height, &iheight,
-                                        &has_viewbox, &viewbox);
+  gdouble gviewbox_width, gviewbox_height;
+  gboolean has_viewbox = FALSE;
+# if LIBRSVG_CHECK_VERSION (2, 52, 1)
+  has_viewbox = rsvg_handle_get_intrinsic_size_in_pixels (rsvg_handle,
+							  &gviewbox_width,
+							  &gviewbox_height);
+# endif
 
-  if (has_width && has_height)
+  if (has_viewbox)
     {
-      /* Success!  We can use these values directly.  */
-      viewbox_width = svg_css_length_to_pixels (iwidth, dpi, img->face_font_size);
-      viewbox_height = svg_css_length_to_pixels (iheight, dpi, img->face_font_size);
-    }
-  else if (has_width && has_viewbox)
-    {
-      viewbox_width = svg_css_length_to_pixels (iwidth, dpi, img->face_font_size);
-      viewbox_height = viewbox_width * viewbox.height / viewbox.width;
-    }
-  else if (has_height && has_viewbox)
-    {
-      viewbox_height = svg_css_length_to_pixels (iheight, dpi, img->face_font_size);
-      viewbox_width = viewbox_height * viewbox.width / viewbox.height;
-    }
-  else if (has_viewbox)
-    {
-      viewbox_width = viewbox.width;
-      viewbox_height = viewbox.height;
+      viewbox_width = gviewbox_width;
+      viewbox_height = gviewbox_height;
     }
   else
-    viewbox_width = viewbox_height = 0;
-
-  if (! (0 < viewbox_width && 0 < viewbox_height))
     {
-      /* We haven't found a usable set of sizes, so try working out
-         the visible area.  */
-      rsvg_handle_get_geometry_for_layer (rsvg_handle, NULL,
-                                          &zero_rect, &viewbox,
-                                          &out_logical_rect, NULL);
-      viewbox_width = viewbox.x + viewbox.width;
-      viewbox_height = viewbox.y + viewbox.height;
+      RsvgRectangle zero_rect, viewbox, out_logical_rect;
+
+      /* Try the intrinsic dimensions first.  */
+      gboolean has_width, has_height;
+      RsvgLength iwidth, iheight;
+      double dpi = FRAME_DISPLAY_INFO (f)->resx;
+
+      rsvg_handle_get_intrinsic_dimensions (rsvg_handle,
+					    &has_width, &iwidth,
+					    &has_height, &iheight,
+					    &has_viewbox, &viewbox);
+
+      if (has_width && has_height)
+	{
+	  /* Success!  We can use these values directly.  */
+	  viewbox_width = svg_css_length_to_pixels (iwidth, dpi,
+						    img->face_font_size);
+	  viewbox_height = svg_css_length_to_pixels (iheight, dpi,
+						     img->face_font_size);
+	}
+      else if (has_width && has_viewbox)
+	{
+	  viewbox_width = svg_css_length_to_pixels (iwidth, dpi,
+						    img->face_font_size);
+	  viewbox_height = viewbox_width * viewbox.height / viewbox.width;
+	}
+      else if (has_height && has_viewbox)
+	{
+	  viewbox_height = svg_css_length_to_pixels (iheight, dpi,
+						     img->face_font_size);
+	  viewbox_width = viewbox_height * viewbox.width / viewbox.height;
+	}
+      else if (has_viewbox)
+	{
+	  viewbox_width = viewbox.width;
+	  viewbox_height = viewbox.height;
+	}
+      else
+	viewbox_width = viewbox_height = 0;
+
+      if (! (0 < viewbox_width && 0 < viewbox_height))
+	{
+	  /* We haven't found a usable set of sizes, so try working out
+	     the visible area.  */
+	  rsvg_handle_get_geometry_for_layer (rsvg_handle, NULL,
+					      &zero_rect, &viewbox,
+					      &out_logical_rect, NULL);
+	  viewbox_width = viewbox.x + viewbox.width;
+	  viewbox_height = viewbox.y + viewbox.height;
+	}
     }
 #else
   /* In librsvg before 2.46.0, guess the viewbox from the image dimensions.  */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-12-03 18:45     ` Paul Eggert
@ 2021-12-04 10:46       ` Alan Third
  2021-12-04 11:00       ` Arash Esbati
  1 sibling, 0 replies; 22+ messages in thread
From: Alan Third @ 2021-12-04 10:46 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 44655

On Fri, Dec 03, 2021 at 10:45:55AM -0800, Paul Eggert wrote:
> On 11/6/21 05:55, Alan Third wrote:
> 
> > There seems to be a lot of churn in the librsvg API at the moment, and
> > their documentation isn't keeping up as it still doesn't mark
> > rsvg_handle_get_dimensions as deprecated.
> > 
> > It appears they've introduced rsvg_handle_get_intrinsic_size_in_pixels
> > which does the same thing (but better) than my sizing code, but anyone
> > using librsvg 2.46-2.52, which is probably most people for now, can't
> > use it and probably don't want to use the old scheme, so I expect
> > there's no point replacing my code at the moment.
> 
> If it improves on your code then let's try using it for bleeding-edge
> librsvg (2.52.0+).
> 
> 
> > Perhaps what we should do is move the final "else" section of the
> > previous code block (where rsvg_handle_get_geometry_for_layer is
> > called) into its own block which is executed
> > 
> >    if (viewbox_width == 0 || viewbox_height == 0)
> > 
> > instead of only in the case where rsvg_handle_get_intrinsic_dimensions
> > fails to return any dimensions. That way we should have *some*
> > dimensions without having to call rsvg_handle_get_dimensions.
> 
> I installed a patch into master to do that, along with other patches to try
> using svg_handle_get_intrinsic_size_in_pixels with bleeding-edge librsvg,
> and to catch some potential integer overflow problems I noticed while doing
> all this (see attached).

This all looks good to me. Thanks!
-- 
Alan Third





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-12-03 18:45     ` Paul Eggert
  2021-12-04 10:46       ` Alan Third
@ 2021-12-04 11:00       ` Arash Esbati
  2021-12-04 11:43         ` Eli Zaretskii
  1 sibling, 1 reply; 22+ messages in thread
From: Arash Esbati @ 2021-12-04 11:00 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Alan Third, 44655

Paul Eggert <eggert@cs.ucla.edu> writes:

> I installed a patch into master to do that, along with other patches
> to try using svg_handle_get_intrinsic_size_in_pixels with
> bleeding-edge librsvg, and to catch some potential integer overflow
> problems I noticed while doing all this (see attached).

I can't build Emacs now with this error:

    image.c: In function 'svg_load_image':
    image.c:10464:15: error: void value not ignored as it ought to be
    10464 |   has_viewbox = rsvg_handle_get_intrinsic_size_in_pixels (rsvg_handle,
          |               ^

This is on Win10 with Msys2, gcc 11.2 and librsvg 2.52.4.

Best, Arash





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-12-04 11:00       ` Arash Esbati
@ 2021-12-04 11:43         ` Eli Zaretskii
  2021-12-04 12:05           ` Arash Esbati
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-12-04 11:43 UTC (permalink / raw)
  To: Arash Esbati; +Cc: alan, eggert, 44655

> From: Arash Esbati <arash@gnu.org>
> Date: Sat, 04 Dec 2021 12:00:29 +0100
> Cc: Alan Third <alan@idiocy.org>, 44655@debbugs.gnu.org
> 
>     image.c: In function 'svg_load_image':
>     image.c:10464:15: error: void value not ignored as it ought to be
>     10464 |   has_viewbox = rsvg_handle_get_intrinsic_size_in_pixels (rsvg_handle,
>           |               ^
> 
> This is on Win10 with Msys2, gcc 11.2 and librsvg 2.52.4.

I hope I fixed this now.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-12-04 11:43         ` Eli Zaretskii
@ 2021-12-04 12:05           ` Arash Esbati
  2021-12-04 16:07             ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: Arash Esbati @ 2021-12-04 12:05 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: alan, eggert, 44655

Eli Zaretskii <eliz@gnu.org> writes:

> I hope I fixed this now.

Yes, thanks for the quick fix.





^ permalink raw reply	[flat|nested] 22+ messages in thread

* bug#44655: 28.0.50; Oversized SVG margin
  2021-12-04 12:05           ` Arash Esbati
@ 2021-12-04 16:07             ` Eli Zaretskii
  0 siblings, 0 replies; 22+ messages in thread
From: Eli Zaretskii @ 2021-12-04 16:07 UTC (permalink / raw)
  To: Arash Esbati; +Cc: alan, eggert, 44655

> From: Arash Esbati <arash@gnu.org>
> Cc: eggert@cs.ucla.edu,  alan@idiocy.org,  44655@debbugs.gnu.org
> Date: Sat, 04 Dec 2021 13:05:03 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I hope I fixed this now.
> 
> Yes, thanks for the quick fix.

Thanks for testing the fix.





^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-12-04 16:07 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-15  8:59 bug#44655: 28.0.50; Oversized SVG margin Matsievskiy S.V.
2020-11-15 17:25 ` Alan Third
2020-11-15 17:31   ` Eli Zaretskii
2020-11-15 17:33     ` Alan Third
2020-11-15 17:47       ` Eli Zaretskii
2020-11-15 17:56         ` Alan Third
2020-11-16 22:34         ` Lars Ingebrigtsen
2020-11-15 17:39   ` Matsievskiy S.V.
2020-11-15 17:52     ` Alan Third
2020-11-18 21:40       ` Matsievskiy S.V.
2020-11-18 23:08         ` Alan Third
2020-11-20  0:18           ` Andy Moreton
2020-11-20 15:02             ` Eli Zaretskii
2020-11-20 15:39               ` Andy Moreton
2021-11-05 19:28 ` Paul Eggert
2021-11-06 12:55   ` Alan Third
2021-12-03 18:45     ` Paul Eggert
2021-12-04 10:46       ` Alan Third
2021-12-04 11:00       ` Arash Esbati
2021-12-04 11:43         ` Eli Zaretskii
2021-12-04 12:05           ` Arash Esbati
2021-12-04 16:07             ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).