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

[Albert Astals Cid]

aacid marked an inline comment as done.
aacid added inline comments.

INLINE COMMENTS

> mwolff wrote in generator.cpp:407
> is `setPixmap` old API? Why create a QPixmap on the heap? That should not be done, pass values instead.

Yes, it's existing (not old :P) API

> mwolff wrote in generator.h:581
> aka: "Must be called from the main thread."

Are you asking for a reword? i see both my and your sentence basically say the same?

> mwolff wrote in generator_pdf.cpp:919
> why is the timer active when there is no remaining time? when does this happen? can you add a comment please?

Because that is how timers work :)

It happens when the 500ms (or more) of the timer have passed (so remaining time is 0) and the timer hasn't been stopped (so its still active).

Do you want me to say "read the QTimer documentation" here? I don't understand which kind of comment you want. Something like "has the timeout passed"?

> mwolff wrote in generator_pdf.cpp:929
> sending raw pointers over a queued connection sounds scary - who owns the pixmap request? will it really outlive everything? could you instead use a shared pointer or similar value type to ensure it really never gets destroyed in the middle?

This is ok. 
This is a partial update callback, so it's always going to happen before the rendering ends, so the request will always survive until later.

REPOSITORY
  R223 Okular

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

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