fix high DPI scaling of RGB overview waveforms

[Be-ing]

before:

after:

I was going to fix the Filtered and HSV overviews too, but they actually look okay already.

[Be-ing]

@ninomp could you review this?

[daschuer]

LGTM, though I have not tested it yet.

[uklotzde]

devicePixelRatioF() was introduced in Qt 5.6. I assume our minimum supported Qt version is still 5.2? Then the scaling code has to be guarded by QT_VERSION_CHECK(5, 6, 0)

[daschuer]

Good catch. Does m_waveformRenderer->getDevicePixelRatio() work instead?

[ninomp]

I have just tested this and it LGTM.

[daschuer]

Please try to use m_waveformRenderer->getDevicePixelRatio()

[Be-ing]

There is no member m_waveformRenderer. How about we merge this to 2.2 then revert the last commit after merging to master?

[daschuer]

Ah, I see. Please use the static function:
WaveformWidgetFactory::getDevicePixelRatio()

[Be-ing]

That would not be as robust as the current implementation. WaveformWidgetFactory::getDevicePixelRatio() gets the device pixel ratio for the focused window, which is not necessarily the same window that the WOverview widget is in. The focused window could be on a different monitor with a different device pixel ratio. I think QPaintDevice::devicePixelRatioF() will always return the correct ratio.

[daschuer]

Yes, but currently we have only one window.
All other users from WaveformWidgetFactory::getDevicePixelRatio() suffer this issue.
So it is not necessary to solve it here. We may considder to make a wrapper that conditional one or the other implementation for all places.

[Be-ing]

Yes, but currently we have only one window.

No, we have the preferences window, the track properties window, and cover art window apart from the main window.

All other users from WaveformWidgetFactory::getDevicePixelRatio() suffer this issue.

This is a bug that should be fixed in master, not exacerbated in this PR.

We may considder to make a wrapper that conditional one or the other implementation for all places.

I don't think a wrapper can work. QPaintDevice::devicePixelRatioF should be called from within the widget; I don't think a static function could work.

[daschuer]

Yes, but currently we have only one window.

No, we have the preferences window, the track properties window, and cover art window apart from the main window.

Sure, but we use QGuiApplication::focusWindow() which is the correct one in this case here.

All other users from WaveformWidgetFactory::getDevicePixelRatio() suffer this issue.

This is a bug that should be fixed in master, not exacerbated in this PR.

This is not a bug, that is required to fix. You was complaining about the a less robust solution. This is a minor issue compared to "no solution" for < QT 5.6 builds.

A wrapper needs to receive the "this" pointer to work. To get the parent window we can crawl up the widget tree. But I am just fine with using WaveformWidgetFactory::getDevicePixelRatio() and fix bugs when they arrive.

[Be-ing]

Getting the device pixel ratio from the focused window is not the best solution. For example, if the preferences window is open on another screen with a different device pixel ratio, that will make the waveforms be scaled incorrectly. Unfortunately I do not have any external monitors to test this with.

[daschuer]

The moving waveform store the ratio during initialization. We should do this here as well.
I think we have no other chance for > Qt 5.6.
A warpper with conditional code would help in recent QT versions. I also don't mind to have the ifdef in every using place. I have counted three.

[Be-ing]

The moving waveform store the ratio during initialization.

So it will be wrong if the waveform is moved to a screen with a different pixel ratio.

I think we have no other chance for > Qt 5.6.

That's fine, we still have no evidence anyone is using Qt <5.5 on a high DPI screen today.

[uklotzde]

Ready for merge? I'm ok with disabling the improvements for outdated Qt versions. This won't add any untested workarounds with the risk of unknown side effects.

[daschuer]

Not OK for me. We use the workaround elsewhere in the code and IMHO it is required to use the same solution in all places.

[daschuer]

I can prepare a PR against this PR if you like.

[uklotzde]

If there is already existing code then both variants should be combined into a utility function for calculating the ratio. Otherwise it becomes tedious to remove all the scattered scaling code.

[daschuer]

That was my idea as well.

[Be-ing]

The existing code is wrong. It assumes that the widget being scaled is in the currently focused window, which may or may not be true. The focused window may be on a screen with a different pixel ratio, in which case the scaling would be wrong. Again, this is a problem that should be fixed but it is beyond the scope of this PR. Exacerbating the problem by making more calls to incorrect code is IMO a bad way to proceed. There is no need for a utility function; every QPaintDevice (which QWidget inherits) has the devicePixelRatioF function already which will return the appropriate ratio for that widget.

[uklotzde]

A QPaintDevice should be passed to the utility function. It may decide to ignore this parameter and use some other means to determine the ratio for Qt < 5.6. With this strategy we could at least isolate the wrong calculation.

[daschuer]

@Be-ing: shoold I jump in or do you?

[Be-ing]

I have already gone out of my way to support an old Qt version. I'm not doing any more work to support old versions of Qt which will have to be reverted in master anyway. All that has to be done is merge this as is to 2.2, merge 2.2 to master, then revert 341ec91 in master.

[daschuer]

A solution is here:
Be-ing#21

[daschuer]

In may case the software moving waveforms, still have double resolution in the device pixel ration of 2 case. Can you confirm that with QT > 5.6?

[daschuer]

Looking at the screenshots, I think we have also a blurry issue with the spinny.
I think the issue is around:
WImageStore::getImageNoCache
because we paint here into a buffer, which has no associated window.
A solution may involve:
pImage->setDevicePixelRatioF(targetWidgetDevicePixelRatio);
Unfortunately it is drawn too big in my case.
Is this fixed with QT > 5.6?
Related:
https://stackoverflow.com/questions/42011410/qt-drawing-high-dpi-qpixmaps

[Be-ing]

Yes, there is another issue with scaling cover art. This affects everywhere cover art is used. I attempted to solve it a few weeks ago but was not successful.

[Be-ing]

WImageStore::getImageNoCache

I think there are multiple bugs. I see this function is called from WaveformRenderMark::generateMarkImage, and that could explain why graphics for cues are blurry. But I think there is another similar issue in CoverArtUtils and/or CoverArtCache, which do not use WImageStore.