this replaces the approach with the expose event in
20b439a4f4a by directly monitoring the xcb screen change
notify native event
Details
- Reviewers
davidedmundson sebas - Group Reviewers
Plasma - Commits
- R120:44c703dcb41c: use a native event filter to notice the screen was swapped
R871:668aba4dfa27: use a native event filter to notice the screen was swapped
R120:668aba4dfa27: use a native event filter to notice the screen was swapped
R871:44c703dcb41c: use a native event filter to notice the screen was swapped
attaching and detaching the external screen on a laptop
configured to deactivate the internal screen upon connection
same behavior as D3777
Diff Detail
- Repository
- R120 Plasma Workspace
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Code-wise, it looks good. let's get it into master quickly, and if we get no complaints, backport to 5.8?
shell/shellcorona.h | ||
---|---|---|
117 | whitespace between * and var name |
Given the only member variable we use is m_screenPool-> shouldn't this just be in ScreenPool?
Also I don't understand how this is works:
qxcbconnection::handleXcbEvent
calls ::filterNativeEvent
before ::handleScreenChange
which means this code is running before qGuiApp->primaryScreen has updated.
the weird thing is that i'm getting a lot of screen change events, so that's probably why it works even if it's called in the wrong order.
putting a bit of debug i get this:
Name of current primary screen "eDP-1" stored name in screenpool "eDP-1"
Name of current primary screen "eDP-1" stored name in screenpool "eDP-1"
Name of current primary screen ":0.0" stored name in screenpool "eDP-1"
Name of current primary screen "HDMI-1" stored name in screenpool ":0.0"
Name of current primary screen "HDMI-1" stored name in screenpool "HDMI-1"
can be seen going from the internal screen to the fake one, then from the fake one to the external one
shell/screenpool.cpp | ||
---|---|---|
203 | Wondering whether returning false always is correct. It feels wrong but I may be missing something. |
shell/screenpool.cpp | ||
---|---|---|
203 | always returning false means you don't want to ever block such events from further management, which is correct. |
shell/screenpool.cpp | ||
---|---|---|
37 | No need to install if not on X? |
shell/screenpool.cpp | ||
---|---|---|
188 | Is that call to X really neccessary everytime? It's called constantly for all kinds of events. Also at least cache the QX11Info::connection() ... |