bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Jambunathan K]

1. Enable session management.

    (custom-set-variables
     '(desktop-base-file-name "~/.emacs.desktop")
     '(desktop-save-mode t)
     '(save-place t nil (saveplace))
     '(savehist-mode t))

2. Open a PDF file.  Make sure the conversion goes through and visit
   some arbitrary page.

   The file I have is 102.3MB and has 382 pages.  

3. C-x C-c and start Emacs.

4. Note that doc-view starts with all conversion all over again.

Expected behaviour: doc-view should access the already "cached" pages
and drop me in the same page that I was in the previous session.


In GNU Emacs 24.3.50.1 (i686-pc-linux-gnu, GTK+ Version 2.20.1)
 of 2013-05-20 on debian-6.05
Bzr revision: 112643 mituharu@math.s.chiba-u.ac.jp-20130520031520-pq1c1qjertan94kq
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
Important settings:
  value of $LANG: en_IN
  locale-coding-system: iso-latin-1-unix
  default enable-multibyte-characters: t

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Tassilo Horn]

Jambunathan K <kjambunathan@gmail.com> writes:

> 1. Enable session management.
>
>     (custom-set-variables
>      '(desktop-base-file-name "~/.emacs.desktop")
>      '(desktop-save-mode t)
>      '(save-place t nil (saveplace))
>      '(savehist-mode t))
>
> 2. Open a PDF file.  Make sure the conversion goes through and visit
>    some arbitrary page.
>
>    The file I have is 102.3MB and has 382 pages.  
>
> 3. C-x C-c and start Emacs.
>
> 4. Note that doc-view starts with all conversion all over again.
>
> Expected behaviour: doc-view should access the already "cached" pages
> and drop me in the same page that I was in the previous session.

I've just tried this receipe (although I've done M-x desktop-save/read
explicitly instead of adding the customs as in 1), but I didn't get a
reconversion.  It used the cached images as it should.

After restoring the desktop however, I'm always on page 1 of the PDF.
I've just committed some desktop.el integration code that saves the
current page and slice as "misc" information that's later used for
restoring the desktop (revno 112712).

However, that's not really perfect.  Firstly, the page/slice information
is managed on a per-window (not per-buffer) basis, so if you have two
windows showing different pages of the same PDF, it's arbitrary whose
page/slice is saved.

