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
Details
Details
- Reviewers
ngraham elvisangelaccio rkflx - Group Reviewers
Frameworks VDG - Maniphest Tasks
- T8552: Polish Open/Save dialogs
- Commits
- R241:1b721c4b25e4: Thumbnail smooth scaling in filepicker
Before:
After:
Dolphin for reference uses smooth transformation:
Diff Detail
Diff Detail
- Repository
- R241 KIO
- Branch
- image_smooth_downscale (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
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.
Comment Actions
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...
Comment Actions
Could we use the fast scaling while resizing, then change to the smooth scaling once the resize operation is done?
Comment Actions
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.
Comment Actions
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.
Comment Actions
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
Comment Actions
I too cannot reproduce the performance issue, even after nuking ~/.cache/thumbnails and having it regenerate the thumbnails. Can you, @cfeck?
Comment Actions
I hit the resizing performance issue without this patch too, so it's unrelated to this.
Comment Actions
Definitely a visual improvement! Let's wait for a few more opinions regarding whether this is the right approach technically.
Comment Actions
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?
Comment Actions
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?
Comment Actions
Any objections to landing this tomorrow if we don't hear any objections from Frameworks people before then?