[KDE/Mac] Review Request 120931: [OS X] improvements to KWindowSystem

[Martin Gräßlin]

> On Nov. 15, 2014, 9:41 a.m., Martin Gräßlin wrote:
> > kdeui/tests/kwindowtest.cpp, lines 218-239
> > <https://git.reviewboard.kde.org/r/120931/diff/2/?file=328515#file328515line218>
> >
> >     please note that your test application is fragile as it's based on sleep which is dangerous on an async windowing system.
> >     
> >     If you want to perform proper tests have a look at the auto-tests in KWindowSystem framework. I would be very pleased if it gets an OSX port and also the auto-tests adjusted.
> 
> René J.V. Bertin wrote:
>     Concerning KF5: there is currently only 1 person who could check if these modifications *compile* and possibly if a well-written test succeeds - Marko.
>     It is my understanding that improving existing features (like OS X support) falls in the category of bug fixes according to Albert Astals's definitions. If this gets committed to an OS X specific branch that's fine with me too.
>     
>     Considering the test: I don't think the test application does async things and I'm not even sure if it really has an event loop. But you're right, using sleep is a very quick hack that I ought to have replaced by something using QTimer or equivalent. Note however that this is an addition to an existing test case from KWindowSystem; where are the auto-tests that you mention?

> according to Albert Astals's definitions.

if Albert acks this change than I'm happy as well. But please note that I'm highly concerned about the changes getting lost due to increasing differences between 4.x and 5.x. Personally I would say the changes should go into frameworks first and be backported to 4.x and not the other way around.

> I don't think the test application does async things

yes they do :-) On X11 calls like setCurrentDesktop are async.

> where are the auto-tests that you mention?

in frameworks.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120931/#review70388
-----------------------------------------------------------


On Nov. 15, 2014, 12:04 a.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120931/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2014, 12:04 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X and kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This is an attempt to improve the Mac-specific implementation of the `KWindowSystem` class.
> For convenience and future-proofness (and also because I like the language) I converted `kwindowsystem_mac.cpp` to ObjC++, i.e. `kwindowsystem_mac.mm`, and added the AppKit framework in the CMakeFile.
> 
> Much of the code in this file is hardly better than gentle hacking, but that probably concerns the functions that are of least interest on a platform where KDE doesn't do session management.
> 
> I should probably update the "not yet implemented" debug statements (to "unsupported"), and I might have another look at kwindowinfo_mac.cpp too.
> 
> 
> Diffs
> -----
> 
>   kdeui/CMakeLists.txt 1454790 
>   kdeui/tests/kwindowtest.cpp b4012d7 
>   kdeui/windowmanagement/kwindowsystem_mac.cpp 4200237 
>   kdeui/windowmanagement/kwindowsystem_mac_p.h PRE-CREATION 
>   kdeui/windowmanagement/kwindowsystem_macobjc.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/120931/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.6.8, mostly with the updated kwindowtest utility (which calls KWindowSystem functions when clicking the Open button in its toolbar).
> Also tested on Mac OS X 10.9.4 rebuilding kdelibs from scratch.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20141115/82fb6902/attachment-0001.html>