D8379: PDF: Support the new poppler renderToImage with update callback

[Albert Astals Cid]

aacid added a comment.


  In https://phabricator.kde.org/D8379#157771, @rkflx wrote:
  
  > That's a really great feature Okular's user will surely love. Does this solve https://bugs.kde.org/show_bug.cgi?id=344081?
  
  
  Yes
  
  > When testing (with https://phabricator.kde.org/D8378 and https://phabricator.kde.org/D8379 both applied at the same time – sorry for that –, as well as the Poppler patch), there were still some issues for me:
  > 
  > - After zooming in and rendering finished (i.e. CPU usage went back to zero), the display would not update. Only after panning enough so a request for a new tile triggered rendering again the display would also update. (Note: I tested whether this is a regression with the HiDPI commit, but it is not.)
  > - Same thing for zooming out.
  
  Right, thanks for finding this, that happens because Page::hasPixmap is not really const when the page has a tile manager, i've added a workaround and FIXME and i'll see if i can find some time to fix that properly, but i don't think we should block this feature of landing because of old code being broken.
  
  > - The progressive rendering seems to only work with Fit Page on startup, but not with 100% Zoom on startup or when zooming in later. (My first thought was that there might be some connection to the scaled raster images shown as a temporary preview, but apparently that is not always the case.)
  
  Nothing to do with Fit Page, basically if you got into tiled rendering the progressive rendering didn't happen, i've also fixed that now.
  
  > Also I am wondering if the partial updates might slow down the total rendering time, i.e. whether there should be some rate limiting to the partial updates (if there isn't already)?
  
  It totally does since there's more "here take this pixmap and put it into screen", but since the user seems things earlier the end result for his mind is "things are faster". I don't see a need to limit it really (other than the initial 500ms barrier)

INLINE COMMENTS

> rkflx wrote in generator_pdf.cpp:905
> Why not set this to something in the region of 30 to 60 fps (e.g. ~30ms instead of 500ms)? This way some overhead would be avoided while potentially still feeling somewhat fluent (i.e. not seeing Okular's loading icon) when scrolling through pages.

Because it looks bad, lots of pages take more than 30ms and you don't really want to see them rendering just twice (say once at 30ms and one at 45ms when it finishes), your brain gets upset about it because you get a render with 66% of the rendering done and just 15ms later you get 100%.

Sure, this may also happen with the page taking 515ms to render and so you get an update at 500ms and one at 515ms, but in that case your brain gets a little less upset since you got a render with 97% of the rendering done and one with 100%.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D8379

To: aacid, #okular
Cc: rkflx, ngraham, michaelweghorn, mlaurent, #okular, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20171023/ba7c222a/attachment-0001.html>