bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Visuwesh]

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

Tags: patch

Severity: wishlist

Attached patch makes mupdf produce svg images rather than png when svg
support is available.  This makes a noticeable improvement in image
quality when zooming in.  This also improves epub, odt, docx,
etc. rendering since they also end up using mupdf.
I'm not sure if this is NEWS worthy though.

I've also attached two images before.png and after.png to illustrate the
difference in rendering before and after applying this patch.


[-- Attachment #2: before.png --]
[-- Type: image/png, Size: 137156 bytes --]

[-- Attachment #3: after.png --]
[-- Type: image/png, Size: 97579 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-docview-Use-svg-images-when-using-mupdf-for-conversi.patch --]
[-- Type: text/patch, Size: 2964 bytes --]

From 3b01128362e28317dfaa738a8c5bbf0a02a22d8f Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Sat, 24 Sep 2022 15:40:27 +0530
Subject: [PATCH] docview: Use svg images when using mupdf for conversion

* lisp/doc-view.el (doc-view-mupdf-use-svg, doc-view-svg-background)
(doc-view-svg-foreground): New user options.
(doc-view-insert-image): Add :background and :foreground image
attributes when display svg images.
(doc-view-set-up-single-converter): Produce svg images when using
mupdf.
---
 lisp/doc-view.el | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index aa0f9fd383..fbd1427946 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -209,6 +209,23 @@ doc-view-pdf->png-converter-function
           function)
   :version "24.4")
 
