[wayland] Add new XdgOutput properties

Authored by davidedmundson on Mar 24 2020, 12:24 AM.



AbstractOutput::name() behaviour is changed so that it matches the X11
behaviour, showing an identifier like "HDMI-0".

XdgOutput.name is set to this name.

XdgOutput.description is currently set to the manufacturer name and
model, but it's not exposed to Qt so we probably don't care too much.

This should fix plasmashell changing applets when switching between X11
and wayland.

Test Plan

Relevant unit test
I still need to run it on my laptop.

Diff Detail

R108 KWin
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
Restricted Application added a project: KWin. · View Herald TranscriptMar 24 2020, 12:24 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
davidedmundson requested review of this revision.Mar 24 2020, 12:24 AM
zzag added a subscriber: zzag.Mar 24 2020, 7:55 AM

What patch do I need to apply to KWayland to test this change?


Please rename AbstractOutput::name() to AbstractOutput::description() to keep AbstractOutput::name() and XdgOutputInterface::name() in sync.

davidedmundson added inline comments.Mar 24 2020, 8:57 AM

I had looked. Without bringing in further behavioural changes it makes things worse.

You then need to change screens to be in sync, and that one gets used a lot.

Then instead of having one piece of code refer to name when they mean description, we have dozens refer to description when they just want a simple readable name.

davidedmundson planned changes to this revision.Mar 24 2020, 9:41 AM
davidedmundson added inline comments.

Maybe it's not so bad.

It seems on X11 the name is in the form HDMI-0 which is what we call "name" here.

So this actually is an opportunity to make things more consistent.

zzag added inline comments.Mar 24 2020, 9:48 AM

So, I assume you're talking about output mapping for touch devices. If that's the case, we could set the udev 'WL_OUTPUT` property on the input device, but nobody does it. Alternatively, we could follow libinput's documentation [1] and use something else.

My main concern about this code is that we start mixing identifiers, names, and descriptions, which makes code more difficult to work with and reason about.

[1] https://wayland.freedesktop.org/libinput/doc/latest/api/group__device.html#gaf48626f6190e9c9bc14abb704e66cc22

zzag added inline comments.Mar 24 2020, 10:24 AM

We probably don't even need to bother with libinput_device_get_output_name() as long as our output names match weston's output names.

I also believe that libinput_device_get_output_name() returns NULL in many cases since I don't see who sets WL_OUTPUT. Either way, worth checking...

davidedmundson edited the summary of this revision. (Show Details)
davidedmundson removed a subscriber: zzag.



No I was referring to OutputScreens::name(int screen)

Following that, it gets used in the UI in context menus.

I initially was trying to avoid a behavioural change on that, but it turns out we probably want that.

zzag accepted this revision.Mar 24 2020, 10:55 AM
zzag added inline comments.

nit: Wrap "FB-0" in QStringLiteral.

This revision is now accepted and ready to land.Mar 24 2020, 10:55 AM
This revision was automatically updated to reflect the committed changes.