search

izuzak / atom-pdf-view

Support for viewing PDF files in Atom.
https://atom.io/packages/pdf-view
MIT License
108 stars 30 forks source link

Refresh on file changes that occur while updating the document #67

Closed timjb closed 9 years ago

timjb commented 9 years ago

This fixes the problem I've described in https://github.com/izuzak/atom-pdf-view/issues/64#issuecomment-123486891.

izuzak commented 9 years ago

@timjb Thanks for taking the time to open this PR and also for the write-up in https://github.com/izuzak/atom-pdf-view/issues/64#issuecomment-123486891. :zap: :zap:

I think I understand what you're describing, and I guess it's possible this happens. I still wish I could reproduce this, though. Have you been able to reliably reproduce the problem with a specific latex document and specific changes made to the document?

Regardless of that, I think we can give this a try. :+1: Before we do, I wanted to run one other idea by you.

Currently, the debounce call in onFileChangeCallback is set to wait 100ms for any other calls to that callback before invoking updatePdf. It seems to me that we might increase that wait period (e.g. to 1s) to make this problem appear less frequently. If a change happens for a file and then no other changes happen for 100ms -- then the file will be updated in pdf-view. But if a change happens soon after those 100 ms pass and while the file is being updated -- then the problem surfaces, right? So, we could wait for a bit longer for changes to happen. When combined with your fix in this PR, this would prevent the file from being re-updated frequently, e.g. if a file changes 10 times with 105ms between two changes then the file would be re-rendered 10 times, right? But if we increase the wait period to 500-100s, then only one render would happen, which might be a better experience.

What do you think?

timjb commented 9 years ago

I can reproduce the problem quite reliably with membrane-surface.tex. Compile the file with pdflatex membrane-surface.tex, open membrane-surface.pdf in pdf-view and the recompile the file. This gives me a black pane in >90% of runs.

This is why this file exposes the bug: The LaTeX drawing at the beginning of the document takes a long time to compile and triggers a rerendering immediately after LaTeX has finished this part of the document. Rendering the document takes a long time, again because of that drawing. The final few paragraphs of text are compiled relatively fast and LaTeX finishes compiling before rerendering has finished.

It seems like a good idea to increase the debounce timeout. But one second is a bit too long in my opinion. Some users might be compiling a simple LaTeX file which takes only about half a second to compile. I think it would be bad user experience to make them wait for 1.5 seconds instead of 0.5 seconds to see the changes to the document.

e.g. if a file changes 10 times with 105ms between two changes then the file would be re-rendered 10 times, right?

Only if rendering takes < 105ms. If it takes longer, the file will be rerendered fewer times. One of my documents currently takes 300ms to render.

Another idea: One could use the file size heurestic to guess when LaTeX (or another program) has finished compiling: it has finished when the file size of the PDF is roughly the same as after the last successful rendering. Of course the file size can change, for example when the user adds an image to the document. This might be combined with a flexible debounce timeout: If the file is still smaller than after the last rerender, then a larger timespan without change is needed to trigger a render than when the file size roughly matches the full size after the last rerendering.

One last idea: The "right" debounce timeout could be dynamically determined on a per-document basis. One could remember how long the longest pause between two file changes was where rendering wasn't successful after the first change.

Finally, it might not be so bad to rerender often: It isn't apparent from the user interface that pdf-view tries to rerender the document multiple times while it's being updated. All the user sees is a black pane, no flickering etc.

izuzak commented 9 years ago

Finally, it might not be so bad to rerender often: It isn't apparent from the user interface that pdf-view tries to rerender the document multiple times while it's being updated. All the user sees is a black pane, no flickering etc.

That sounds great. I think you're right -- let's merge this and see if anyone sees any problems and we can come back to these ideas then.

Thanks again for fixing this :heart: :heart: