- User Since
- Apr 18 2015, 8:19 AM (110 w, 9 h)
Tue, May 23
I wouldn't call it color correction as we used to have a mechanism called color correction in the past which did something very different. Personal suggestion: gamma correction or color temperature correction.
Mon, May 22
Thu, May 18
Tue, May 16
Eh what? That is not thread save?!? Oh sh*** I didn't expect that. Good catch and a nice solution.
like David explained: we don't need the change signal and IMHO we shouldn't expose "wrong API". I consider having a changed signal for a PID wrong API as it indicates that the PID could change. But it won't ever change, so we shouldn't expose it. I know that all other attributes have a changed signal, but they can change.
Sun, May 14
Sat, May 13
Can reproduce the failing test locally. Please fix ASAP! Broken tests is not acceptable in KWayland. This change has been on review so long, I find it quite disappointing that it was pushed without checking whether the tests work.
Please note that I'm going to revert all changes which are built up on a broken API change in KWayland. We need to take quality serious, so if the KWayland regression doesn't get fixed asap I will have to revert everything.
xdg_shell v6 should improve the situation. Let's wait for support on xdg shell v6 prior to further work on it.
Fri, May 12
Your tests in plasma window management do not test the new requests.
It's KWin. Who gets the information from the socket.
Thu, May 11
Forget my last two comments - confused the review
Are you sure the API is so compatible that you can run it like that?
Wed, May 10
@mart could you please incorporate the suggested change and then push to 5.8?
Tue, May 9
Mon, May 8
As it depends on newer frameworks: commit embargo till 5.10 is branched.
Obviously commit embargo till 5.10 is branched is still in place.
Please also extend the test in autotests/client/test_wayland_windowmanagement.cpp
Why was this change pushed although it had a "This has commit embargo till 5.10 is branched." comment? Yes I accepted it, but obviously the embargo still holds. Even more the commit builds up on changes in KWayland which are not pushed.
Please note that the code you wrote will give you an incorrect PID for all XWayland windows. Instead of going through the ClientConnection I suggest to use KWin::Toplevel::pid(). That one returns the correct pid for all X windows. But it would crash for Wayland windows currently. So make this method virtual and override it in ShellClient and add the code for getting the pid from the surface in ShellClient.
Commit embargo till 5.10 is branched.
This has commit embargo till 5.10 is branched.
Sun, May 7
I assume this are adjustments for Qt 5.8 or later. How does this work with Qt 5.7? I fear if we change like this we are going to break Qt 5.7.
Please also extend the autotests/client/test_wayland_windowmanagement.cpp
Please fix the style issues.
Some additional information: terminateClientConnections needs to happen before we destroy the workspace and our compositor. The problem is that when we terminate the connections there is still cleanup code running through the protocol. E.g. if we have an internal QWindow (example Alt+Tab) the ShellClient for it will be destroyed when we terminate the client connection. Destroying the ShellClient interacts with Compositor and Workspace. Thus things might crash when going down. Thus the terminateClientConnection is exactly at that place prior to destroying the compositor to ensure it does not crash or access already freed memory.
WaylandServer is a Qobject child of Application. Please be extremely careful with the destroying of the Wayland connection. I spent days in making the sequence so that asan doesn't complain any more.
Sat, May 6
New version at D5731
Going for Thomas's suggestion to check in setX11Time
I had already investigated the problem, pushed a change and reverted it. I think the qstyle gets destroyed too early.
This is genius, using release! I spent hours on thinking how to properly clean that up and always came to the conclusion it's not possible as we cannot properly clean up because we don't get informed when the connection is destroyed.
This caused a regression for XWayland windows. On Wayland QX11Info::getTimestamp always returns 0 and thus our timestamp gets set to 0.
Fri, May 5
Thanks for adding the test!
Please add a unit test case.
Thu, May 4
Wed, May 3
as the one who implemented this, I have to say that I disagree with the argumentation. I think it is wrong to activate all and fear that this could result in annoying issues especially as launchers close when losing focus. This could result in random launcher activating.
How are users supposed to know about this? I'm strictly against adding further "magic" to PW or to implement personal features.