> On Sept. 1, 2015, 2:35 p.m., Thomas Lübking wrote: > > src/client/blur.h, line 51 > > <https://git.reviewboard.kde.org/r/125015/diff/1/?file=399883#file399883line51> > > > > a) Why are those classes QObject? Only for parent/child maintainance? > > > > b) Does it make sense to stash the WaylandPointer or would client code > > perhaps benefit from access to an inherited WaylandPointer? > > > > c) What's the point of the extra setup function (which is supposed to > > be called only once? but allow for invalid objects and lead to "please use > > a creator function" comments) - stack members?? > > Martin Gräßlin wrote: > a) Not only. On client side all objects are QObject to ensure that we can > emit signals in future (in case the wayland interface emits events it needs a > signal) > b) I don't get the question. > c) yes questionable. The idea is to have an interface which allows to > integrate with low level Wayland calls. An example could be wl_compositor > which can be retrieved through the Qt QPA. If one would want to create a > Surface with that one, one would need the low level calls. It might not make > sense on all wrappers, but for API consistency I decided to add it to all > interfaces. > > Thomas Lübking wrote: > b) class Blur : public WaylandPointer<.,.>, public QObject - ie. no > wrapper+operator but OOP-style (adding some convenience functions to the > actual data) > > c) Not sure whether I understand that; where's the problem with passing > the pre-manipulated data as parameter to the constructor? > If one could "setup" to switch internal "manager"s at runtime, ie. a > setter/getter, i'd see the point, but right now, i'd rather go for passing > the (in this case manager) as parameter and in doubt have an invalid > constructor and a copy opertor (for QHash/QMap etc)
concerning b) I didn't want to expose the WaylandPointer as public API. concerning c) there is an additional feature where that is used: reconnecting after a Wayland server crash. But in both cases: it's difficult to change now without breaking the ABI. There are some things I would like to change. E.g. for BlurManger I would like to have something like a parent Global and for Blur something like a parent Resource. I'm not particular happy with the API. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125015/#review84701 ----------------------------------------------------------- On Sept. 1, 2015, 4:18 p.m., Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125015/ > ----------------------------------------------------------- > > (Updated Sept. 1, 2015, 4:18 p.m.) > > > Review request for kwin and Plasma. > > > Repository: kwayland > > > Description > ------- > > a protocol to activate the blur behind windows and to optionally set a sub > region of the window where to apply the blur to, in case the window is shaped > > > Diffs > ----- > > autotests/client/CMakeLists.txt c2c1df2 > autotests/client/test_wayland_blur.cpp PRE-CREATION > autotests/client/test_wayland_registry.cpp 281618d > src/client/CMakeLists.txt 7d0d38a > src/client/blur.h PRE-CREATION > src/client/blur.cpp PRE-CREATION > src/client/protocols/blur.xml PRE-CREATION > src/client/registry.h 3e9d1f4 > src/client/registry.cpp 0c1c213 > src/client/surface.h fe744bc > src/client/surface.cpp 98a3cec > src/server/CMakeLists.txt 1cf09d3 > src/server/blur_interface.h PRE-CREATION > src/server/blur_interface.cpp PRE-CREATION > src/server/display.h 4c0e0c7 > src/server/display.cpp 884d7ea > src/server/surface_interface.h 4935ff6 > src/server/surface_interface.cpp be885bd > src/server/surface_interface_p.h 062c7e7 > > Diff: https://git.reviewboard.kde.org/r/125015/diff/ > > > Testing > ------- > > > Thanks, > > Marco Martin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