The more pressing problem is that currently you'll get an error when
restoring the PDF from `desktop-read'.

--8<---------------cut here---------------start------------->8---
Debugger entered--Lisp error: (wrong-type-argument listp t)
  image-mode-winprops(nil)
  image-mode-window-put(page 0 nil)
  (progn (image-mode-window-put (quote page) page nil) (image-mode-window-put (quote info) (concat (propertize (format "Page %d of %d." page len) (quote face) (quote bold)) (if doc-view-current-converter-processes " (still converting...)\n" "\n") (if (and doc-view-current-search-matches (assq page doc-view-current-search-matches)) (progn (concat (propertize "Search matches:\n" (quote face) (quote bold)) (let (...) (progn ...) contexts))))) nil))
  (let ((len (doc-view-last-page-number))) (if (< page 1) (setq page 1) (if (and (> page len) (null doc-view-current-converter-processes)) (progn (setq page len)))) (progn (image-mode-window-put (quote page) page nil) (image-mode-window-put (quote info) (concat (propertize (format "Page %d of %d." page len) (quote face) (quote bold)) (if doc-view-current-converter-processes " (still converting...)\n" "\n") (if (and doc-view-current-search-matches (assq page doc-view-current-search-matches)) (progn (concat (propertize "Search matches:\n" ... ...) (let ... ... contexts))))) nil)) (let ((file (expand-file-name (format doc-view--image-file-pattern page) (doc-view-current-cache-dir)))) (doc-view-insert-image file :pointer (quote arrow)) (if (and (not (file-exists-p file)) doc-view-current-converter-processes) (progn (funcall doc-view-single-page-converter-function doc-view-buffer-file-name file page (let ((win ...)) (function (lambda nil ...))))))) (overlay-put (image-mode-window-get (quote overlay)) (quote help-echo) (image-mode-window-get (quote info))))
  doc-view-goto-page(1)
  (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let ((page (assq (quote page) winprops))) (if (numberp page) page 1))))
  (unwind-protect (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let ((page (assq (quote page) winprops))) (if (numberp page) page 1)))) (internal--after-with-selected-window save-selected-window--state))
  (save-current-buffer (unwind-protect (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let ((page (assq ... winprops))) (if (numberp page) page 1)))) (internal--after-with-selected-window save-selected-window--state)))
  (let ((save-selected-window--state (internal--before-with-selected-window (car winprops)))) (save-current-buffer (unwind-protect (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let ((page ...)) (if (numberp page) page 1)))) (internal--after-with-selected-window save-selected-window--state))))
  (if doc-view-current-converter-processes nil (let ((save-selected-window--state (internal--before-with-selected-window (car winprops)))) (save-current-buffer (unwind-protect (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let (...) (if ... page 1)))) (internal--after-with-selected-window save-selected-window--state)))))
  (closure ((ol . #<overlay from 1 to 800661 in ABJKT10.pdf>) (winprops #<window 0x7280b00 on doc-view.el> (image image :type imagemagick :file "/tmp/docview1000/ABJKT10.pdf-079d2ad420811f97f127c85c9de74b5c/page-3.png" :pointer arrow :width 850) (info . #("Page 3 of 18.\n" 0 13 (face bold))) (page . 3) (slice 166 181 517 891) (overlay . #<overlay from 1 to 800661 in ABJKT10.pdf>)) t) nil (if doc-view-current-converter-processes nil (let ((save-selected-window--state (internal--before-with-selected-window (car winprops)))) (save-current-buffer (unwind-protect (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let ... ...))) (internal--after-with-selected-window save-selected-window--state))))))()
  apply((closure ((ol . #<overlay from 1 to 800661 in ABJKT10.pdf>) (winprops #<window 0x7280b00 on doc-view.el> (image image :type imagemagick :file "/tmp/docview1000/ABJKT10.pdf-079d2ad420811f97f127c85c9de74b5c/page-3.png" :pointer arrow :width 850) (info . #("Page 3 of 18.\n" 0 13 (face bold))) (page . 3) (slice 166 181 517 891) (overlay . #<overlay from 1 to 800661 in ABJKT10.pdf>)) t) nil (if doc-view-current-converter-processes nil (let ((save-selected-window--state (internal--before-with-selected-window (car winprops)))) (save-current-buffer (unwind-protect (progn (select-window (car save-selected-window--state) (quote norecord)) (doc-view-goto-page (let ... ...))) (internal--after-with-selected-window save-selected-window--state)))))) nil)
  byte-code("r\301\b\302H\b\303H\"\210)\301\207" [timer apply 5 6] 4)
  timer-event-handler([t 20895 18569 782360 nil (closure ((ol . #<overlay from 1 to 800661 in ABJKT10.pdf>) (winprops #<window 0x7280b00 on doc-view.el> (image image :type imagemagick :file "/tmp/docview1000/ABJKT10.pdf-079d2ad420811f97f127c85c9de74b5c/page-3.png" :pointer arrow :width 850) (info . #("Page 3 of 18.\n" 0 13 (face bold))) (page . 3) (slice 166 181 517 891) (overlay . #<overlay from 1 to 800661 in ABJKT10.pdf>)) t) nil (if doc-view-current-converter-processes nil (let ((save-selected-window--state (internal--before-with-selected-window (car winprops)))) (save-current-buffer (unwind-protect (progn (select-window ... ...) (doc-view-goto-page ...)) (internal--after-with-selected-window save-selected-window--state)))))) nil nil 35000])
--8<---------------cut here---------------end--------------->8---

The error comes from the closure run from a timer in
`doc-view-new-window-function'.  The document is shown with the correct
page and slice anyway.  Argh, that image-mode winprops stuff is a beast.

Stefan, since you've said the functions run by timers in
`doc-view-new-window-function' shouldn't be needed anyway and probably
work around a bug elsewhere, could you please have a look?

Bye,
Tassilo

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Jambunathan K]

Tassilo Horn <tsdh@gnu.org> writes:

Quick feedback from my side.

Session restoration aborts with 
        Error running timer: (wrong-type-argument listp t)

Usecase:
=======

The PDF file is actually created as below

        Scanned Book (Pre-dominantly English with some Chinese) ->
        Unpaper -> Tesseract-> Text -> Org -> HTML -> EPUB -> Kobo Reader

Since the original book has Chinese characters and lots of Footnotes,
there were a lot of errors in Tesseracted file which I had to copy-edit
and fix.

During the above process, a single Emacs frame was split in to two
windows.  One window displaying page (of PDF) and the other window
corresponding text.  During the copy-edit and proof-reading stage, there
were occasions when I found it convenient to

  (i)  C-x 2 and have the PDF page fit to window width.
  (ii) C-x 3 and have the PDF page fit to window height.

Requirements from the above case:
================================

1. Restoration of page number
2. Restoration of "Fit to Height" (H) and "Fit to Width" (W) settings.

Nitpicks
========

1. Why am I asked this question again.  This question shouldn't be asked at
   all.
   
    ,----
    | File unpapered-blyth-single-page.pdf is large (102.3M), really open? (y or n)  y
    `----

2. I don't know what to make of the changing page numbers in the
   modeline when the PDF page is "loaded."  Does this "counting up to
   total number of pages" happen even if the page is loaded from cache.

> (revno 112712).

Tried it out. 

,---- *Messages*
| Loading desktop...done
| Loading savehist...done
| 
|   -------------------------------------- 
|   |
|   V
| File unpapered-blyth-single-page.pdf is large (102.3M), really open? (y or n)  y
| DocView: using cached files!
| Type C-c C-c to toggle between editing or viewing the document.
| Wrote /home/kjambunathan/.emacs.d/.emacs.desktop.lock
| Desktop: 3 buffers restored, 1 failed to restore.
| For information about GNU Emacs and the GNU system, type C-h C-a.
| Error running timer: (wrong-type-argument listp t)  <-------------------------
| gQuit [2 times]
| Mark set
| Beginning of buffer [6 times]
| Mark set
| Quit
| File `~/.emacs' exists; overwrite? (y or n)  y
| Saving file /home/kjambunathan/.emacs...
| Wrote /home/kjambunathan/.emacs
`----

> However, that's not really perfect.

Same here. I am running in to a timer issue.

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Stefan Monnier]

> Stefan, since you've said the functions run by timers in
> `doc-view-new-window-function' shouldn't be needed anyway and probably
> work around a bug elsewhere, could you please have a look?

I just installed a patch which removes the use of those timers.
I haven't tested it with desktop and I'm not sure it doesn't reintroduce
some old problem, but at least it seems to work in my tests.


        Stefan

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Tassilo Horn]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Stefan, since you've said the functions run by timers in
>> `doc-view-new-window-function' shouldn't be needed anyway and
>> probably work around a bug elsewhere, could you please have a look?
>
> I just installed a patch which removes the use of those timers.  I
> haven't tested it with desktop and I'm not sure it doesn't reintroduce
> some old problem, but at least it seems to work in my tests.

Well, the desktop.el integration now works flawlessly, but the two old
problems that were handled by those timers are now back again.

  1. When opening a doc that already has cached images, you'll only see
     the raw contents but no image.  In order to make the image appear,
     you have to do something like switch pages or scroll.

  2. When splitting a doc-view window, the new window only shows the raw
     contents.  In order to make the image appear, you need to select
     the new window once.

Bye,
Tassilo

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Stefan Monnier]

>   1. When opening a doc that already has cached images, you'll only see
>      the raw contents but no image.  In order to make the image appear,
>      you have to do something like switch pages or scroll.

That was my main test case while writing the patch, so "it works for me".
Could you give a detailed recipe?

>   2. When splitting a doc-view window, the new window only shows the raw
>      contents.  In order to make the image appear, you need to select
>      the new window once.

