Add a static setDefaultDevicePixelRatio method to PreviewJob

I have pretty much the patch in kio-extras ready.

In D29397#664536, @meven wrote:

In D29397#663800, @dfaure wrote:

Oh, I thought it was sent as an int. But 8 is QImage::Format_ARGB8565_Premultiplied. Did you mean 0x80?

No I meant 8, since format is passed as quint8 (a single byte, 8-bit) and the format is forced to QImage::Format_ARGB32 "5", so this works.

OK, this works today. But if one day we want to start actually using other image formats, we'll end up with a clash here.
Why not use 0x80 in order to stay away from valid image format values? This seems safer to me, in the long run.

For another stupid question (the first one was already asked by someone else and answered :) ):
Given some generated thumbnails are cached, does the thumbnail cache specification support logical resolution? How would cached thumbnails work cross-screen?

In D29397#666132, @kossebau wrote:

For another stupid question (the first one was already asked by someone else and answered :) ):
Given some generated thumbnails are cached, does the thumbnail cache specification support logical resolution?

I am evolving here the specification : the images stored are stored with their HiDpi size (width * dpr, height * dpr), and then some metadata is written.
When reading the cached version for large, the user expects a 256*256 (modulo aspect ratio) and can find a (256 * dpr, 256 * dpr) image (or can read the devicePixelRatio from metadata)
You can see in https://phabricator.kde.org/D29526#C575294OL738 and line 582 how to detect and handle such a case.

How would cached thumbnails work cross-screen?

Cache thumbnails with low dpr will look blurry on hidpi screen, other than that's not an issue.
We would need applications to update to use Hidpi thumbnails to renew thumbnails.
I might want to evict low dpr thumbnails.

In D29397#666134, @meven wrote:

In D29397#666132, @kossebau wrote:

For another stupid question (the first one was already asked by someone else and answered :) ):
Given some generated thumbnails are cached, does the thumbnail cache specification support logical resolution?

I am evolving here the specification : the images stored are stored with their HiDpi size (width * dpr, height * dpr), and then some metadata is written.

Specification as in, https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html? So there is some work on extending the specs not yet reflected on that page?
How is the metadata written?

How would cached thumbnails work cross-screen?

Cache thumbnails with low dpr will look blurry on hidpi screen, other than that's not an issue.

Cached thumbnails rendered for hidpi (so e.g. being 256x256 with bigger details due to hidpi context) , but used for lowdpi elsewhere will be an issue as well. E.g. when it comes to text rendered with minfontsize like in case of plain text documents, it will be too large then.

I wonder if the thumbnail cache would not need the same extension like the icon spec had, with a @2x variant, to make up for that. There are still a lot of lowdpi devices out there, and they are getting mixed.

In D29397#666153, @kossebau wrote:

In D29397#666134, @meven wrote:

In D29397#666132, @kossebau wrote:

For another stupid question (the first one was already asked by someone else and answered :) ):
Given some generated thumbnails are cached, does the thumbnail cache specification support logical resolution?

I am evolving here the specification : the images stored are stored with their HiDpi size (width * dpr, height * dpr), and then some metadata is written.

Specification as in, https://specifications.freedesktop.org/thumbnail-spec/thumbnail-spec-latest.html? So there is some work on extending the specs not yet reflected on that page?

Thanks for the reference.
I meant I am beyond the spec, adding some by facts, that is not great :/
This spec is showing its age, and would need to evolve to cover HiDPi use case, but I am not editing it ATM.

How is the metadata written?

line 751 of PreviewJob.

How would cached thumbnails work cross-screen?

Cache thumbnails with low dpr will look blurry on hidpi screen, other than that's not an issue.

Cached thumbnails rendered for hidpi (so e.g. being 256x256 with bigger details due to hidpi context) , but used for lowdpi elsewhere will be an issue as well. E.g. when it comes to text rendered with minfontsize like in case of plain text documents, it will be too large then.

I wonder if the thumbnail cache would not need the same extension like the icon spec had, with a @2x variant, to make up for that. There are still a lot of lowdpi devices out there, and they are getting mixed.

That looks like the way to go, I would favor adding normal@2x and large@2x in .cache/thumbnails

ping reviewers

Would it make sense to initialize devicePixelRatio to QGuiApplication::devicePixelRatio() and add an API to change it (if desired) ?

This way we wouldn't need to call a static method in the main of every client app (i.e. D29525 wouldn't be needed).

In D29397#672557, @elvisangelaccio wrote:

Would it make sense to initialize devicePixelRatio to QGuiApplication::devicePixelRatio() and add an API to change it (if desired) ?

It would make KPreviewJob casting a QCoreApplication::instance to QGuiApplication::devicePixelRatio that would be tolerable.

But this would make the feature opt-out : apps could get broken when their previews double their size just when changing KF version.

This way we wouldn't need to call a static method in the main of every client app (i.e. D29525 wouldn't be needed).

This is a single static call for the lifetime of the app, not that hard.
This makes the feature opt-in, meaning the developer has tested it.

ping dear reviewers

Overall seems sane. Two questions though:

In D29397#673758, @ngraham wrote:

Overall seems sane. Two questions though:

Is this @2x suffix standardized?

No but it is already being used for icon caching.

What happens if I'm using 125% scaling, generate some previews, and then switch back to 100% (no scaling?)

When your screen is scaled 125%, the application gets a dpr of 2.
When you go back to 100%. the thumbnails in ~/.cache/thumbnails/large and ~/.cache/thumbnails/normal will be used again.
Meaning you will have two sets of previews.

The images using a pixel ratio are stored in cache in ~/.cache/thumbnails/normal@2x for 256*256 (i.e 128*128@2x) and ~/.cache/thumbnails/large@2x for 512*512 (i.e 256*256@2x)

@meven Have you already got in contact with the other users/maintainers of he thumnbail cache spec about the idea to extend it with support for high dpi? If not, please consider to do so, so things will also work cross-toolkit/platforms in the future there, by being based on an agreed & formalized specification. Not being involved here or having full understanding of the topic, but I would guess your approach with the separate x2 should run into "make sense" responses, so the additional effort might be low for the gain of being based on an official spec.

This would be discussed on https://lists.freedesktop.org/mailman/listinfo/xdg, possible best by cc:ing other potential parties interested in highdpi thumbnails, would need to be researched, possibly some Dolphin contributors know their counterparts in non-KDE FLOSS projects

The approach makes sense then. I agree that making high DPI a part of the FDO spec would be nice, but IMO that shouldn't block this. The approach currently taken is logical and it could be submitted as an extension to the spec later.

In D29397#673830, @ngraham wrote:

The approach makes sense then. I agree that making high DPI a part of the FDO spec would be nice, but IMO that shouldn't block this. The approach currently taken is logical and it could be submitted as an extension to the spec later.

It might be seen logical to us from our current POV, but better to do as desktop developer generations have done before and try to get others on board from the start, once there is a first plan. Would we want others do just extend specs on their own and start to write stuff without namespacing onto common data grounds? I suspect: no :) So we better behave as well as we would like others to do.

Here's where the spec lives, FWIW: https://gitlab.freedesktop.org/xdg/xdg-specs

Expanding it is mostly just a matter of writing up a reasonable proposal in the form of a merge request and getting enough people to agree. Discussing on the mailing list first can help, to gauge people's opinions.

Thanks for starting the discussion about the spec, by what I saw on the mailinglist. Hopefully that soon gets traction by others.

To not have that block this improvement, you could in parallel for now use a "kde" namespaced directories, say "*@kde2x/", where we/you could just use the for-now-KF-only theme extension. And once there is an agreed specification extension, the code could be switched to just any non-namespace shared hidpi folder matching whatever the specification ended up to be.

In D29397#674365, @kossebau wrote:

To not have that block this improvement, you could in parallel for now use a "kde" namespaced directories, say "*@kde2x/", where we/you could just use the for-now-KF-only theme extension. And once there is an agreed specification extension, the code could be switched to just any non-namespace shared hidpi folder matching whatever the specification ended up to be.

Well I don't think it is needed, since those @2x are not used by anyone as far as I can tell.
I'd rather move on here without the spec finalized and then update the implementation once it is finalized.
I don't expect it to be much different from the current implementation, and I hope this implementation serve the specification process since we will have an implementation to debate on.
And this will save two commits that serves no purpose IMO, we are not Web Browsers after all, we are not breaking anyones business.

Alternatively I'd rather wait for the spec to be finalized.
Just to avoid the friction.

The proposed additions to the spec are non-controversial IMO. Let's push that forward. I left a supportive comment in the email thread, so maybe it's time to put together a patch that people can comment on.

Most thumbnailers are completely DPR agnostic, and will create identical thumbnails for large@1 and normal@2. Having both is a waste of disk space and CPU time.

The thumbnailer should have a key in its metadata to tell if it depends on the DPR, and only then different thumbnails should be created.

In D29397#663902, @broulik wrote:

At the risk of asking a stupid question, why?

The text thumbnailer should be able to produce readable text on high dpi, or the folder thumbnailer should render the folder icon sharply and the picture frames non-pixelated

The folder thumbnailer does not care for DPR, the folder backdrop is rendered at full native resolution, and the sub-thumnails are rendered with a scale ~0.45 on top.

The text thumbnailer currently renders previews in the file list with a height of either 256 or 128, and then scales it down. For an icon size of e.g. 64 the 128px thumb is scaled down, so the effective DPR becomes nativeDpr * 0.5. Assuming a native DPR of 2.0 (HiDPI), should the text in this case be rendered with a DPR of 1.0 or 2.0? One will be blurry, and the other one will hardly contain any text at all (how many characters can you squeeze into 48x64px?).

For dolphin, there is actually another reason for blurriness. The thumbnail icons are scaled so after adding the drop shadow the icon has the selected size - at 256px, the 256px thumbnail is scaled to 250px. For natural images (Photos, Video thumbnails, cover images ...) this is hardly visible, but very noticeable for text.

FYI on this patch serie, I am currently focusing on the specification work : https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/35
Then I am thinking about moving my patch serie to gitlab reflecting on the last feedback of course.
Those patchs are getting old and are going to need some degree of rebasing.

Specification work is so slow I am hesitant to bypass it for now and work on code, even if it means resync with spec later.
https://gitlab.freedesktop.org/xdg/xdg-specs/-/merge_requests/35