feat: parse EDID on the backend side
AcceptedPublic

Authored by dvratil on Jan 23 2018, 12:14 PM.

Details

Reviewers
sebas
romangg
Group Reviewers
Plasma
Summary

Parse EDID on the backend side so that backends that don't have access
to raw EDID blob can still populate the Edid object with data they have
available.

This allows clients (KScreen KCM, mostly) to not rely on the output name
constructed by KWin, but can assemble the string on its own.

Test Plan

kscreen-console on Wayland now shows partially populated EDID info.

Diff Detail

Repository
R110 KScreen Library
Branch
arcpatch-D10042
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23815
Build 23833: arc lint + arc unit
dvratil created this revision.Jan 23 2018, 12:14 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 23 2018, 12:14 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
dvratil requested review of this revision.Jan 23 2018, 12:14 PM
sebas accepted this revision.Jan 30 2018, 3:34 PM

One minor thing to fix, otherwise the idea + approach look fine to me. Thanks!

src/edid_p.h
3

typo

This revision is now accepted and ready to land.Jan 30 2018, 3:34 PM
dvratil updated this revision to Diff 76459.Feb 26 2020, 12:14 PM
  • Rebased on master
dvratil retitled this revision from Parse EDID on the backend side to feat: parse EDID on the backend side.Feb 26 2020, 12:15 PM
dvratil edited the summary of this revision. (Show Details)
dvratil edited the test plan for this revision. (Show Details)
dvratil added reviewers: Plasma, romangg.
dvratil requested review of this revision.Feb 26 2020, 12:18 PM

The idea to parse on the backend side is sound. Can you paste the output you get in a Wayland session? For me it shows the XWayland EDID of the outputs.

backends/kwayland/waylandbackend.cpp
80

Let the backend populate the Private here is unusual, or is it not? On the other side it makes sense. Let the consumer only access the public interface and the backend access the private one. I don't know what's the commonly used approach to that in Qt dev. Out of interest you can say? In any case we can leave it like that for now.

89

A WaylandOutput::edid() function should be put in the WaylandOutput class.

dvratil marked an inline comment as done.Tue, Mar 17, 2:23 PM
dvratil added inline comments.
backends/kwayland/waylandbackend.cpp
80

I found this to be the simplest solution to avoid exposing some setters/factory functions into Edid'public interface. It's all internal code, so no risk of API/ABI breakages...

dvratil updated this revision to Diff 77825.Tue, Mar 17, 2:24 PM
dvratil edited the summary of this revision. (Show Details)
  • Rebase on current master
  • Move Edid construction to WaylandOutput::edid

Here's kscreen-console output for one of my monitors under Wayland with this patch applied:

