Review Request 126291: initial implementation of a platform plugin for OS X (WIP)

Fri Mar 3 14:12:20 UTC 2017

> On Dec. 27, 2016, 6:39 p.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowinfo.mm, line 33
> > <https://git.reviewboard.kde.org/r/126291/diff/8/?file=488149#file488149line33>
> >
> >     what's "Ext"?

Stood for Extended.

> On Dec. 27, 2016, 6:39 p.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowinfo_p_cocoa.h, line 74
> > <https://git.reviewboard.kde.org/r/126291/diff/8/?file=488151#file488151line74>
> >
> >     wtf is that?

system macros for Mac devs. Nothing to be seen here if you're not, move along ;)

> On Dec. 27, 2016, 6:39 p.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_mac_p.h, line 24
> > <https://git.reviewboard.kde.org/r/126291/diff/8/?file=488153#file488153line24>
> >
> >     do we really need this? That's way too much ifedery for me. Either we have that feature or not. Have finished things I don't want to see in our code.

In a way that's indeed the question.
Did you read the comment? This is an unfinished bit of code that could lead to a more feature-complete implementation where kwindowsystem can track individual windows also on Mac (rather than only processes.

It needs updating, something I don't want to do until the basic plugin has been committed and used in the wild to assess whether or not there's really need for the feature.
If I were sure that I would be the one finalising the implementation I could discard the code and keep a local copy around, but I am not (sure of that).

I understand it's not nice to keep experimental code #ifdeffed away inline. I see 2 solutions:
- move it to a README.windowtracking file (or something like that)
- remove it in a dedicated commit AFTER committing the current patch, so it will be easier to find for anyone wishing to work on individual window tracking, and the corresponding patch will be more likely to apply at that future moment. (I hope I make myself clear.)

Please tell me which you prefer.

> On Dec. 27, 2016, 6:39 p.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 104
> > <https://git.reviewboard.kde.org/r/126291/diff/8/?file=488154#file488154line104>
> >
> >     is there a possibility that Qt does not use Cocoa?

Yes, given that Qt provides the macro. I have no idea however how feasible it still is to build recent Qt versions against Carbon on recent OS versions. I have thus replaced the #ifdefs with a single #ifndef in the preamble, invoking #error if the macro is not defined.
If it turns out to be a popular enough use case we can always find a solution.

- René J.V.

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

On Dec. 27, 2016, 5 p.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126291/
> -----------------------------------------------------------
> 
> (Updated Dec. 27, 2016, 5 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> KWindowSystem has been lacking a platform plugin for OS X. This RR presents a "backport" of the modified KDE4 KWindowSystem implementation that has been used in the MacPorts kdelibs4 port for the last 2 or 3 (or more) years.
> 
> I have made some initial steps to remove deprecated Carbon API calls, but this is clearly a work in progress.
> 
> Open questions include
> - is there any justification to run an event handler (or Cocoa observer) to keep track of running applications, possibly even listing all their open windows?
> - is there any use for the Qt event listener framework (cf. the NETEventFilter in the X11 plugin)? I haven't yet had time to try to figure out what this "could be good for", and am very open to suggestions in this departments.
> - one application for such an event filter would be a listener that catches the opening and closing of all windows by the running process, and keeps track of their `WId`s. A new method could then be added (to `KWindowInfo`?) to distinguish `WId`s created by the running application from "foreign" ones (useful also on Wayland and MS Windows).
> 
> `KWindowSystem::setMainWindow` should become a front for payload provided by the plugins. I'll leave that to the regular/official maintainer(s) of this framework.
> 
> This code could probably do with *lots* of comments; I'll try to add them as questions about this or that bit of code arise.
> 
> 
> Diffs
> -----
> 
>   src/kwindowsystem.h a282ecd 
>   src/kwindowsystem.cpp fda1682 
>   src/platforms/osx/CMakeLists.txt 4fc3347 
>   src/platforms/osx/cocoa.json PRE-CREATION 
>   src/platforms/osx/kkeyserver.cpp 3ddb921 
>   src/platforms/osx/kwindowinfo.cpp e8555bb 
>   src/platforms/osx/kwindowinfo.mm PRE-CREATION 
>   src/platforms/osx/kwindowinfo_mac_p.h c8f307e 
>   src/platforms/osx/kwindowinfo_p_cocoa.h PRE-CREATION 
>   src/platforms/osx/kwindowsystem.cpp 1758829 
>   src/platforms/osx/kwindowsystem_mac_p.h PRE-CREATION 
>   src/platforms/osx/kwindowsystem_macobjc.mm PRE-CREATION 
>   src/platforms/osx/kwindowsystem_p_cocoa.h PRE-CREATION 
>   src/platforms/osx/plugin.h PRE-CREATION 
>   src/platforms/osx/plugin.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126291/diff/
> 
> 
> Testing
> -------
> 
> On OS X 10.9.5 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20170303/e9504ca0/attachment.html>