Same here: it works for me.  So please provide a recipe.


        Stefan

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Tassilo Horn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>   1. When opening a doc that already has cached images, you'll only see
>>      the raw contents but no image.  In order to make the image appear,
>>      you have to do something like switch pages or scroll.
>
> That was my main test case while writing the patch, so "it works for me".
> Could you give a detailed recipe?

Sure.

  1. emacs -Q
  2. C-x C-f some.pdf
     conversion is done and I see the first page of the PDF
  3. k
  4. C-x C-f some.pdf
     I only see the raw PDF contents
  5. <down>
     Now I see the image.

>>   2. When splitting a doc-view window, the new window only shows the raw
>>      contents.  In order to make the image appear, you need to select
>>      the new window once.
>
> Same here: it works for me.  So please provide a recipe.

Hm, when I did C-x 2 or C-x 3 after step 5 above, I used to see the raw
PDF contents in the new window, but now I get the image immediately.

But the problem still happens with my usual emacs config, i.e., without
-Q.  The only configuration wrt. doc-view is

    (setq doc-view-resolution 200).

and

    ;; Enable ImageMagick
    (when (fboundp 'imagemagick-types)
      (setq imagemagick-render-type 1)
      (imagemagick-register-types))

for image-mode.  That doesn't seem to be needed anymore, so I've removed
it.  But that doesn't change anything...

Ah, now I found the difference.  You get that behavior when you disable
`scroll-bar-mode' as I do.

Bye,
Tassilo

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Stefan Monnier]

>> That was my main test case while writing the patch, so "it works for me".
>> Could you give a detailed recipe?

> Sure.

>   1. emacs -Q
>   2. C-x C-f some.pdf
>      conversion is done and I see the first page of the PDF
>   3. k
>   4. C-x C-f some.pdf
>      I only see the raw PDF contents
>   5. <down>
>      Now I see the image.

Aha!  Now I see it, and it seems like it's a long standing bug in
image-mode.el.  I installed the patch below which seems to fix this problem.
It seems to also fix the scroll-bar-mode issue.


        Stefan


--- lisp/image-mode.el	2013-05-24 17:42:23 +0000
+++ lisp/image-mode.el	2013-05-27 00:55:05 +0000
@@ -104,13 +104,16 @@
 (defun image-mode-reapply-winprops ()
   ;; When set-window-buffer, set hscroll and vscroll to what they were
   ;; last time the image was displayed in this window.
-  (when (and (image-get-display-property)
-	     (listp image-mode-winprops-alist))
+  (when (listp image-mode-winprops-alist)
+    ;; Beware: this call to image-mode-winprops can't be optimized away,
+    ;; because it not only gets the winprops data but sets it up if needed
+    ;; (e.g. it's used by doc-view to display the image in a new window).
     (let* ((winprops (image-mode-winprops nil t))
            (hscroll (image-mode-window-get 'hscroll winprops))
            (vscroll (image-mode-window-get 'vscroll winprops)))
+      (when (image-get-display-property) ;Only do it if we display an image!
       (if hscroll (set-window-hscroll (selected-window) hscroll))
-      (if vscroll (set-window-vscroll (selected-window) vscroll)))))
+	(if vscroll (set-window-vscroll (selected-window) vscroll))))))
 
 (defun image-mode-setup-winprops ()
   ;; Record current scroll settings.

bug#14435: 24.3.50; Co-operation of doc-view + desktop-save-mode, save-place etc.

[Tassilo Horn]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> That was my main test case while writing the patch, so "it works for me".
>>> Could you give a detailed recipe?
>
>> Sure.
>
>>   1. emacs -Q
>>   2. C-x C-f some.pdf
>>      conversion is done and I see the first page of the PDF
>>   3. k
>>   4. C-x C-f some.pdf
>>      I only see the raw PDF contents
>>   5. <down>
>>      Now I see the image.
>
> Aha!  Now I see it, and it seems like it's a long standing bug in
> image-mode.el.  I installed the patch below which seems to fix this problem.
> It seems to also fix the scroll-bar-mode issue.

Yes, it fixes both issues for me as well.  Great!

Bye,
Tassilo