[platforms/x11/windowed] Port to AbstractOutput
ClosedPublic

Authored by romangg on Feb 21 2019, 6:43 PM.

Details

Summary

To homogenize our backends and as another step to remove the Screens class
use the AbstractOutput class in the windowed X11 backend.

Test Plan

Manually in X session.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
romangg created this revision.Feb 21 2019, 6:43 PM
Restricted Application added a project: KWin. · View Herald TranscriptFeb 21 2019, 6:43 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Feb 21 2019, 6:43 PM
zzag added a subscriber: zzag.Apr 26 2019, 11:45 AM

In general, +1, though useractions popup has very small text now.

plugins/platforms/x11/windowed/x11windowed_backend.cpp
213–217

Do you think it would be worth to introduce a method that can be used for querying an output with a given xcb_window_t? i.e.

X11WindowedOutput *X11WindowedBackend::findOutput(xcb_window_t id) const;

romangg added inline comments.Thu, Jun 6, 8:32 PM
plugins/platforms/x11/windowed/x11windowed_backend.cpp
213–217

Looking at the repetitive code, sure.

romangg updated this revision to Diff 59304.Thu, Jun 6, 11:38 PM
  • General cleanup
In D19207#456572, @zzag wrote:

In general, +1, though useractions popup has very small text now.

You have an idea what the reason could be for that? I didn't find one. Tbh the whole X11 windowed mode is under-developed and needs more work.

zzag added a comment.EditedSat, Jun 8, 1:59 PM

Logical dpi is used to convert point sizes to pixel sizes. I am most certainly sure that our QPA plugin returns trash logicalDpi values. You could check it by running kwin with QT_WAYLAND_FORCE_DPI=96.

The qpa plugin uses QPlatformScreen::logicalDpi() to compute logical dpi of the screen, but QPlatformScreen::logicalDpi() needs proper physical size. :/

zzag added a comment.Sat, Jun 8, 2:08 PM

Perhaps we need to do

setRawPhysicalSize(pixelSize / 96.0 * 25.4);

It's pretty similar to what QPlatformScreen::physicalSize() does.

Logical dpi is used to convert point sizes to pixel sizes.

In modern Qt wayland qpa (qwaylandscreen) this is effectively hardcoded to 96.

We should do the same in our qpa.

zzag added a comment.Tue, Jun 11, 10:11 AM

In modern Qt wayland qpa (qwaylandscreen) this is effectively hardcoded to 96.

Hmm, yeah, I see, though perhaps we still need to set proper physical size.

We should do the same in our qpa.

Ack.

Is there something to change in regards to DPI now here or somewhere else in a separate Patch?

zzag added a comment.Tue, Jun 11, 11:05 AM

You need to set adjusted physical size in this patch and in another one hardcode logical dpi to 96.

romangg updated this revision to Diff 59598.Tue, Jun 11, 2:49 PM
  • Rebase on master
  • Adjust raw physical size
zzag added a comment.EditedTue, Jun 11, 4:16 PM

I have some nitpicks, please address them before landing this revision.

plugins/platforms/x11/windowed/x11windowed_backend.h
119

Coding style nitpick: missing whitespace before *.

plugins/platforms/x11/windowed/x11windowed_output.h
47

I'm pretty sure that QObject has virtual destructor, so we can drop virtual part. On the other hand, we could add override keyword just to be sure that our base class has virtual destructor.

69

**/

zzag accepted this revision.Tue, Jun 11, 4:16 PM
This revision is now accepted and ready to land.Tue, Jun 11, 4:16 PM
This revision was automatically updated to reflect the committed changes.