Review Request 128567: Add a convenient API to query the windowing system/platform used by Qt

[Martin Gräßlin]

> On Aug. 1, 2016, 10:36 p.m., Sune Vuorela wrote:
> > src/kwindowsystem.h, line 587
> > <https://git.reviewboard.kde.org/r/128567/diff/1/?file=472716#file472716line587>
> >
> >     Something about the word platform feels wrong here. But windowsystem also feels a bit wrong. But I guess Platform is the term used by Qt, so maybe it is okay.

I deliberately decided for the name "Platform" as it's used by Qt.


> On Aug. 1, 2016, 10:36 p.m., Sune Vuorela wrote:
> > src/kwindowsystem.h, line 617
> > <https://git.reviewboard.kde.org/r/128567/diff/1/?file=472716#file472716line617>
> >
> >     These convenience helpers just seems ... candidates for extreme growth. isPlatformSurfaceFlinger isPlatformWindows isPlatformWindowsRT isPlatformMacOsForWatch isPlatformMacOsForTV isPlatform.... Do we need them?

Based on the usage in kde/workspace already I think we do need convenience helpers for the most common usages. Otherwise it might happen that people prefer if (QX11Info::isPlatformX11()) over if (KWindowSystem::platform() == KWindowSystem::Platform::X11).

Also I don't expect that we will ever see support for these "exotic" platforms here in KWindowSystem. I'll only accept new platforms which also have a platform implementation. I don't expect that KWindowSystem will support Android or iOs.


> On Aug. 1, 2016, 10:36 p.m., Sune Vuorela wrote:
> > autotests/kwindowsystem_platform_wayland_test.cpp, line 61
> > <https://git.reviewboard.kde.org/r/128567/diff/1/?file=472714#file472714line61>
> >
> >     Doesn't this stop when first wait fails?

yes, that's fine. It would mean that in overall 5500 msec the socket did not appear. Which can be put as "Weston failed to start". The reason for the loop is rather to handle situation that other files are created before the socket file (e.g. the lock file of the socket) and we really wait for the socket to appear.


- Martin


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


On Aug. 1, 2016, 5:27 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128567/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2016, 5:27 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> -------
> 
> A common pattern for our applications which have to do platform specific
> tasks is to query the used platform with code like:
> if (QX11Info::isPlatformX11()) {
>     // do X11 specific stuff
> } else if (QGuiApplication::platformName().startsWith(QLatin1String("wayland"))) {
>     // do Wayland specific stuff
> }
> 
> The problem with that is that it involves string comparisons and is easy
> to get wrong. E.g. for X11 one should use the QX11Info helper, for
> Wayland one has to compare with startsWith. There is lot of domain
> specific knowledge going into it to make it work correctly and in an
> efficient way.
> 
> This change introduces an enum based API which encapsulates the
> knowledge: it does the proper comparisons and caches the result, so
> that one does not need to do string comparisons again and again.
> 
> So far the implementation supports the platforms X11/XCB and Wayland.
> In addition there are convenient methods to check for a specific
> platform modelled after QX11Info::isPlatformX11.
> 
> Further platforms can be added by the maintainers of the respective
> platforms.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 65ed8d4da50fd41dd3060576284456c83c3629ab 
>   autotests/helper/CMakeLists.txt PRE-CREATION 
>   autotests/helper/wayland_platform.cpp PRE-CREATION 
>   autotests/kwindowsystem_platform_wayland_test.cpp PRE-CREATION 
>   autotests/kwindowsystemx11test.cpp cdf94e3f6573faea6dfd6038dded3ee1269e2552 
>   src/kwindowsystem.h 551b01f5831e2fbdf0d0ad30f3debf0955da189e 
>   src/kwindowsystem.cpp 4dfcce8bbfeed09f3f6ddec10a1eb5fd2d418313 
> 
> Diff: https://git.reviewboard.kde.org/r/128567/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160802/4e18949a/attachment.html>