The scaling of thumbnail previews in the Open/Save dialog was done with the fast transformation that looked out of place and ugly.
This uses the same scaling method as Dolphin to look consistent and nice too.
BUG: 345578
FIXED-IN: 5.46
ngraham | |
elvisangelaccio | |
rkflx |
Frameworks | |
VDG |
The scaling of thumbnail previews in the Open/Save dialog was done with the fast transformation that looked out of place and ugly.
This uses the same scaling method as Dolphin to look consistent and nice too.
BUG: 345578
FIXED-IN: 5.46
Before:
After:
Dolphin for reference uses smooth transformation:
No Linters Available |
No Unit Test Coverage |
What's the performance penalty of that? Does the file dialog request the correct size from the preview job? It shouldn't have to end up in this codepath that often, I think.
Thanks.
Performance is not good right now when you move the zoom slider.
Dolphins performance is good because it has less icon sizes available on the slider and does not do the scaling until the designated zoom value is selected.
I still have to work on this one...
Could we use the fast scaling while resizing, then change to the smooth scaling once the resize operation is done?
Revert resize change.
Turns out the image we got here is already downsized to around 128x128 and we further downsize it (if needed), dependent on the icon size.
Working with this small images the smooth scaling should be fast.
BTW I don't know what caused the performance issue when moving the slider before, maybe something heavy was running in the background, but I can't reproduce the problem anymore.
The XDG thumbnail spec has two thumbnail sizes, 128 and 256. It is possible that you hit the "slow" issue when only one size of thumbnail had been created, but the other was still pending.
EDIT: maybe rm ~/.cache/thumbnails to test
I too cannot reproduce the performance issue, even after nuking ~/.cache/thumbnails and having it regenerate the thumbnails. Can you, @cfeck?
I hit the resizing performance issue without this patch too, so it's unrelated to this.
Definitely a visual improvement! Let's wait for a few more opinions regarding whether this is the right approach technically.
Ping.
We are scaling 128px icons to lower sizes.
In this case smooth transformation does not really affect performance.
Can someone confirm that this is the right approach?
Thanks Kai, great questions. I think I can answer them:
I don't see any performance issues, both when testing the perceived performance, as well as when looking at the code.
Does the file dialog request the correct size from the preview job?
It's always scaling down 128px icons (i.e. the "normal" thumbnail size) and never touches 256px icons ("large"), because 128px is the maximum setting of the slider (icon sizes verified with qDebug). (For HiDPI, thumbnail handling is broken like everywhere, sadly.)
It shouldn't have to end up in this codepath that often, I think.
This code path is called once per item upon entering directories, switching view modes or changing zoom levels, as well as when hovering over an item.
I'd say the patch is okay. We get better visuals with no perceived slow-down (maybe a theoretical performance loss, but not really relevant for those tiny thumbnails, apparently).
As for the intermittent issues described above: Maybe recreating the cache due to 371e523f5d7e is the reason while comparing patched version and system version of the file dialog?
Any objections to landing this tomorrow if we don't hear any objections from Frameworks people before then?