KScreen now shows "Dell Inc." instead of DEL and
"Eizo Nano Corporation" instead of ENC in output names, which
matches closer to what's written on my monitors.
Details
- Reviewers
graesslin davidedmundson - Group Reviewers
Plasma - Commits
- R108:33a1777a5ab0: Decode full monitor vendor name from EDID using hwdata
Diff Detail
- Repository
- R108 KWin
- Branch
- arcpatch-D10041
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 22914 Build 22932: arc lint + arc unit
@mart: what will be the impact on plamashell if the provided values are changed?
I thought you said in the other review that this interface is not to be used by anyone else but kscreen :-)
plugins/platforms/drm/drm_output.cpp | ||
---|---|---|
401 | The path is somewhat standardized between all distros. There are some distros (like Arch) that simply don't have this file because they use different upstream source (gentoo vs. fedora) that is missing that but from I can tell they still use /usr/share/hwdata for all distros I checked. I can add a cmake check if you want. |
I thought you said in the other review that this interface is not to be used by anyone else but kscreen :-)
The data which is set on the OutputDevice is also synced into the Output (see line 350). And to my knowledge plasmashell uses this information for mapping containments to it.
plugins/platforms/drm/drm_output.cpp | ||
---|---|---|
401 |
Yep, I would prefer this. Just to also make clear at compile time that it's missing. I don't like hidden dependencies which fail on some distros. |
And to my knowledge plasmashell uses this information for mapping containments to it.
Not really. We use QScreen name. QScreen's name is:
("Screen%1").arg(id))
where id is the literal wl_id.
It's pretty rubbish
This will change when we implement XdgOutputV2 (which is still in review) and when that gets into Qt.
In any case it's not a reason to block this.
I can add a cmake check if you want.
Marking as "request changes" to remove it from the queue.
This causes a build failure for me:
/home/nate/kde/src/kwin/plugins/platforms/drm/edid.cpp:149:19: error: expected primary-expression before ‘enum’ 149 | QFile pnpFile(QStringLiteral(HWDATA_PNPIDS_FILE)); | ^~~~~~~~~~~~~~
config-kwin.h.cmake | ||
---|---|---|
28 | So it's not a 'RUNTIME' build dependency as stated in the main CMakeLists.txt. It's an 'OPTIONAL' one |
Same thing here:
[ 94%] Building CXX object plugins/platforms/drm/CMakeFiles/KWinWaylandDrmBackend.dir/edid.cpp.o /data/kde/src/kwin/plugins/platforms/drm/edid.cpp:149:34: error: expected ')' QFile pnpFile(QStringLiteral(HWDATA_PNPIDS_FILE)); ^
cmake/modules/Findhwdata.cmake | ||
---|---|---|
38 | The correct syntax is if NOT hwdata_DIR_NAMES OR NOT hwdata_PNPIDS_FILE) -NOTFOUND evaluates to FALSE [1] | |
config-kwin.h.cmake | ||
47 | This produces: #if HAVE_HWDATA /* #undef HWDATA_PNPIDS_FILE */ #endif |
cmake/modules/Findhwdata.cmake | ||
---|---|---|
38 | The correct syntax is if (NOT hwdata_DIR_NAMES OR NOT hwdata_PNPIDS_FILE) -NOTFOUND evaluates to FALSE [1] |