[15:31:16.520] kscreen-console(422251:422251) unknown: Id:  3
[15:31:16.520] kscreen-console(422251:422251) unknown: Name:  "Eizo Nanao Corporation DP-7-EV2736W/96211045"
[15:31:16.520] kscreen-console(422251:422251) unknown: Type:  "DisplayPort"
[15:31:16.520] kscreen-console(422251:422251) unknown: Connected:  true
[15:31:16.520] kscreen-console(422251:422251) unknown: Enabled:  true
[15:31:16.520] kscreen-console(422251:422251) unknown: Primary:  true
[15:31:16.520] kscreen-console(422251:422251) unknown: Rotation:  2
[15:31:16.520] kscreen-console(422251:422251) unknown: Pos:  QPoint(0,0)
[15:31:16.520] kscreen-console(422251:422251) unknown: MMSize:  QSize(600, 340)
[15:31:16.520] kscreen-console(422251:422251) unknown: FollowPreferredMode:  false
[15:31:16.520] kscreen-console(422251:422251) unknown: Size:  QSize(-1, -1)
[15:31:16.520] kscreen-console(422251:422251) unknown: Scale:  1
[15:31:16.520] kscreen-console(422251:422251) unknown: Clones:  None
[15:31:16.520] kscreen-console(422251:422251) unknown: Mode:  "0"
[15:31:16.520] kscreen-console(422251:422251) unknown: Preferred Mode:  "0"
[15:31:16.520] kscreen-console(422251:422251) unknown: Preferred modes:  ("0")
[15:31:16.520] kscreen-console(422251:422251) unknown: Modes: 
[15:31:16.520] kscreen-console(422251:422251) unknown: 	 "0"    "2560x1440@60"   QSize(2560, 1440)   59.951
[15:31:16.520] kscreen-console(422251:422251) unknown: 	 "1"    "2560x1440@30"   QSize(2560, 1440)   29.935
[15:31:16.520] kscreen-console(422251:422251) unknown: 	 "10"    "1280x720@60"   QSize(1280, 720)   60
[15:31:16.520] kscreen-console(422251:422251) unknown: 	 "11"    "1280x720@60"   QSize(1280, 720)   59.94
[15:31:16.520] kscreen-console(422251:422251) unknown: 	 "12"    "1024x768@60"   QSize(1024, 768)   60.004
[15:31:16.520] kscreen-console(422251:422251) unknown: 	 "13"    "800x600@60"   QSize(800, 600)   60.317
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "14"    "720x480@60"   QSize(720, 480)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "15"    "720x480@60"   QSize(720, 480)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "16"    "720x480@60"   QSize(720, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "17"    "720x480@60"   QSize(720, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "18"    "720x480@60"   QSize(720, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "19"    "640x480@60"   QSize(640, 480)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "2"    "1920x1200@60"   QSize(1920, 1200)   59.885
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "20"    "640x480@60"   QSize(640, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "21"    "640x480@60"   QSize(640, 480)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "22"    "720x400@70"   QSize(720, 400)   70.082
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "3"    "1920x1080@60"   QSize(1920, 1080)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "4"    "1920x1080@60"   QSize(1920, 1080)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "5"    "1920x1080@60"   QSize(1920, 1080)   59.94
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "6"    "1600x1200@60"   QSize(1600, 1200)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "7"    "1280x1024@60"   QSize(1280, 1024)   60.02
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "8"    "1280x960@60"   QSize(1280, 960)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	 "9"    "1280x720@60"   QSize(1280, 720)   60
[15:31:16.521] kscreen-console(422251:422251) unknown: EDID Info: 
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Device ID:  "xrandr-Eizo Nanao Corporation-DP-7-EV2736W/96211045"
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Name:  "DP-7-EV2736W/96211045"
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Vendor:  "Eizo Nanao Corporation"
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Serial:  ""
[15:31:16.521] kscreen-console(422251:422251) unknown: 	EISA ID:  ""
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Hash:  "0aaedf5ca2"
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Width:  60
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Height:  34
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Gamma:  0
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Red:  QQuaternion(scalar:1, vector:(0, 0, 0))
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Green:  QQuaternion(scalar:1, vector:(0, 0, 0))
[15:31:16.521] kscreen-console(422251:422251) unknown: 	Blue:  QQuaternion(scalar:1, vector:(0, 0, 0))
[15:31:16.521] kscreen-console(422251:422251) unknown: 	White:  QQuaternion(scalar:1, vector:(0, 0, 0))

Thanks, so I'm talking about this line:

[15:31:16.521] kscreen-console(422251:422251) unknown: 	Device ID:  "xrandr-Eizo Nanao Corporation-DP-7-EV2736W/96211045"

There it says xrandr, which should likely be not there.

Agreed. deviceId is not actually used (it's stored in KDED config files, but the value doesn't seem to be used anywhere). I'll fix it in a separate commit (since it also needs adjusting a bunch of unit-tests in libkscreen and kscreen).

romangg accepted this revision.Tue, Mar 31, 2:34 PM

Agreed. deviceId is not actually used (it's stored in KDED config files, but the value doesn't seem to be used anywhere). I'll fix it in a separate commit (since it also needs adjusting a bunch of unit-tests in libkscreen and kscreen).

That's ok. Thanks!

CMakeLists.txt
4

Why project version change?

This revision is now accepted and ready to land.Tue, Mar 31, 2:34 PM