unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Visuwesh <visuweshm@gmail.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: 58041@debbugs.gnu.org
Subject: bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion
Date: Thu, 12 Jan 2023 12:13:27 +0530	[thread overview]
Message-ID: <87fscgxng0.fsf@gmail.com> (raw)
In-Reply-To: <jwveds3z5d8.fsf-monnier+emacs@gnu.org> (Stefan Monnier via's message of "Mon, 09 Jan 2023 18:02:49 -0500")

[-- 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

  reply	other threads:[~2023-01-12  6:43 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-24 10:18 bug#58041: [PATCH] docview: Use svg images when using mupdf for conversion Visuwesh
2022-09-24 12:10 ` Lars Ingebrigtsen
     [not found] ` <jwva62yxevs.fsf-monnier+emacs@gnu.org>
2023-01-08  6:09   ` Visuwesh
2023-01-09 14:51     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-09 16:32       ` Visuwesh
2023-01-09 17:18         ` Gregory Heytings
2023-01-09 23:02         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-12  6:43           ` Visuwesh [this message]
2023-01-12  8:11             ` Eli Zaretskii
2023-01-12  8:20               ` Visuwesh
2023-01-12  8:48                 ` Eli Zaretskii
2023-01-12 16:27                   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-14  8:33                     ` Eli Zaretskii
2023-01-12 16:33             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-01-12 16:47               ` Visuwesh
2023-01-12 21:11                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fscgxng0.fsf@gmail.com \
    --to=visuweshm@gmail.com \
    --cc=58041@debbugs.gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).