D18883: Add PDF thumbnailer

meven added a comment. In D18883#424888 , @aacid wrote: > Also do we have a way to limit thumbnailers to run say for X seconds? There's some files in which poppler will run "forever" kio-extras thumbnail is a an ioslave so I guess we can

D18883: Add PDF thumbnailer

ngraham added a comment. Yeah, seems like this got bikeshedded to death but I think it would still be quite worthwhile to have. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc: meven, bruns, dhaumann, ngraham, pino,

D18883: Add PDF thumbnailer

meven added a comment. ping Is it still standing ? REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc: meven, bruns, dhaumann, ngraham, pino, ltoscano, kde-frameworks-devel, kfm-devel, waitquietly, azyx, nikolaik,

D18883: Add PDF thumbnailer

aacid added a comment. Also do we have a way to limit thumbnailers to run say for X seconds? There's some files in which poppler will run "forever" REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc: bruns, dhaumann,

D18883: Add PDF thumbnailer

broulik added a comment. Perhaps but it's only recently become an issue since kioslaves now support KCrash. Previously such thumbnailer crashes just went unnoticed. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc:

D18883: Add PDF thumbnailer

bruns added a comment. In D18883#414517 , @broulik wrote: > However, you will get a Dr Konqi each time it encounters that file as it will try to generate a thumbnail for it every time you open the folder. Shouldn't this be covered

D18883: Add PDF thumbnailer

broulik added a comment. However, you will get a Dr Konqi each time it encounters that file as it will try to generate a thumbnail for it every time you open the folder. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate

D18883: Add PDF thumbnailer

dfaure added a comment. kio_thumbnail is a separate process (like all kioslaves) so a crash doesn't bring down the user-visible application. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc: dhaumann, ngraham, pino,

D18883: Add PDF thumbnailer

aacid added a comment. The code can't be much simpler than this, there's not much places it can go wrong, i guess the only other thing to mention is, poppler will crash with some bad data, so i don't know if this is on it's own process or not, but if it's not it may bring down whatever app

D18883: Add PDF thumbnailer

broulik added a comment. Ping REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc: dhaumann, ngraham, pino, ltoscano, kde-frameworks-devel, kfm-devel, alexde, feverfew, michaelh, spoorun, navarromorales, firef,

D18883: Add PDF thumbnailer

broulik added inline comments. INLINE COMMENTS > dhaumann wrote in pdfcreator.cpp:62 > Does this also give nice results with high dpi displays? KIO Thumbnailer doesn't support high dpi at all at this point. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To:

D18883: Add PDF thumbnailer

dhaumann added inline comments. INLINE COMMENTS > pdfcreator.cpp:62 > +// Compute a dpi that results in a thumbnail roughly the size we > requested > +const qreal fakeDpiX = width / page->pageSizeF().width() * 72.0; > +const qreal fakeDpiY = height / page->pageSizeF().height() *

D18883: Add PDF thumbnailer

pino added a comment. In D18883#408893 , @ngraham wrote: > Conceptually I think it makes sense to have all the thumbnailers in their own package so it's easy to remove them all if for people who really don't want them. ... resulting in

D18883: Add PDF thumbnailer

broulik updated this revision to Diff 51292. broulik added a comment. - Render image at desired size already - Remove unused include REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18883?vs=51290=51292 REVISION DETAIL

D18883: Add PDF thumbnailer

ngraham added a comment. Conceptually I think it makes sense to have all the thumbnailers in their own package so it's easy to remove them all if for people who really don't want them. That package (`kdegraphics-thumbnailers`) is shipped by default by at least Kubuntu, openSUSE, Manjaro and

D18883: Add PDF thumbnailer

pino added inline comments. INLINE COMMENTS > pdfcreator.cpp:23 > + > +#include > +#include the QFile include is no more needed now > broulik wrote in pdfcreator.cpp:45-46 > I can't. The `renderToImage` can only be told a resolution or part of the > page to render, to render downscaled into

D18883: Add PDF thumbnailer

broulik added inline comments. INLINE COMMENTS > pino wrote in pdfcreator.cpp:45-46 > please honor the requested width and height I can't. The `renderToImage` can only be told a resolution or part of the page to render, to render downscaled into a certain box. The `ThumbnailJob` downscales

D18883: Add PDF thumbnailer

broulik updated this revision to Diff 51290. broulik added a comment. - Remove mimetypes left from testing - Don't encode filename (missed that there's one other than the `QByteArray` one) REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE

D18883: Add PDF thumbnailer

pino added inline comments. INLINE COMMENTS > pdfcreator.cpp:45-46 > +{ > +Q_UNUSED(width); > +Q_UNUSED(height); > + please honor the requested width and height > pdfcreator.cpp:49 > +QScopedPointer document; > +document.reset(Document::load(QFile::encodeName(path))); > +

D18883: Add PDF thumbnailer

ltoscano added a comment. Why can't this go to kdegraphics-thumbnailer? Kio-extras shouldn't be, IMHO, a "drop everything which does not fit elsewhere" REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D18883 To: broulik, dfaure, aacid, jtamate Cc: ltoscano,

D18883: Add PDF thumbnailer

broulik created this revision. broulik added reviewers: dfaure, aacid, jtamate. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. broulik requested review of this revision. REVISION SUMMARY This adds a PDF thumbnailer using libpoppler.