block screen filtering for global menu applet
ClosedPublic

Authored by mvourlakos on Feb 18 2019, 7:33 PM.

Details

Summary

--when the user has enabled PLASMA_USE_QT_SCALING
under X11 environment the screen and window geometries
can not be trusted for comparison. In such case
the global menu should be always visible

BUG: 404500

Test Plan

check that global menu still works properly

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mvourlakos created this revision.Feb 18 2019, 7:33 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 18 2019, 7:33 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mvourlakos requested review of this revision.Feb 18 2019, 7:33 PM

If we're going to put in code, lets do:

QPoint windowCentre = info.geometry().center();
if (KWindowSystem::isPlatformX11()) {
  windowCenter /= qApp->scale();
}

I closed the bug because I was quite angry at the user randomly setting variables and then not even mentioning it, but we may as well make use of the fact that the testing found something.

If we're going to put in code, lets do:

QPoint windowCentre = info.geometry().center();
if (KWindowSystem::isPlatformX11()) {
  windowCenter /= qApp->scale();
}

I closed the bug because I was quite angry at the user randomly setting variables and then not even mentioning it, but we may as well make use of the fact that the testing found something.

personally I have no opinion whatever you want...

David for the proposed code does it catch all the cases under X11 ?
For example if the user has set different scale factors between its screens is it going to work?

For example if the user has set different scale factors between its screens is it going to work?

It will not, but there is no UI option for that.

For example if the user has set different scale factors between its screens is it going to work?

It will not, but there is no UI option for that.

one more aspect please, in case you can answer some technical questions:

  1. I heard that under Wayland, Plasma is enabling PLASMA_USE_QT_SCALING is that true?
  2. If [1] is true, the problem of geometries comparison between windows and screens is valid also for wayland by default? OR the issue does not exist in wayland because the compositor solves it?

I heard that under Wayland, Plasma is enabling PLASMA_USE_QT_SCALING is that true?

Yes.

OR the issue does not exist in wayland because the compositor solves it?

The problem does not exist in wayland.
Wayland uses normalised geometry for everything, things "just work" you don't have the problem we have on X of multiple co-ordinate systems at once talking to different parts.

(Whether KWindowInfo::geometry works on wayland is another matter)

The problem does not exist in wayland.

nice, in that case I think that your proposal with a small comment why this is needed is good. I will update shortly.

I tried to adjust to your proposal David but there is no qApp->scale() .

What I found is, QScreen::devicePixelRatio() so we need to know first the screen for which we adjusting things.

How about doing this in qml side and pass to appmenu c++ model not only the plasmoid.screenGeometry but also the Screen.devicePixelRatio ?

I suppose that in multi screen environments with different scales it would also work ok for all screens

There's a QGuiApplication::devicePixelRation()

I gave the wrong method name, sorry.

I suppose that in multi screen environments with different scales it would also work ok for all screens

I'm afraid it's not that simple.

mvourlakos updated this revision to Diff 52168.Feb 20 2019, 6:15 PM
  • follow David comment for devicePixelRation usage
davidedmundson accepted this revision.Feb 20 2019, 6:17 PM

<3 thanks

This revision is now accepted and ready to land.Feb 20 2019, 6:17 PM

David I followed your proposal, in case this is approved I believe that it should be provided also for plasma 5.15.

Do you agree to cherry-pick it from master if it is approved?

This revision was automatically updated to reflect the committed changes.