+(defcustom doc-view-mupdf-use-svg (image-type-available-p 'svg)
+  "Whether to use svg images for PDF files."
+  :type 'boolean
+  :version "29.1")
+
+(defcustom doc-view-svg-background "white"
+  "Background color for svg images.
+See `doc-view-mupdf-use-svg'."
+  :type 'color
+  :version "29.1")
+
+(defcustom doc-view-svg-foreground "black"
+  "Foreground color for svg images.
+See `doc-view-mupdf-use-svg'."
+  :type 'color
+  :version "29.1")
+
 (defcustom doc-view-ghostscript-options
   '("-dSAFER" ;; Avoid security problems when rendering files from untrusted
     ;; sources.
@@ -1562,6 +1579,9 @@ doc-view-insert-image
 			    (setq args `(,@args :width ,doc-view-image-width)))
                           (unless (member :transform-smoothing args)
                             (setq args `(,@args :transform-smoothing t)))
+                          (when (eq doc-view--image-type 'svg)
+                            (setq args `(,@args :background ,doc-view-svg-background
+                                               :foreground ,doc-view-svg-foreground)))
 			  (apply #'create-image file doc-view--image-type nil args))))
 	     (slice (doc-view-current-slice))
 	     (img-width (and image (car (image-size image))))
@@ -1983,7 +2003,11 @@ doc-view-set-up-single-converter
   (pcase-let ((`(,conv-function ,type ,extension)
                (pcase doc-view-doc-type
                  ('djvu (list #'doc-view-djvu->tiff-converter-ddjvu 'tiff "tif"))
-                 (_     (list doc-view-pdf->png-converter-function  'png  "png")))))
+                 (_ (if (and (eq doc-view-pdf->png-converter-function
+                                 #'doc-view-pdf->png-converter-mupdf)
+                             doc-view-mupdf-use-svg)
+                        (list doc-view-pdf->png-converter-function 'svg "svg")
+                      (list doc-view-pdf->png-converter-function 'png "png"))))))
     (setq-local doc-view-single-page-converter-function conv-function)
     (setq-local doc-view--image-type type)
     (setq-local doc-view--image-file-pattern (concat "page-%s." extension))))
-- 
2.35.1

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Lars Ingebrigtsen]

Visuwesh <visuweshm@gmail.com> writes:

> Attached patch makes mupdf produce svg images rather than png when svg
> support is available.  This makes a noticeable improvement in image
> quality when zooming in.  This also improves epub, odt, docx,
> etc. rendering since they also end up using mupdf.
> I'm not sure if this is NEWS worthy though.
>
> I've also attached two images before.png and after.png to illustrate the
> difference in rendering before and after applying this patch.

Looks good; pushed to Emacs 29.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Visuwesh]

[புதன் ஜனவரி 04, 2023] Stefan Monnier wrote:

>> Attached patch makes mupdf produce svg images rather than png when svg
>> support is available.  This makes a noticeable improvement in image
>> quality when zooming in.
>
> IIUC this means that `+` and `-` now don't need to re-process the PDF, right?
> I think this is particularly valuable for things like ODT where `+/-`
> was pretty slow (because it re-created the PDF each time before having
> a chance to focus on the current page).

It depends on the value of `doc-view-scale-internally'.  The default
value (t) implies that we change the :width image property which leads
to blurry images when zooming.  In my case, even without zooming in, the
image quality was noticeably worse.
If `doc-view-scale-internally' is nil though, what you say happens.

> If that's the case it's probably worth mentioning in NEWS.
>
>> This also improves epub, odt, docx, etc. rendering since they also end
>> up using mupdf.  I'm not sure if this is NEWS worthy though.
>
> Other reasons it's worth mentioning in NEWS is because there's a new
> Custom to control it, and because it causes a regression for those LaTeX
> files which end up embedding bitmap fonts.  I just bumped into one and
> couldn't understand why every page took almost a minute to load;
> Removing `\usepackage[T1]{fontenc}` fixed the problem.

For the most part, I assumed MuPDF's svg and png conversion was
one-to-one.  My testing with small docx and Excel files went smooth so I
didn't think this feature warranted a NEWS entry.

>
>         Stefan

P.S. Sorry for the late reply, start of the semester had me busy.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> Attached patch makes mupdf produce svg images rather than png when svg
>>> support is available.  This makes a noticeable improvement in image
>>> quality when zooming in.
>> IIUC this means that `+` and `-` now don't need to re-process the PDF, right?
>> I think this is particularly valuable for things like ODT where `+/-`
>> was pretty slow (because it re-created the PDF each time before having
>> a chance to focus on the current page).
> It depends on the value of `doc-view-scale-internally'.  The default
> value (t) implies that we change the :width image property which leads
> to blurry images when zooming.  In my case, even without zooming in, the
> image quality was noticeably worse.

So `doc-view-scale-internally` should default to nil when we use SVGs, right?

> If `doc-view-scale-internally' is nil though, what you say happens.

IIUC you're saying here that when `doc-view-scale-internally` is nil we
re-create the SVGs every time the users try to zoom in/out?  While not
strictly a bug, it's a significant inefficiency we should address, no?

Another thing that's odd now is that we use
`doc-view-pdf->png-converter-function` to convert to SVG, despite
its name.

>> Other reasons it's worth mentioning in NEWS is because there's a new
>> Custom to control it, and because it causes a regression for those LaTeX
>> files which end up embedding bitmap fonts.  I just bumped into one and
>> couldn't understand why every page took almost a minute to load;
>> Removing `\usepackage[T1]{fontenc}` fixed the problem.
> For the most part, I assumed MuPDF's svg and png conversion was
> one-to-one.  My testing with small docx and Excel files went smooth so I
> didn't think this feature warranted a NEWS entry.

I don't know exactly what happens with those LaTeX-generated PDF files,
indeed.  I haven't bumped into the problem with any other PDF files yet
(including scans).  But those PDF files with embedded bitmap fonts used
the be the norm in the LaTeX world in some distant past, so I'm sure
some of our users will bump into them.


        Stefan

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Visuwesh]

[திங்கள் ஜனவரி 09, 2023] Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

>>> IIUC this means that `+` and `-` now don't need to re-process the PDF, right?
>>> I think this is particularly valuable for things like ODT where `+/-`
>>> was pretty slow (because it re-created the PDF each time before having
>>> a chance to focus on the current page).
>> It depends on the value of `doc-view-scale-internally'.  The default
>> value (t) implies that we change the :width image property which leads
>> to blurry images when zooming.  In my case, even without zooming in, the
>> image quality was noticeably worse.
>
> So `doc-view-scale-internally` should default to nil when we use SVGs, right?

It should default to t, which is the case currently.

>> If `doc-view-scale-internally' is nil though, what you say happens.
>
> IIUC you're saying here that when `doc-view-scale-internally` is nil we
> re-create the SVGs every time the users try to zoom in/out?  While not
> strictly a bug, it's a significant inefficiency we should address, no?

I suppose so.  But I'm not really sure if users out in the wild adjust
the variable.  If they do, then it might be worth looking into ignoring
that variable if we're generating SVG images.

> Another thing that's odd now is that we use
> `doc-view-pdf->png-converter-function` to convert to SVG, despite
> its name.

It felt like a waste to create a separate pdf->svg function since it
would contain the same exact BODY of mupdf pdf->png function since we
only change the PNG argument to end with a .svg extension at the caller
site (see doc-view-set-up-single-converter).
[ And AFAIK, mupdf is the only program that can convert pdf->svg (hence
  'mupdf' in the new defcustom's name).  ]

>>> Other reasons it's worth mentioning in NEWS is because there's a new
>>> Custom to control it, and because it causes a regression for those LaTeX
>>> files which end up embedding bitmap fonts.  I just bumped into one and
>>> couldn't understand why every page took almost a minute to load;
>>> Removing `\usepackage[T1]{fontenc}` fixed the problem.
>> For the most part, I assumed MuPDF's svg and png conversion was
>> one-to-one.  My testing with small docx and Excel files went smooth so I
>> didn't think this feature warranted a NEWS entry.
>
> I don't know exactly what happens with those LaTeX-generated PDF files,
> indeed.  I haven't bumped into the problem with any other PDF files yet
> (including scans).  But those PDF files with embedded bitmap fonts used
> the be the norm in the LaTeX world in some distant past, so I'm sure
> some of our users will bump into them.

From your report and the other recent one about buggy svg files, it
would be nice if we could gracefully fall back to png image generation
if there was an error during the conversion process but I currently
don't have the time to understand doc-view to propose such a patch.

In any case, what do you all think about the following NEWS entry?

    * doc-view generates SVG images when viewing PDF files if possible.
    If Emacs is built with SVG support, doc-view defaults to generating SVG
    files when using MuPDF as the converter for PDF files.  To get the old
    behaviour, set 'doc-view-mupdf-use-svg' to nil.
    Note that MuPDF SVG generation is known to be buggy for certain files.

>
>         Stefan

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Gregory Heytings]

>
> And AFAIK, mupdf is the only program that can convert pdf->svg (hence 
> 'mupdf' in the new defcustom's name).
>

That's not correct: pdftocairo does that, too.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>>> IIUC this means that `+` and `-` now don't need to re-process the PDF, right?
>>>> I think this is particularly valuable for things like ODT where `+/-`
>>>> was pretty slow (because it re-created the PDF each time before having
>>>> a chance to focus on the current page).
>>> It depends on the value of `doc-view-scale-internally'.  The default
>>> value (t) implies that we change the :width image property which leads
>>> to blurry images when zooming.  In my case, even without zooming in, the
>>> image quality was noticeably worse.
>>
>> So `doc-view-scale-internally` should default to nil when we use SVGs, right?
>
> It should default to t, which is the case currently.

Then what did you mean by:

    The default value (t) implies that we change the :width image
    property which leads to blurry images when zooming.

>>> If `doc-view-scale-internally' is nil though, what you say happens.
>> IIUC you're saying here that when `doc-view-scale-internally` is nil we
>> re-create the SVGs every time the users try to zoom in/out?  While not
>> strictly a bug, it's a significant inefficiency we should address, no?
> I suppose so.  But I'm not really sure if users out in the wild adjust
> the variable.

I set it to nil because of the blurriness.

> If they do, then it might be worth looking into ignoring
> that variable if we're generating SVG images.

Ah, that'd be a good option, indeed.

>> Another thing that's odd now is that we use
>> `doc-view-pdf->png-converter-function` to convert to SVG, despite
>> its name.
>
> It felt like a waste to create a separate pdf->svg function since it
> would contain the same exact BODY of mupdf pdf->png function since we
> only change the PNG argument to end with a .svg extension at the caller
> site (see doc-view-set-up-single-converter).

I wasn't thinking of duplicating the code, but of rethinking the naming
a bit.  I think what we meant by "pdf->png" is actually the process that
extracts pages (which just happened to use the PNG format and now can
also use the SVG format).

>     * doc-view generates SVG images when viewing PDF files if possible.
>     If Emacs is built with SVG support, doc-view defaults to generating SVG
>     files when using MuPDF as the converter for PDF files.  To get the old
>     behaviour, set 'doc-view-mupdf-use-svg' to nil.
>     Note that MuPDF SVG generation is known to be buggy for certain files.

Sounds good.  In my case, it wasn't "buggy" but just very slow (the
generation of the SVG is fast, but the SVG is very slow to render), so
maybe the last line should say:

      Note that MuPDF SVG generation is known to sometimes generate
      files that are buggy or can take a long time to render.

Also, the wording suggests the new default is a poor choice, so we may
want to include a more positive note about why we choose it as a default
despite its occasional downside (i.e. better rendering).


        Stefan

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Visuwesh]

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

[திங்கள் ஜனவரி 09, 2023] Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

> Then what did you mean by:
>
>     The default value (t) implies that we change the :width image
>     property which leads to blurry images when zooming.

I should have said "leads to blurry images when zooming when using PNG
images."  My bad.  It is the same as when we use +, - to zoom in on a
PNG image in image-mode.

>>> IIUC you're saying here that when `doc-view-scale-internally` is nil we
>>> re-create the SVGs every time the users try to zoom in/out?  While not
>>> strictly a bug, it's a significant inefficiency we should address, no?
>> I suppose so.  But I'm not really sure if users out in the wild adjust
>> the variable.
>
> I set it to nil because of the blurriness.
>> If they do, then it might be worth looking into ignoring
>> that variable if we're generating SVG images.
>
> Ah, that'd be a good option, indeed.

Right, then I guess adjustment of that sort is in order.  How about the
following patch?  It should be safe enough to go to emacs-29.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Use-internal-image-scaling-when-using-SVG-images-in-.patch --]
[-- Type: text/x-diff, Size: 1380 bytes --]

From 8e003e7127ff929306faef8e65af31cf8c7e4d08 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Thu, 12 Jan 2023 12:03:49 +0530
Subject: [PATCH 2/2] Use internal image scaling when using SVG images in
 doc-view

* lisp/doc-view.el (doc-view-enlarge, doc-view-scale-reset): Default
to changing the :width image property when using SVG images even if
doc-view-scaling-internally is nil.  (bug#58041)
---
 lisp/doc-view.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 7c272f52fb..19204f25b4 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -921,7 +921,7 @@ doc-view-shrink-factor
 (defun doc-view-enlarge (factor)
   "Enlarge the document by FACTOR."
   (interactive (list doc-view-shrink-factor))
-  (if doc-view-scale-internally
+  (if (or doc-view-scale-internally doc-view-mupdf-use-svg)
       (let ((new (ceiling (* factor doc-view-image-width))))
         (unless (equal new doc-view-image-width)
           (setq-local doc-view-image-width new)
@@ -941,7 +941,7 @@ doc-view-shrink
 (defun doc-view-scale-reset ()
   "Reset the document size/zoom level to the initial one."
   (interactive)
-  (if doc-view-scale-internally
+  (if (or doc-view-scale-internally doc-view-mupdf-use-svg)
       (progn
 	(kill-local-variable 'doc-view-image-width)
 	(doc-view-insert-image
-- 
2.38.1


[-- Attachment #3: Type: text/plain, Size: 2229 bytes --]


I see that the user option is also used by doc-view-insert-image but I'm
not sure if the adjustment is needed there as well.

>>> Another thing that's odd now is that we use
>>> `doc-view-pdf->png-converter-function` to convert to SVG, despite
>>> its name.
>>
>> It felt like a waste to create a separate pdf->svg function since it
>> would contain the same exact BODY of mupdf pdf->png function since we
>> only change the PNG argument to end with a .svg extension at the caller
>> site (see doc-view-set-up-single-converter).
>
> I wasn't thinking of duplicating the code, but of rethinking the naming
> a bit.  I think what we meant by "pdf->png" is actually the process that
> extracts pages (which just happened to use the PNG format and now can
> also use the SVG format).

Indeed, it is a misleading name.  This change will have to go to master,
I believe?  I have to look around a bit more to see where the function
is being used.
There's also the fact that there's more than one more program that can
generate SVG files (as Gregory pointed out in this thread) so it might
be nice to have pdf->png and pdf->svg "function variables" and a "super
function" that actually does the job.  Hopefully, this will allow to
fall back gracefully to PNG if SVG generation is faulty.

>>     * doc-view generates SVG images when viewing PDF files if possible.
>>     If Emacs is built with SVG support, doc-view defaults to generating SVG
>>     files when using MuPDF as the converter for PDF files.  To get the old
>>     behaviour, set 'doc-view-mupdf-use-svg' to nil.
>>     Note that MuPDF SVG generation is known to be buggy for certain files.
>
> Sounds good.  In my case, it wasn't "buggy" but just very slow (the
> generation of the SVG is fast, but the SVG is very slow to render), so
> maybe the last line should say:
>
>       Note that MuPDF SVG generation is known to sometimes generate
>       files that are buggy or can take a long time to render.
>
> Also, the wording suggests the new default is a poor choice, so we may
> want to include a more positive note about why we choose it as a default
> despite its occasional downside (i.e. better rendering).

Thanks for the feedback, how about the following patch?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0001-etc-NEWS-Announce-doc-view-s-SVG-support.patch --]
[-- Type: text/x-diff, Size: 1282 bytes --]

From 5b05c545eb3b974416daedcb78f3187ffb94af72 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Thu, 12 Jan 2023 11:58:29 +0530
Subject: [PATCH 1/2] * etc/NEWS: Announce doc-view's SVG support. (bug#58041)

---
 etc/NEWS | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index 16d17821b7..505943ffcf 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -561,6 +561,18 @@ option) and can be set to nil to disable Just-in-time Lock mode.
 \f
 * Changes in Emacs 29.1

+---
+** doc-view now generates SVG images when viewing PDF files if possible.
+If Emacs is built with SVG support, doc-view defaults to generating
+SVG files when using MuPDF as the converter for PDF files which leads
+to genearlly sharper images (especially when zooming) and
+customization of background and foreground color of the page via the
+new user options 'doc-view-svg-background' and
+'doc-view-svg-foreground'.  To get the old behaviour, set
+'doc-view-mupdf-use-svg' to nil.
+Note that MuPDF SVG generation is known to sometimes generate files
+that are buggy or can take a long time to render.
+
 +++
 ** New user option 'major-mode-remap-alist' to specify favorite major modes.
 This user option lets you remap the default modes (e.g. 'perl-mode' or
--
2.38.1

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Eli Zaretskii]

> Cc: 58041@debbugs.gnu.org
> From: Visuwesh <visuweshm@gmail.com>
> Date: Thu, 12 Jan 2023 12:13:27 +0530
> 
> >>> IIUC you're saying here that when `doc-view-scale-internally` is nil we
> >>> re-create the SVGs every time the users try to zoom in/out?  While not
> >>> strictly a bug, it's a significant inefficiency we should address, no?
> >> I suppose so.  But I'm not really sure if users out in the wild adjust
> >> the variable.
> >
> > I set it to nil because of the blurriness.
> >> If they do, then it might be worth looking into ignoring
> >> that variable if we're generating SVG images.
> >
> > Ah, that'd be a good option, indeed.
> 
> Right, then I guess adjustment of that sort is in order.  How about the
> following patch?  It should be safe enough to go to emacs-29.

Since this is not fixing a bug, I'd prefer it to go to master.

> +** doc-view now generates SVG images when viewing PDF files if possible.
> +If Emacs is built with SVG support, doc-view defaults to generating
> +SVG files when using MuPDF as the converter for PDF files which leads
> +to genearlly sharper images (especially when zooming) and
> +customization of background and foreground color of the page via the
> +new user options 'doc-view-svg-background' and
> +'doc-view-svg-foreground'.  To get the old behaviour, set
> +'doc-view-mupdf-use-svg' to nil.
> +Note that MuPDF SVG generation is known to sometimes generate files
> +that are buggy or can take a long time to render.

The last sentence makes me wonder why we made a less-than-perfect
option the default.  It's against our usual conservative approach.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Visuwesh]

[வியாழன் ஜனவரி 12, 2023] Eli Zaretskii wrote:

>> >> I suppose so.  But I'm not really sure if users out in the wild adjust
>> >> the variable.
>> >
>> > I set it to nil because of the blurriness.
>> >> If they do, then it might be worth looking into ignoring
>> >> that variable if we're generating SVG images.
>> >
>> > Ah, that'd be a good option, indeed.
>> 
>> Right, then I guess adjustment of that sort is in order.  How about the
>> following patch?  It should be safe enough to go to emacs-29.
>
> Since this is not fixing a bug, I'd prefer it to go to master.

IMO it would be better to go to emacs-29 since it prevents wasteful
(re-)generation of SVG files but if you insist, master is fine by me.

>> [...]
>> +Note that MuPDF SVG generation is known to sometimes generate files
>> +that are buggy or can take a long time to render.
>
> The last sentence makes me wonder why we made a less-than-perfect
> option the default.  It's against our usual conservative approach.

When I daily drove the patch to test it, I did not hit into the troubles
mentioned above so I turned the feature on by default but if the
troubles come up a lot more often, then I agree that it should be off by
default.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Eli Zaretskii]

> From: Visuwesh <visuweshm@gmail.com>
> Cc: monnier@iro.umontreal.ca,  58041@debbugs.gnu.org
> Date: Thu, 12 Jan 2023 13:50:24 +0530
> 
> >> +Note that MuPDF SVG generation is known to sometimes generate files
> >> +that are buggy or can take a long time to render.
> >
> > The last sentence makes me wonder why we made a less-than-perfect
> > option the default.  It's against our usual conservative approach.
> 
> When I daily drove the patch to test it, I did not hit into the troubles
> mentioned above so I turned the feature on by default but if the
> troubles come up a lot more often, then I agree that it should be off by
> default.

How frequently this happens probably depends on the use pattern, and
those can differ between people.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> How frequently this happens probably depends on the use pattern, and
> those can differ between people.

Maybe keep it disabled by default in `emacs-29` and enable it in
`master`?


        Stefan

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Visuwesh <visuweshm@gmail.com>,  58041@debbugs.gnu.org
> Date: Thu, 12 Jan 2023 11:27:37 -0500
> 
> > How frequently this happens probably depends on the use pattern, and
> > those can differ between people.
> 
> Maybe keep it disabled by default in `emacs-29` and enable it in
> `master`?

Thanks, done.

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> --- a/lisp/doc-view.el
> +++ b/lisp/doc-view.el
> @@ -921,7 +921,7 @@ doc-view-shrink-factor
>  (defun doc-view-enlarge (factor)
>    "Enlarge the document by FACTOR."
>    (interactive (list doc-view-shrink-factor))
> -  (if doc-view-scale-internally
> +  (if (or doc-view-scale-internally doc-view-mupdf-use-svg)
>        (let ((new (ceiling (* factor doc-view-image-width))))
>          (unless (equal new doc-view-image-width)
>            (setq-local doc-view-image-width new)
> @@ -941,7 +941,7 @@ doc-view-shrink
>  (defun doc-view-scale-reset ()
>    "Reset the document size/zoom level to the initial one."
>    (interactive)
> -  (if doc-view-scale-internally
> +  (if (or doc-view-scale-internally doc-view-mupdf-use-svg)
>        (progn
>  	(kill-local-variable 'doc-view-image-width)
>  	(doc-view-insert-image

Hmm.... `doc-view-mupdf-use-svg` means "use SVG when the backend
is mupdf" but we don't know here whether the backend is mupdf, so this
will misfire when using something else than mupdf, no?

>> I wasn't thinking of duplicating the code, but of rethinking the naming
>> a bit.  I think what we meant by "pdf->png" is actually the process that
>> extracts pages (which just happened to use the PNG format and now can
>> also use the SVG format).

> Indeed, it is a misleading name.  This change will have to go to master,
> I believe?  I have to look around a bit more to see where the function
> is being used.
> There's also the fact that there's more than one more program that can
> generate SVG files (as Gregory pointed out in this thread) so it might
> be nice to have pdf->png and pdf->svg "function variables" and a "super
> function" that actually does the job.  Hopefully, this will allow to
> fall back gracefully to PNG if SVG generation is faulty.

Indeed, I think there's some cleanup/orthogonalization in order here.


        Stefan

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Visuwesh]

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

[வியாழன் ஜனவரி 12, 2023] Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" wrote:

>> --- a/lisp/doc-view.el
>> +++ b/lisp/doc-view.el
>> @@ -921,7 +921,7 @@ doc-view-shrink-factor
>>  (defun doc-view-enlarge (factor)
>>    "Enlarge the document by FACTOR."
>>    (interactive (list doc-view-shrink-factor))
>> -  (if doc-view-scale-internally
>> +  (if (or doc-view-scale-internally doc-view-mupdf-use-svg)
>>        (let ((new (ceiling (* factor doc-view-image-width))))
>>          (unless (equal new doc-view-image-width)
>>            (setq-local doc-view-image-width new)
>> @@ -941,7 +941,7 @@ doc-view-shrink
>>  (defun doc-view-scale-reset ()
>>    "Reset the document size/zoom level to the initial one."
>>    (interactive)
>> -  (if doc-view-scale-internally
>> +  (if (or doc-view-scale-internally doc-view-mupdf-use-svg)
>>        (progn
>>  	(kill-local-variable 'doc-view-image-width)
>>  	(doc-view-insert-image
>
> Hmm.... `doc-view-mupdf-use-svg` means "use SVG when the backend
> is mupdf" but we don't know here whether the backend is mupdf, so this
> will misfire when using something else than mupdf, no?

Ah yes, of course.  Somehow, I managed to completely forget about the
other file formats supported by doc-view like djvu (since I only use
file formats that use mupdf in the end).  How about the below revised
patch?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-internal-image-scaling-when-using-SVG-images-in-.patch --]
[-- Type: text/x-diff, Size: 1877 bytes --]

From 70864205f1595815470639cd2ad47c5465206f03 Mon Sep 17 00:00:00 2001
From: Visuwesh <visuweshm@gmail.com>
Date: Thu, 12 Jan 2023 12:03:49 +0530
Subject: [PATCH] Use internal image scaling when using SVG images in doc-view

* lisp/doc-view.el (doc-view-enlarge, doc-view-scale-reset): Default
to changing the :width image property when using SVG images even if
doc-view-scaling-internally is nil.
(doc-view--image-type): Document the new possible image type.  (bug#58041)
---
 lisp/doc-view.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 7c272f52fb..cdca17f5af 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -489,7 +489,7 @@ doc-view-single-page-converter-function
 
 (defvar-local doc-view--image-type nil
   "The type of image in the current buffer.
-Can be `png' or `tiff'.")
+Can be `png', `svg', or `tiff'.")
 
 (defvar-local doc-view--image-file-pattern nil
   "The `format' pattern of image file names.
@@ -921,7 +921,9 @@ doc-view-shrink-factor
 (defun doc-view-enlarge (factor)
   "Enlarge the document by FACTOR."
   (interactive (list doc-view-shrink-factor))
-  (if doc-view-scale-internally
+  (if (or doc-view-scale-internally
+          (and (eq doc-view--image-type 'svg)
+               doc-view-mupdf-use-svg))
       (let ((new (ceiling (* factor doc-view-image-width))))
         (unless (equal new doc-view-image-width)
           (setq-local doc-view-image-width new)
@@ -941,7 +943,9 @@ doc-view-shrink
 (defun doc-view-scale-reset ()
   "Reset the document size/zoom level to the initial one."
   (interactive)
-  (if doc-view-scale-internally
+  (if (or doc-view-scale-internally
+          (and (eq doc-view--image-type 'svg)
+               doc-view-mupdf-use-svg))
       (progn
 	(kill-local-variable 'doc-view-image-width)
 	(doc-view-insert-image
-- 
2.38.1


[-- Attachment #3: Type: text/plain, Size: 1194 bytes --]


This does the expected on my end with doc-view-scale-internally to nil:
when visiting a djvu file and zooming in, it regenerates the images;
when visiting a docx file and zooming in, it modifies the :width image
property (i.e., no regeneration).

>>> I wasn't thinking of duplicating the code, but of rethinking the naming
>>> a bit.  I think what we meant by "pdf->png" is actually the process that
>>> extracts pages (which just happened to use the PNG format and now can
>>> also use the SVG format).
>
>> Indeed, it is a misleading name.  This change will have to go to master,
>> I believe?  I have to look around a bit more to see where the function
>> is being used.
>> There's also the fact that there's more than one more program that can
>> generate SVG files (as Gregory pointed out in this thread) so it might
>> be nice to have pdf->png and pdf->svg "function variables" and a "super
>> function" that actually does the job.  Hopefully, this will allow to
>> fall back gracefully to PNG if SVG generation is faulty.
>
> Indeed, I think there's some cleanup/orthogonalization in order here.

When I have more time and familiarity, I will try to give this a shot.

>         Stefan

bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> @@ -489,7 +489,7 @@ doc-view-single-page-converter-function
>  
>  (defvar-local doc-view--image-type nil
>    "The type of image in the current buffer.
> -Can be `png' or `tiff'.")
> +Can be `png', `svg', or `tiff'.")
>  
>  (defvar-local doc-view--image-file-pattern nil
>    "The `format' pattern of image file names.
> @@ -921,7 +921,9 @@ doc-view-shrink-factor
>  (defun doc-view-enlarge (factor)
>    "Enlarge the document by FACTOR."
>    (interactive (list doc-view-shrink-factor))
> -  (if doc-view-scale-internally
> +  (if (or doc-view-scale-internally
> +          (and (eq doc-view--image-type 'svg)
> +               doc-view-mupdf-use-svg))
>        (let ((new (ceiling (* factor doc-view-image-width))))
>          (unless (equal new doc-view-image-width)
>            (setq-local doc-view-image-width new)
> @@ -941,7 +943,9 @@ doc-view-shrink
>  (defun doc-view-scale-reset ()
>    "Reset the document size/zoom level to the initial one."
>    (interactive)
> -  (if doc-view-scale-internally
> +  (if (or doc-view-scale-internally
> +          (and (eq doc-view--image-type 'svg)
> +               doc-view-mupdf-use-svg))
>        (progn
>  	(kill-local-variable 'doc-view-image-width)
>  	(doc-view-insert-image

Looks OK to me, tho I wonder why we still need to check
`doc-view-mupdf-use-svg`.
Isn't it sufficient to check (eq doc-view--image-type 'svg)?


        Stefan