D6083: Bump max thumbnail size to 512px

[Henrik Fehlauer]

rkflx added a project: Gwenview.
rkflx edited reviewers, added: Gwenview; removed: Konsole, tcanabrava, dhaumann.
rkflx requested changes to this revision.
rkflx added a comment.
This revision now requires changes to proceed.


  Thanks for coming back to this. Yes, HiDPI support is still in progress. If you could help with the thumbnail-spec/caching side of things that would be great!
  
  In D6083#292991 <https://phabricator.kde.org/D6083#292991>, @sandsmark wrote:
  
  > In D6083#114550 <https://phabricator.kde.org/D6083#114550>, @gateau wrote:
  >
  > > I love the idea, but what about caching? Cached thumbnails are supposed to follow https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html#THUMBSIZE, which still, sadly, says 256.
  >
  >
  > it says the sizes are just suggestions, so I don't think bumping it up a bit would violate this.
  
  
  Quotes from the spec:
  
  > - Normal: […]The image size must not exceed 128x128 pixel.
  > - Large: […] must be 256x256 pixel.
  > - However, these are suggestions. Implementations should cope also with images that are smaller […]
  
  I don't see anything in there about larger thumbnails.
  
  Some observations running Gwenview with this patch:
  
  - Even with the size slider set to 512px, the thumbnails being written to the cache are still 256px. I guess this makes sense, since the "normal" and "large" folders of the cache are meant for 128px and 256px thumbnails respectively as per the spec. However, what's the point of the cache then? Perhaps the spec should be extended to allow for something like a "huge" folder with 512px thumbnails?
  - The pixmap used for the thumbnail is still only 256px, this patch merely allows scaling that up to 512px, making it look blurry (can be seen easily by viewing the thumbnail of a screenshot and setting `MaxThumbnailSize = 2048`). You can achieve the same effect with the default slider maximum of 256px and `QT_SCALE_FACTOR=2`.
  - For large sizes, using the zoom buttons or scrolling over the slider becomes really tedious. It's still acceptable around 256px, but much too slow up to 512px. This should work more like the progressive zoom slider in View mode.
  
  I'm all for allowing larger thumbnail sizes, but I'm not yet comfortable approving the patch. Is showing a large blurry thumbnail really what we want, or should we rather fix things in the proper places? Ideally the spec could be extended, but in any case we'd have to display and (somewhere) store proper 512px thumbnails (which this patch is not implementing – yet?).
  
  ---
  
  > @ngraham added reviewers: Konsole, tcanabrava, dhaumann.
  
  Sorry to disagree with you there. Yes, the repo is set wrongly, but title, file list and the diff are quite clear in this case.

REPOSITORY
  R260 Gwenview

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

To: sandsmark, ltoscano, gateau, #gwenview, rkflx, #konsole, tcanabrava, dhaumann
Cc: rkflx, ngraham, huoni
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20180716/9b0b9da8/attachment.html>