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

Reply via email to