> On Sept. 26, 2013, 2:27 a.m., Fredrik Höglund wrote: > > I'm just going to point out something I know you already know since we've > > discussed it many times; > > that an xcb port of the NETWM classes already exists in a branch. > > Martin Gräßlin wrote: > my aim was to write the unit test and that was the big junk of this work > compared to the xcb changes (which I thought to be easier than trying to > rebase the branch :-( ) > > Fredrik Höglund wrote: > The problem with rebasing the branch was solved a long time ago. > There shouldn't be any conflicts except maybe in CMakeLists.txt files. > > > Martin Gräßlin wrote: > ah, I didn't know that. I will have a look and see whether I can > integrate the unit tests and the bug fixes on top of your branch. > > Kevin Ottens wrote: > Any news about that patch? > > Kevin Ottens wrote: > Should I close this one too next monday?
Last warning: I'll close it next time I do my review round and there's no activity. ;-) - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112896/#review40840 ----------------------------------------------------------- On Sept. 23, 2013, 1:11 p.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112896/ > ----------------------------------------------------------- > > (Updated Sept. 23, 2013, 1:11 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > ------- > > This is a patch series, if needed I can push the branch. > > The patches address the following topics: > * add unit tests for NETRootInfo and NETWinInfo which do not require a > running window manager. Test coverage of netwm.h is: > ** line coverage: 75 % > ** functions coverage: 84 % > ** branch coverage: 62 % > The tests start their own Xvfb to have a clean state and not mess up with > the Window Manager of a user or cause followint tests to fail on the CI system > * areas which are covered by unit tests are changed from XLib to XCB. This > mostly affects changing and reading window properties > * API is changed from XLib types to XCB types. E.g. xcb_window_t instead of > Window. Note: this is an API break, which I consider as necessary, given that > especially the type "Window" is problematic. > * several bugs which were discovered through the added tests are fixed > * NETWinInfo2 is merged into NETWinInfo (marked as TODO KDE5) > * Small wrapper class for intern atom added to KXUtils. Similar code already > in usage in KWin. > > > ===== > Commits: > > commit 2e50845a5d0df436106aeb776e3936691c32a753 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 23 14:31:42 2013 +0200 > > Use XCB for reading properties in NETRootInfo::update > > Viewport handling was incorrect and is adjusted to be read correctly. > > commit 23887726c03c49b4e0021c01f319653d3b9f36c5 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 23 11:41:26 2013 +0200 > > Use XCB for reading properties in NETWinInfo::update > > Those areas which are covered by unit tests are migrated from > XGetWindowProperty to the xcb variant. > > BlocksCompositing had an incorrect read - it expected a string atom, > but actually uses a cardinal. This bug is fixed. > > commit 2731ebbc85eddb885700232f98e0e429cb0e066b > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 23 09:41:28 2013 +0200 > > Use XCB to change the Client dependent properties of NETWinInfo > > Those properties for which we have unit tests are changed to XCB to > either change or delete the property. > > commit 1bb85e440ec0004ef6b18b6fa1855c08c8f6697a > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Fri Sep 20 09:58:09 2013 +0200 > > Unit test for Client aspect of NETWinInfo > > Similar to the NETWinInfo test with WindowManager aspect, just > verifying the properties which can only be set by a Client. > > The test found highlights a few possible problems: > * for some window types a fallback type is specified, but the lenght > is set to 1, thus the fallback type is not set at all > * Fullscreen monitors property is not handled in the event function > > commit 2731ebbc85eddb885700232f98e0e429cb0e066b > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 23 09:41:28 2013 +0200 > > Use XCB to change the Client dependent properties of NETWinInfo > > Those properties for which we have unit tests are changed to XCB to > either change or delete the property. > > commit 1bb85e440ec0004ef6b18b6fa1855c08c8f6697a > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Fri Sep 20 09:58:09 2013 +0200 > > Unit test for Client aspect of NETWinInfo > > Similar to the NETWinInfo test with WindowManager aspect, just > verifying the properties which can only be set by a Client. > > The test found highlights a few possible problems: > * for some window types a fallback type is specified, but the lenght > is set to 1, thus the fallback type is not set at all > * Fullscreen monitors property is not handled in the event function > > commit 448f200ecdd642d1a4c85522eaa7d28072cda873 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Thu Sep 19 13:57:03 2013 +0200 > > Use XCB to change the WM dependent properties of NETWinInfo > > Those properties for which we have unit tests are changed to XCB to > either change or delete the property. > > commit 736024c6c6233a9c66a03972b9dbfbd2e6d383f4 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Thu Sep 19 13:16:03 2013 +0200 > > Add unit test for window manager aspect of NETWinInfo > > Similar to the test for NETRootInfo it tests setting the properties > which can only be set by the window manager. Also running in its own > Xvfb. > > commit 480842406c2c070280a8be8ae3e4af93674369f3 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Thu Sep 19 07:20:57 2013 +0200 > > Merge NETWinInfo2 into NETWinInfo > > Class only existed for BIC changes and had a TODO remove KDE5 marker. > > commit 4e71654d95715f5b44efef80fb95fa9bee295b6c > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Thu Sep 19 07:16:00 2013 +0200 > > Drop desktop argument from NETRootInfo::(set)DesktopGeometry > > The desktop argument was only there because of a different semantic in > an early draft of NETwm and completely unused. Doesn't make sense to > carry it into the next version. > > commit 78a818af6937a01d0464b27c8256b05e9889f692 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Wed Sep 18 14:35:52 2013 +0200 > > Use xcb to change properties in NETRootInfo > > Change is done in all the methods which are covered by the unit test. > > commit a34612f1841cf063a7e5b3e433b45e87cb16ef36 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Wed Sep 18 13:47:16 2013 +0200 > > NETRootInfo opperates on WindowTypeMask not on WindowType > > ::isSupported and ::setSupported for WindowType operated on the wrong > type. This needs to be the mask. > > commit 43ebfba6ee0f309eddb9517af992e8490eef3e24 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Wed Sep 18 12:41:06 2013 +0200 > > Add unit test for NETRootInfo with role WindowManager > > Verifies that the properties are set correctly. The unit test found > problems in the following areas: > * setSupported for window types is taking the types instead of the > mask and by that completely messing up the supported windows. > Test is set to expected fail for those elements > > The unit test starts an Xvfb in the initTestCase and closes it again > when the tests end. The display number is chosen by random in [1,98]. > 0 is skipped as that is the normal X server and 99 is skipped as that's > what KDE's CI infrastructure already uses. > > An explicit Xvfb is started instead of using an already running > X Server to not interfere with already running window managers or > left-over states from other tests. > > It would be even better to have one Xvfb instance per test method, > but this is not possible as the atoms are only resolved once and > thus NETRootInfo is indirectly bound to a Display although the API > kind of implies that it would be able to use different NETRootInfo > for different Displays. > > commit 99dbcd1d352c5d1fb69595899bb653ec195939fa > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 14:20:24 2013 +0200 > > Use xcb datatypes for helper functions in netwm.cpp > > Some more Window -> xcb_window_t > > commit 8a8e0edcb4c697ae8539e8a91885a0e422f30b3a > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 14:16:37 2013 +0200 > > Remove Xlib includes from netwm.h > > We don't need them in the header any more. Only need to forward > declare the Display variable. > > Unfortunately we still need the includes in other parts of > kwindowsystems where Xlib is still used and netwm.h was included. > > commit e4a78dbc3fda5c20a11b6ede708f5390ac399915 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 14:06:49 2013 +0200 > > Use bool/true/false instead of Bool/True/False in netwm > > For our internal API we can just use the proper datatypes instead of > the XLib macros. > > commit 2c86f1898b8c0898a6c9c73cf90dda87b1af5095 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 13:59:48 2013 +0200 > > Use xcb_atom_t instead of Atom in NET(Root|Win)Info > > Not all are replaced as XGetWindowProperty starts to complain if > we pass in a pointer to xcb_atom_t instead of pointer to Atom. > > commit 79be0cbce1719c3fcbc0d3e89a8374192cfd6af7 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 13:50:52 2013 +0200 > > Use xcb_timestamp_t instead of Time in NET(Root|Win)Info > > Same as with Window - using xcb type. > > commit 28ae8e12894ab6cb948772bceee4ac4798417233 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 13:44:39 2013 +0200 > > Use xcb_window_t instead of Window in NETWinInfo > > Just like for NETRootInfo. > > commit c2a8a37926b16b39c0d468de0aec4cfaee0abb5d > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 13:25:09 2013 +0200 > > Switch NETRootInfo from Window to xcb_window_t > > Window is the old XLib type whose name is rather conflicting as > it's not in a namespace. So let's use the XCB type. > > For the properties which take a list of Windows, the list is > still passed as Window and not as xcb_window_t. Reason for this > is that apparently the wrong value gets written if it's the xcb > datatype. This will be removed once it is using xcb to change > the property. > > commit 9ddf0634f4973a0ba4effeddca2b7edda1921e73 > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 12:55:12 2013 +0200 > > Use KXUtils::Atom wrapper for all the items in netwm > > By using the Atom wrapper the code is ported to XCB and the reply is > only retrieved once it is needed. > > commit c63db861ea4224967bb0330e92ef4f914339d7bd > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 12:47:43 2013 +0200 > > Add xcb_connection_t* to NET(Root|Win)InfoPrivate > > For future use: have an xcb_connection_t* created from the Display* > in the private classes. Once we get rid of XLib it should be passed > in through the ctor instead of the Display pointer. > > commit 8b73a064a5773c3cc8b6b2207b7c3f656dbf5afb > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 12:39:18 2013 +0200 > > Add wrapper class for intern atom to KXUtils > > Small wrapper class to simplify request and reply for an intern > atom. > > commit a89950dd09a1aaf15ba196009e50aa137a490a8e > Author: Martin Gräßlin <mgraess...@kde.org> > Date: Mon Sep 16 11:24:12 2013 +0200 > > Add dependency for X11_XCB to KWindowSystems framework > > X11_XCB allows us to transit to XCB gradually. That is without > changing the external API. > > > Diffs > ----- > > tier1/kwindowsystem/autotests/CMakeLists.txt a1a7066 > tier1/kwindowsystem/autotests/netrootinfotestwm.cpp PRE-CREATION > tier1/kwindowsystem/autotests/netwininfotestclient.cpp PRE-CREATION > tier1/kwindowsystem/autotests/netwininfotestwm.cpp PRE-CREATION > tier1/kwindowsystem/src/CMakeLists.txt 31fb53e > tier1/kwindowsystem/src/kstartupinfo.cpp 971a23f > tier1/kwindowsystem/src/kwindowinfo_x11.cpp f382e9c > tier1/kwindowsystem/src/kwindowsystem_x11.cpp 9a1f05b > tier1/kwindowsystem/src/kxutils_p.h 84d639b > tier1/kwindowsystem/src/netwm.h 71877c0 > tier1/kwindowsystem/src/netwm.cpp c4f9989 > tier1/kwindowsystem/src/netwm_p.h d7f4599 > > Diff: http://git.reviewboard.kde.org/r/112896/diff/ > > > Testing > ------- > > see unit tests, for the window manager dependent code areas I want to write > some test application and include it in /test > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel