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

[René J . V . Bertin]

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



https://phabricator.kde.org/D5069

- René J.V. Bertin


On March 3, 2017, 6:23 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 March 3, 2017, 6:23 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 27e9e09 
>   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
> 
>

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

[René J . V . Bertin]

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

(Updated March 3, 2017, 6:23 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

Updated version against git/master, taking most comments into account.

I don't understand how I can have missed or ignored those revision requests for 
almost a year, and regret that's the case because at the time the potential 
interest of that `EXPERIMENTAL_WINDOW_TRACKING` feature and the reason I 
decided to hold off on finalising it were almost certainly fresher in my mind.


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 (updated)
-

  src/kwindowsystem.h 27e9e09 
  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

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

[René J . V . Bertin]

> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > src/kwindowsystem.cpp, lines 465-467
> > 
> >
> > I would prefer to not introduce new platform specific code in the 
> > shared part.
> > 
> > The idea you have here is right, but it's not OSX specific, the same 
> > applies for e.g. Wayland
> 
> René J.V. Bertin wrote:
> I agree. This function should probably be moved to the plugin. That's 
> just not a responsability I'd love to take for X11 and Wayland, otherwise I'd 
> have implemented this in my patch.
> 
> Can you or one of the other KWindowSystem devs take care of this on the 
> current supported platforms, so that I can adapt this patch?
> 
> René J.V. Bertin wrote:
> Ping?

Dropping this part for now then.


- René J.V.


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


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
> 
>

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

[René J . V . Bertin]

> On Dec. 27, 2016, 6:39 p.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowinfo.mm, line 33
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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 
>   

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

[Martin Gräßlin]

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




src/kwindowsystem.cpp (lines 728 - 732)


why did you ifdef the section? It's just a runtime switch not depending on 
any platform specific libraries like the X11 case.



src/platforms/osx/kwindowinfo.mm (line 33)


what's "Ext"?



src/platforms/osx/kwindowinfo.mm (line 36)


?



src/platforms/osx/kwindowinfo.mm (lines 38 - 40)


why are there both public and private variables? For a d-ptr class I don't 
really understand this



src/platforms/osx/kwindowinfo.mm (lines 273 - 276)


?



src/platforms/osx/kwindowinfo.mm (line 336)


QByteArrayLiteral



src/platforms/osx/kwindowinfo.mm (line 346)


QByteArrayLiteral



src/platforms/osx/kwindowinfo.mm (line 349)


I'm pretty sure the NETWM spec is irrelevant on cocoa



src/platforms/osx/kwindowinfo_p_cocoa.h (line 74)


wtf is that?



src/platforms/osx/kwindowsystem.cpp (lines 47 - 53)


?



src/platforms/osx/kwindowsystem_mac_p.h (line 24)


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.



src/platforms/osx/kwindowsystem_macobjc.mm (lines 63 - 65)


?



src/platforms/osx/kwindowsystem_macobjc.mm (line 104)


is there a possibility that Qt does not use Cocoa?



src/platforms/osx/kwindowsystem_macobjc.mm (line 108)


?



src/platforms/osx/kwindowsystem_macobjc.mm (lines 111 - 113)


?



src/platforms/osx/kwindowsystem_macobjc.mm (lines 137 - 138)


?



src/platforms/osx/kwindowsystem_macobjc.mm (lines 286 - 295)


?



src/platforms/osx/kwindowsystem_macobjc.mm (lines 301 - 324)


?



src/platforms/osx/kwindowsystem_macobjc.mm (line 518)


?



src/platforms/osx/plugin.h (line 2)


I'm certainly not the author of this header file



src/platforms/osx/plugin.cpp (line 2)


Also not the author of this file.


- Martin Gräßlin


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 

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

[René J . V . Bertin]

---
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.


Changes
---

bump

refactored against v4.100.0-rc1-189-g824d4f2 for 5.30.0


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 (updated)
-

  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

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

[René J . V . Bertin]

> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > src/kwindowsystem.cpp, lines 465-467
> > 
> >
> > I would prefer to not introduce new platform specific code in the 
> > shared part.
> > 
> > The idea you have here is right, but it's not OSX specific, the same 
> > applies for e.g. Wayland
> 
> René J.V. Bertin wrote:
> I agree. This function should probably be moved to the plugin. That's 
> just not a responsability I'd love to take for X11 and Wayland, otherwise I'd 
> have implemented this in my patch.
> 
> Can you or one of the other KWindowSystem devs take care of this on the 
> current supported platforms, so that I can adapt this patch?

Ping?


- René J.V.


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


On March 4, 2016, 8:19 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 March 4, 2016, 8:19 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
> -
> 
>   autotests/CMakeLists.txt 65ed8d4 
>   autotests/kwindowsystem_threadtest.cpp a142bae 
>   src/debug_p.h 5ef8996 
>   src/debug_p.cpp 72abfb7 
>   src/kwindowsystem.cpp 407a67d 
>   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
> 
>

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

[René J . V . Bertin]

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

(Updated March 4, 2016, 8:19 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

This revision has a more complete (as complete as possible?) internal WId 
registry feature. WIds on OS X are tricky in that they do not usually 
correspond to `NSWindow` instances but instead to an `NSView` instance. As such 
there is no guarantee to receive any kind of notification when the NSView is 
created, which (I think) means that one has to take stock of an application's 
windows and views at certain times like during appropriate calls to 
KWindowSystem.

With these latest modifications, the `kwindowsystem_threadtest` autotest 
completes successfully, but only when doing an explicit action after 
`m_widget->show()` that causes the widget to be displayed (basically, the 
equivalent of a call to `XSync()`).


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 (updated)
-

  autotests/CMakeLists.txt 65ed8d4 
  autotests/kwindowsystem_threadtest.cpp a142bae 
  src/debug_p.h 5ef8996 
  src/debug_p.cpp 72abfb7 
  src/kwindowsystem.cpp 407a67d 
  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

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

(Updated Feb. 28, 2016, 6:15 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

I've activated the only autotest that can be ported with only trivial changes. 
I've also made some progress with the internal WId registry; it doesn't update 
yet when new windows are created so not all of the tests from 
kwindowsystem_threadtest succeed ATM.

I had a look at moving `setMainWindow()` method to the plugin architecture (and 
calling it through the plugin on OS X only). I got stuck on a compiler error 
I'd never seen before, about `connect()` being undefined. I've reverted those 
changes for now for lack of time to pursue that issue.


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 (updated)
-

  autotests/CMakeLists.txt 65ed8d4 
  autotests/kwindowsystem_threadtest.cpp a142bae 
  src/debug_p.h 5ef8996 
  src/debug_p.cpp 72abfb7 
  src/kwindowsystem.cpp 407a67d 
  src/platforms/osx/CMakeLists.txt 4fc3347 
  src/platforms/osx/SCRAPBOOK PRE-CREATION 
  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > I don't like the introduction of the SCRAPBOOK. The repository is not the 
> > place for dead and old code. That's what git already supports with keeping 
> > the whole history.
> > 
> > We have some autotests for kwindowsystem. Could you try whether the tests 
> > work also on OSX?
> 
> René J.V. Bertin wrote:
> I agree to a certain extent. Git's history feature isn't exactly 
> comparable to a history book in which you can go look around to see if at 
> some point maybe someone contributed some code that was never finished. At 
> the very least you'd need to leave comments in the code that "here used to be 
> some junk DNA that maybe could have led to a whole better world" (and in that 
> case, why not just leave in the code #ifdeffed-out ...)
> 
> As to the autotests: they're built only when `X11_FOUND`. Are you in fact 
> asking me to port them?
> 
> Martin Gräßlin wrote:
> > At the very least you'd need to leave comments in the code that "here 
> used to be some junk DNA that maybe could have led to a whole better world" 
> (and in that case, why not just leave in the code #ifdeffed-out ...)
> 
> eh no, we are not StarOffice. We have a version control system. No need 
> to reference old code.
> 
> > they're built only when X11_FOUND. Are you in fact asking me to port 
> them?
> 
> No, of course not. There are some which might not be X11 specific. E.g. 
> kwindowsystem_threadtest.cpp. It would be good to know which ones can work on 
> OSX
> 
> René J.V. Bertin wrote:
> I'll see for the autotests.
> 
> My point is that you cannot expect that a future developer will study the 
> old commit diffs to check if maybe they contained latent/experimental code 
> that implemented an idea s/he was planning to test.
> The experimental code I've moved is exactly of that nature; it aimed to 
> implement a mechanism by which a table would be maintained of all windows 
> belonging to all running applications. You pointed out correctly that it 
> wouldn't compile and on my end I am not sure whether such a feature would 
> actually be useful (never got feedback on the question I asked about that). 
> So yeah, when the idea of cleaning out inactive code came up and you didn't 
> seem adverse to the idea of keeping code snippets in another place for future 
> reference I came up with the SCRAPBOOK idea.
> Apparently I should have left in the code until I got the green light, 
> and then remove it in an additional commit that does only that, with a 
> sufficiently detailed commit message that the revision history could actually 
> be of some real help here. I can't say I feel like moving all that code back 
> to where it came from, so if you don't want the SCRAPBOOK file I guess it'll 
> just do down the drain.

Re: kwindowsystem_threadtest : that's the only one that doesn't require 
rewriting outside of the autotests CMake file. And it confirms a fear I had: 
there currently is no such thing as an internal list of owned windows, despite 
some efforts to maintain internal structures that allow to return something 
useful in certain situations (that I'll really need to look into again).
Maintaining such a list of windows feels a bit like a chicken-and-egg problem; 
can I rely on a Qt signal or do I need to use a lower-level API. The fact that 
most Qt/KDE applications do not have a Window menu like most Cocoa applications 
do 
(https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Tasks/UsingWindowsMenu.html)
 may be an indication that Qt does something peculiar.

Anyway, supposing I'm going to sit down and try to come up with a mechanism to 
maintain a list of the running app's open windows (`WId`s). Can I presume that 
the list will be created (= plugin loaded) when there are no windows yet, 
meaning I only have to listen to window creation events? Or will I need to 
fetch the current list, add that, and then listen for window open events? I 
know how to go from a `WId` to the underlying Cocoa instance, but the other way 
round may be quite a bit more tricky.


- René J.V.


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


On Feb. 23, 2016, 10:54 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 Feb. 23, 2016, 10:54 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 

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

[René J . V . Bertin]

> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > I don't like the introduction of the SCRAPBOOK. The repository is not the 
> > place for dead and old code. That's what git already supports with keeping 
> > the whole history.
> > 
> > We have some autotests for kwindowsystem. Could you try whether the tests 
> > work also on OSX?
> 
> René J.V. Bertin wrote:
> I agree to a certain extent. Git's history feature isn't exactly 
> comparable to a history book in which you can go look around to see if at 
> some point maybe someone contributed some code that was never finished. At 
> the very least you'd need to leave comments in the code that "here used to be 
> some junk DNA that maybe could have led to a whole better world" (and in that 
> case, why not just leave in the code #ifdeffed-out ...)
> 
> As to the autotests: they're built only when `X11_FOUND`. Are you in fact 
> asking me to port them?
> 
> Martin Gräßlin wrote:
> > At the very least you'd need to leave comments in the code that "here 
> used to be some junk DNA that maybe could have led to a whole better world" 
> (and in that case, why not just leave in the code #ifdeffed-out ...)
> 
> eh no, we are not StarOffice. We have a version control system. No need 
> to reference old code.
> 
> > they're built only when X11_FOUND. Are you in fact asking me to port 
> them?
> 
> No, of course not. There are some which might not be X11 specific. E.g. 
> kwindowsystem_threadtest.cpp. It would be good to know which ones can work on 
> OSX

I'll see for the autotests.

My point is that you cannot expect that a future developer will study the old 
commit diffs to check if maybe they contained latent/experimental code that 
implemented an idea s/he was planning to test.
The experimental code I've moved is exactly of that nature; it aimed to 
implement a mechanism by which a table would be maintained of all windows 
belonging to all running applications. You pointed out correctly that it 
wouldn't compile and on my end I am not sure whether such a feature would 
actually be useful (never got feedback on the question I asked about that). So 
yeah, when the idea of cleaning out inactive code came up and you didn't seem 
adverse to the idea of keeping code snippets in another place for future 
reference I came up with the SCRAPBOOK idea.
Apparently I should have left in the code until I got the green light, and then 
remove it in an additional commit that does only that, with a sufficiently 
detailed commit message that the revision history could actually be of some 
real help here. I can't say I feel like moving all that code back to where it 
came from, so if you don't want the SCRAPBOOK file I guess it'll just do down 
the drain.


- René J.V.


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


On Feb. 23, 2016, 10:54 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 Feb. 23, 2016, 10:54 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/debug_p.h 5ef8996 
>   

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

[Martin Gräßlin]

> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > I don't like the introduction of the SCRAPBOOK. The repository is not the 
> > place for dead and old code. That's what git already supports with keeping 
> > the whole history.
> > 
> > We have some autotests for kwindowsystem. Could you try whether the tests 
> > work also on OSX?
> 
> René J.V. Bertin wrote:
> I agree to a certain extent. Git's history feature isn't exactly 
> comparable to a history book in which you can go look around to see if at 
> some point maybe someone contributed some code that was never finished. At 
> the very least you'd need to leave comments in the code that "here used to be 
> some junk DNA that maybe could have led to a whole better world" (and in that 
> case, why not just leave in the code #ifdeffed-out ...)
> 
> As to the autotests: they're built only when `X11_FOUND`. Are you in fact 
> asking me to port them?

> At the very least you'd need to leave comments in the code that "here used to 
> be some junk DNA that maybe could have led to a whole better world" (and in 
> that case, why not just leave in the code #ifdeffed-out ...)

eh no, we are not StarOffice. We have a version control system. No need to 
reference old code.

> they're built only when X11_FOUND. Are you in fact asking me to port them?

No, of course not. There are some which might not be X11 specific. E.g. 
kwindowsystem_threadtest.cpp. It would be good to know which ones can work on 
OSX


- Martin


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


On Feb. 23, 2016, 10:54 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 Feb. 23, 2016, 10:54 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/debug_p.h 5ef8996 
>   src/debug_p.cpp 72abfb7 
>   src/kwindowsystem.cpp 407a67d 
>   src/platforms/osx/CMakeLists.txt 4fc3347 
>   src/platforms/osx/SCRAPBOOK PRE-CREATION 
>   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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > I don't like the introduction of the SCRAPBOOK. The repository is not the 
> > place for dead and old code. That's what git already supports with keeping 
> > the whole history.
> > 
> > We have some autotests for kwindowsystem. Could you try whether the tests 
> > work also on OSX?

I agree to a certain extent. Git's history feature isn't exactly comparable to 
a history book in which you can go look around to see if at some point maybe 
someone contributed some code that was never finished. At the very least you'd 
need to leave comments in the code that "here used to be some junk DNA that 
maybe could have led to a whole better world" (and in that case, why not just 
leave in the code #ifdeffed-out ...)

As to the autotests: they're built only when `X11_FOUND`. Are you in fact 
asking me to port them?


> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > src/kwindowsystem.cpp, lines 465-467
> > 
> >
> > I would prefer to not introduce new platform specific code in the 
> > shared part.
> > 
> > The idea you have here is right, but it's not OSX specific, the same 
> > applies for e.g. Wayland

I agree. This function should probably be moved to the plugin. That's just not 
a responsability I'd love to take for X11 and Wayland, otherwise I'd have 
implemented this in my patch.

Can you or one of the other KWindowSystem devs take care of this on the current 
supported platforms, so that I can adapt this patch?


> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/plugin.h, line 2
> > 
> >
> > I'm not the author of this file

Actually, you apparently are the author of the template I used to create this 
file :)

(It's an oversight I didn't add myself though)


- René J.V.


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


On Feb. 23, 2016, 10:54 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 Feb. 23, 2016, 10:54 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/debug_p.h 5ef8996 
>   src/debug_p.cpp 72abfb7 
>   src/kwindowsystem.cpp 407a67d 
>   src/platforms/osx/CMakeLists.txt 4fc3347 
>   src/platforms/osx/SCRAPBOOK PRE-CREATION 
>   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/

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

[Martin Gräßlin]

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



I don't like the introduction of the SCRAPBOOK. The repository is not the place 
for dead and old code. That's what git already supports with keeping the whole 
history.

We have some autotests for kwindowsystem. Could you try whether the tests work 
also on OSX?


src/kwindowsystem.cpp (lines 465 - 467)


I would prefer to not introduce new platform specific code in the shared 
part.

The idea you have here is right, but it's not OSX specific, the same 
applies for e.g. Wayland



src/platforms/osx/plugin.h (line 2)


I'm not the author of this file



src/platforms/osx/plugin.cpp (line 2)


I'm not the author of this file


- Martin Gräßlin


On Feb. 23, 2016, 10:54 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 Feb. 23, 2016, 10:54 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/debug_p.h 5ef8996 
>   src/debug_p.cpp 72abfb7 
>   src/kwindowsystem.cpp 407a67d 
>   src/platforms/osx/CMakeLists.txt 4fc3347 
>   src/platforms/osx/SCRAPBOOK PRE-CREATION 
>   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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

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

(Updated Feb. 23, 2016, 10:54 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

Some more code cleanup, and I got rid of most dependencies on the Carbon 
framework


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 (updated)
-

  src/debug_p.h 5ef8996 
  src/debug_p.cpp 72abfb7 
  src/kwindowsystem.cpp 407a67d 
  src/platforms/osx/CMakeLists.txt 4fc3347 
  src/platforms/osx/SCRAPBOOK PRE-CREATION 
  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > Thanks for contributing the code, I'm very happy to see this happening! 
> > Sorry, that I cannot provide a good review as I'm not really into the OSX 
> > API, thus my review here might look a little bit nitpicky.
> > 
> > General comment: maybe rename the directory from "osx" to "cocoa"? After 
> > all it's more about the windowing system than the operating system.
> 
> René J.V. Bertin wrote:
> Nitpickyness is no problem here.
> 
> I have nothing against renaming the subdirectory, but it'd be something 
> I'd want to leave to the end, for convenience on my end. Also, we'd probably 
> want to remain consistent, or work toward consistency. Currently there are no 
> "cocoa" backend/plugin directories; 3 frameworks use "osx" and another uses 
> "mac". There is also the question to what extent "cocoa" still is and will 
> remain the most appropriate term in this context ... and there is the issue 
> of iOS where some frameworks might be deployed and which might or might not 
> require specific adaptations. (And I'm probably hardly more "into" the iOS 
> APIs as you are into the OS X APIs.)
> 
> Martin Gräßlin wrote:
> sure, we can think about a more general approach to the naming. Having 
> that standardized over all framework might make sense (assuming we never care 
> about the difference between operating system and windowing system or it's 
> always windowing system).

On systems (platforms) where there is no choice of windowing and/or rendering 
system one could settle for a single term (question is, which ...).
Another approach would be to follow Qt's QPA naming, as far as that's relevant. 
That would have the benefit that the name choice would be standardised all the 
way up to the `-platform` argument available to the end user (on platforms 
supporting multiple QPAs).
It would also fit in with messages like
`org.kde.kwindowsystem: Loaded plugin 
"/Volumes/Debian/MP9/share/qt5/plugins/kf5/org.kde.kwindowsystem.platforms/KF5WindowSystemCocoaPlugin.so"
 for platform "cocoa"`


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 89
> > 
> >
> > Why is the define needed? Doesn't it make more sense to just not build 
> > the OSX backend if COCOA is disabled?
> 
> René J.V. Bertin wrote:
> That would require knowing the answer to that question in the CMake file; 
> I'd have to look into that. Is it trivial to do the equivalent of an `#ifdef 
> TOKEN` in a CMake file?
> 
> Martin Gräßlin wrote:
> That should be defined in the cmake config for Qt5Gui. At least where I 
> needed similar things in the past it was defined.

I cannot find any variable/flag that indicates against what SDK Qt has been 
built, sadly.


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, lines 331-342
> > 
> >
> > why support setting the current desktop if the number of desktops is 
> > hardcoded to 1?
> 
> René J.V. Bertin wrote:
> Is there an accepted way to include a kind of scrapbook with code 
> snippets that might be useful or "kinda work"?
> 
> Martin Gräßlin wrote:
> good question. I don't know of one.

This will need some more testing, but rather than hardcoding the number of 
desktops to 1 one could adapt the number when more desktops are discovered (= 
the max. observed "current desktop number").
KWindowSystem::numberOfDesktops() can change at runtime anyway, can't it?


- René J.V.


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


On Feb. 14, 2016, 5:44 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 Feb. 14, 2016, 5:44 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. 

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

[René J . V . Bertin]

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

(Updated Feb. 17, 2016, 12:23 a.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 (updated)
-

  src/debug_p.h 5ef8996 
  src/debug_p.cpp 72abfb7 
  src/kwindowsystem.cpp 407a67d 
  src/platforms/osx/CMakeLists.txt 4fc3347 
  src/platforms/osx/SCRAPBOOK PRE-CREATION 
  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[Martin Gräßlin]

> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > Thanks for contributing the code, I'm very happy to see this happening! 
> > Sorry, that I cannot provide a good review as I'm not really into the OSX 
> > API, thus my review here might look a little bit nitpicky.
> > 
> > General comment: maybe rename the directory from "osx" to "cocoa"? After 
> > all it's more about the windowing system than the operating system.
> 
> René J.V. Bertin wrote:
> Nitpickyness is no problem here.
> 
> I have nothing against renaming the subdirectory, but it'd be something 
> I'd want to leave to the end, for convenience on my end. Also, we'd probably 
> want to remain consistent, or work toward consistency. Currently there are no 
> "cocoa" backend/plugin directories; 3 frameworks use "osx" and another uses 
> "mac". There is also the question to what extent "cocoa" still is and will 
> remain the most appropriate term in this context ... and there is the issue 
> of iOS where some frameworks might be deployed and which might or might not 
> require specific adaptations. (And I'm probably hardly more "into" the iOS 
> APIs as you are into the OS X APIs.)

sure, we can think about a more general approach to the naming. Having that 
standardized over all framework might make sense (assuming we never care about 
the difference between operating system and windowing system or it's always 
windowing system).


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 89
> > 
> >
> > Why is the define needed? Doesn't it make more sense to just not build 
> > the OSX backend if COCOA is disabled?
> 
> René J.V. Bertin wrote:
> That would require knowing the answer to that question in the CMake file; 
> I'd have to look into that. Is it trivial to do the equivalent of an `#ifdef 
> TOKEN` in a CMake file?

That should be defined in the cmake config for Qt5Gui. At least where I needed 
similar things in the past it was defined.


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, lines 331-342
> > 
> >
> > why support setting the current desktop if the number of desktops is 
> > hardcoded to 1?
> 
> René J.V. Bertin wrote:
> Is there an accepted way to include a kind of scrapbook with code 
> snippets that might be useful or "kinda work"?

good question. I don't know of one.


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 446
> > 
> >
> > what's experimental window tracking?
> > 
> > And that ifdef if enabled would not compile
> 
> René J.V. Bertin wrote:
> The whole EXPERIMENTAL_WINDOW_TRACKING feature is an experiment started 
> by someone who worked on this code before me. From what I understand, the 
> idea was to catch window creation and destruction events to maintain some 
> kind of internal database that would allow to implement a number of the 
> windowing features that aren't supported by the underlying layers. I don't 
> think the feature was ever completed, and it also uses deprecated APIs.
> 
> I left it in because I thought it could fuel more discussion about 
> whether or not such a feature could make sense to complete the feature set. 
> But it could probably play that same role when moved to the aforementioned 
> scrapbook or some other kind of "junk DNA" file.

the way it's currently I would highly recommend to remove it as it just cannot 
compile.


- Martin


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


On Feb. 14, 2016, 5:44 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 Feb. 14, 2016, 5:44 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 

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

[René J . V . Bertin]

> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > Thanks for contributing the code, I'm very happy to see this happening! 
> > Sorry, that I cannot provide a good review as I'm not really into the OSX 
> > API, thus my review here might look a little bit nitpicky.
> > 
> > General comment: maybe rename the directory from "osx" to "cocoa"? After 
> > all it's more about the windowing system than the operating system.

Nitpickyness is no problem here.

I have nothing against renaming the subdirectory, but it'd be something I'd 
want to leave to the end, for convenience on my end. Also, we'd probably want 
to remain consistent, or work toward consistency. Currently there are no 
"cocoa" backend/plugin directories; 3 frameworks use "osx" and another uses 
"mac". There is also the question to what extent "cocoa" still is and will 
remain the most appropriate term in this context ... and there is the issue of 
iOS where some frameworks might be deployed and which might or might not 
require specific adaptations. (And I'm probably hardly more "into" the iOS APIs 
as you are into the OS X APIs.)


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 89
> > 
> >
> > Why is the define needed? Doesn't it make more sense to just not build 
> > the OSX backend if COCOA is disabled?

That would require knowing the answer to that question in the CMake file; I'd 
have to look into that. Is it trivial to do the equivalent of an `#ifdef TOKEN` 
in a CMake file?


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, lines 111-113
> > 
> >
> > please no commented code

Will disappear from the final code, of course, but probably not as long as I 
still might need this kind of test probe ;)


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, lines 331-342
> > 
> >
> > why support setting the current desktop if the number of desktops is 
> > hardcoded to 1?

Is there an accepted way to include a kind of scrapbook with code snippets that 
might be useful or "kinda work"?


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 446
> > 
> >
> > what's experimental window tracking?
> > 
> > And that ifdef if enabled would not compile

The whole EXPERIMENTAL_WINDOW_TRACKING feature is an experiment started by 
someone who worked on this code before me. From what I understand, the idea was 
to catch window creation and destruction events to maintain some kind of 
internal database that would allow to implement a number of the windowing 
features that aren't supported by the underlying layers. I don't think the 
feature was ever completed, and it also uses deprecated APIs.

I left it in because I thought it could fuel more discussion about whether or 
not such a feature could make sense to complete the feature set. But it could 
probably play that same role when moved to the aforementioned scrapbook or some 
other kind of "junk DNA" file.


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 175
> > 
> >
> > Please use categorized logging. I suggest adding a new category for the 
> > OSX backend.

Roger.


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, lines 302-303
> > 
> >
> > then please remove the code

OK.


> On Feb. 15, 2016, 8:42 a.m., Martin Gräßlin wrote:
> > src/platforms/osx/kwindowsystem_macobjc.mm, line 447
> > 
> >
> > QObject()

Indeed.


- René J.V.


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


On Feb. 14, 2016, 5:44 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 Feb. 14, 2016, 5:44 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 
> 

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

[Martin Gräßlin]

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



Thanks for contributing the code, I'm very happy to see this happening! Sorry, 
that I cannot provide a good review as I'm not really into the OSX API, thus my 
review here might look a little bit nitpicky.

General comment: maybe rename the directory from "osx" to "cocoa"? After all 
it's more about the windowing system than the operating system.


src/platforms/osx/kwindowsystem_macobjc.mm (line 89)


Why is the define needed? Doesn't it make more sense to just not build the 
OSX backend if COCOA is disabled?



src/platforms/osx/kwindowsystem_macobjc.mm (lines 111 - 113)


please no commented code



src/platforms/osx/kwindowsystem_macobjc.mm (line 175)


Please use categorized logging. I suggest adding a new category for the OSX 
backend.



src/platforms/osx/kwindowsystem_macobjc.mm (lines 302 - 303)


then please remove the code



src/platforms/osx/kwindowsystem_macobjc.mm (lines 331 - 342)


why support setting the current desktop if the number of desktops is 
hardcoded to 1?



src/platforms/osx/kwindowsystem_macobjc.mm (line 446)


what's experimental window tracking?

And that ifdef if enabled would not compile



src/platforms/osx/kwindowsystem_macobjc.mm (line 447)


QObject()


- Martin Gräßlin


On Feb. 14, 2016, 5:44 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 Feb. 14, 2016, 5:44 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.cpp 407a67d 
>   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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

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

(Updated Feb. 14, 2016, 5:44 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

Updated for the current source and tested against FWs 5.19.0

It would be nice to get some feedback on this one, if not simply a simple "Ship 
It" ...


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 (updated)
-

  src/kwindowsystem.cpp 407a67d 
  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

[René J . V . Bertin]

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

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/platforms/osx/plugin.cpp PRE-CREATION 
  src/platforms/osx/plugin.h PRE-CREATION 
  src/platforms/osx/kwindowsystem_macobjc.mm PRE-CREATION 
  src/platforms/osx/kwindowsystem_p_cocoa.h PRE-CREATION 
  src/platforms/osx/kwindowsystem_mac_p.h PRE-CREATION 
  src/platforms/osx/kwindowsystem.cpp 2fcec18 
  src/platforms/osx/kwindowinfo_mac_p.h 26ff790 
  src/platforms/osx/kwindowinfo_p_cocoa.h PRE-CREATION 
  src/platforms/osx/kwindowinfo.mm PRE-CREATION 
  src/platforms/osx/cocoa.json PRE-CREATION 
  src/platforms/osx/kwindowinfo.cpp 328d6a9 
  src/kwindowsystem.cpp 9ef273f 
  src/platforms/osx/CMakeLists.txt 4fc3347 

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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel