<table><tr><td style="">rkflx added a project: Gwenview.<br />rkflx edited reviewers, added: Gwenview; removed: Konsole, tcanabrava, dhaumann.<br />rkflx requested changes to this revision.<br />rkflx added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D6083">View Revision</a></tr></table><br /><div><div><p>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!</p>
<blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D6083#292991" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D6083#292991</a>, <a href="https://phabricator.kde.org/p/sandsmark/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@sandsmark</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D6083#114550" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D6083#114550</a>, <a href="https://phabricator.kde.org/p/gateau/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@gateau</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>I love the idea, but what about caching? Cached thumbnails are supposed to follow <a href="https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html#THUMBSIZE" class="remarkup-link" target="_blank" rel="noreferrer">https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html#THUMBSIZE</a>, which still, sadly, says 256.</p></div>
</blockquote>
<p>it says the sizes are just suggestions, so I don't think bumping it up a bit would violate this.</p></div>
</blockquote>
<p>Quotes from the spec:</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">Normal: […]The image size must not exceed 128x128 pixel.</li>
<li class="remarkup-list-item">Large: […] must be 256x256 pixel.</li>
<li class="remarkup-list-item">However, these are suggestions. Implementations should cope also with images that are smaller […]</li>
</ul></blockquote>
<p>I don't see anything in there about larger thumbnails.</p>
<p>Some observations running Gwenview with this patch:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">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?</li>
<li class="remarkup-list-item">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 <tt style="background: #ebebeb; font-size: 13px;">MaxThumbnailSize = 2048</tt>). You can achieve the same effect with the default slider maximum of 256px and <tt style="background: #ebebeb; font-size: 13px;">QT_SCALE_FACTOR=2</tt>.</li>
<li class="remarkup-list-item">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 <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">View</span></span></span> mode.</li>
</ul>
<p>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?).</p>
<hr class="remarkup-hr" />
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p><a href="https://phabricator.kde.org/p/ngraham/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@ngraham</a> added reviewers: Konsole, tcanabrava, dhaumann.</p></blockquote>
<p>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.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R260 Gwenview</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D6083">https://phabricator.kde.org/D6083</a></div></div><br /><div><strong>To: </strong>sandsmark, ltoscano, gateau, Gwenview, rkflx, Konsole, tcanabrava, dhaumann<br /><strong>Cc: </strong>rkflx, ngraham, huoni<br /></div>