The string format by default is Vendor Model (output), where Vendor
and Model are read from EDID and output is the name of the output
reported by the platform.
This patch adds logic to skip the (output) part if its the same as
Vendor Model.
No Linters Available |
No Unit Test Coverage |
Buildable 23822 | |
Build 23840: arc lint + arc unit |
This is needed for D10042 (which I'm rebasing right now) when even on KWayland the KScreen::Output will have EDID attached (assembled from information provided by KWayland/KWin). Since on KWayland the output name already is Vendor Model adding the information from EDID ends up with the string Vendor Model (Vendor Model), that is in my case Eizo Nanao Corporation DP-5-EV2736W/96211045 (Eizo Nanao Corporation DP-5-EV2736W/96211045), which is really long and kinda pointless.
I am currently working on some projects. But if you don't hear till Sunday from me about this, ping me.
Ok, I see the issue. The question is if Vendor Model is not a bit stupid as a name for an output and if KWin/KWayland should not be a bit more intelligent about choosing a name. Then it could make sense again to have (Vendor model) appended afterwards. For example I have here a monitor for testing which is called on the website: PROLITE XUB2395WSU-B1. But KWin returns as Vendor "IVM" and as name "DP-1-PL2395W/11638JJ60250".
common/utils.cpp | ||
---|---|---|
36 | Please don't use single character variable names. Also can this be made more explicit? I can't guess directly what the meaning of a, b and s is. I don't think it is that generic, is it? | |
58–63 | Full-stop in the end. |
common/utils.cpp | ||
---|---|---|
64 | If I am reading this correctly, you could keep the old code and use simplified(): then later on: return name + QLatin1Char('(') + outName + QLatin1Char(')'); } |
As you described, the system makes sense on X11 where you get something like HDMI-1 (Sony GigaTV-12345) so you see which device it is and what physical output it's connected to. Since KWin does not use to output names but only the names of the actually connected devices, the needed behavior is different.
I don't know if it's safe/feasible to change this on KWin side or not.
common/utils.cpp | ||
---|---|---|
64 | Not really, the catch is in the whitespace at the end of name (then "Vendor Model " == "Vendor Model" is never true) |
Allright, let's go with your solution now and if somebody thinks it can be improved on KWin side it can be revisited later.
Please handle the nitpicks I raised. When you are at it also add two to three comments to the lambda making it clearer what it does in each line and why. Thanks!
Sorry for late review. I didn't get notified by this bullshit platform (which might be because I disabled all notifications at one point to not get spammed constantly).
common/utils.cpp | ||
---|---|---|
46 | full-stop |