> On Feb. 4, 2014, 5:19 p.m., Aurélien Gâteau wrote: > > Any particular reason for not using an abstract class? The D macro makes > > the code a bit surprising to read. > > Martin Gräßlin wrote: > see discussion of https://git.reviewboard.kde.org/r/115225/ (comment > thread started by Aaron) - I wanted to keep the same pattern. > > Aurélien Gâteau wrote: > Sounds like premature optimization to me, but then, you are the > maintainer, so it's your call. Naming the macro DELEGATE like in the other > patch would make it easier to read, and a good idea if you want to follow the > same pattern. > > Martin Gräßlin wrote: > Yes it kind of sounds like premature optimization, but I like another > aspect of it: each backend needs to provide an implementation for all methods > and cannot rely on the dummy implementation. Thus the maintainer of the > backend has to think about the proper default value. Also a change like > introducing a new method will compile fail on other platforms. > > Renaming the macro is a good suggestion, though.
You would get the compilation failures with a pure abstract class as well. - Aurélien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115459/#review48944 ----------------------------------------------------------- On Feb. 5, 2014, 5:55 p.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115459/ > ----------------------------------------------------------- > > (Updated Feb. 5, 2014, 5:55 p.m.) > > > Review request for KDE Frameworks, kdewin and Alexander Richardson. > > > Repository: kwindowsystem > > > Description > ------- > > Introduce runtime platform support in KWindowSystem > > This is a change similar to the one in KWindowInfo, but with variation > to the pattern due to the static container. > > There is now a generic implementation of KWindowSystem which is > completely windowing system platform independent. This implementation > delegates all methods into a KWindowSystemPrivate class. > > Each windowing system platform implementation needs to provide a > subclass (e.g. KWindowSystemPrivateX11) and provide all the methods > which are delegated. Note that there are no virtual methods defined, > instead the d-pointer gets casted into the proper type. Thus if a > method is not provided it will end in a compile error. > > To make use of a platform implementation it needs to be included in > the ctor of KWindowSystemStaticContainer and the PlatformImplementation > enum needs to be extended by a value for the platform. This is used in > the D macro to cast and delegate correctly. > > There is a dummy implementation for all not supported windowing system > platforms. > > This change also includes some API changes: > * KWindowSystem::windows() returns a copy instead of const-ref > * All methods are provided, there is no longer X11 specific methods > * private methods and enums are removed > > NOTE: This change breaks the implementation for Windows and Mac OS! > They are currently excluded from build. > > > Diffs > ----- > > src/CMakeLists.txt 23133d581944a8373b9b753b300d97054b7d6f18 > src/kwindowinfo.cpp c706b29b306b65c992a178d490819b76e1aeca84 > src/kwindowsystem.h d288e1ab7c49f68482c94285c2aab695f08f3524 > src/kwindowsystem.cpp PRE-CREATION > src/kwindowsystem_p.h PRE-CREATION > src/kwindowsystem_p_x11.h PRE-CREATION > src/kwindowsystem_x11.cpp 8d841cbcec41dc2d4df381338803902badf3f35e > > Diff: https://git.reviewboard.kde.org/r/115459/diff/ > > > Testing > ------- > > Unit tests still succeed for X11, but they are not complete, though the most > important aspects are tested. > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Kde-windows mailing list Kde-windows@kde.org https://mail.kde.org/mailman/listinfo/kde-windows
