Review Request 124697: Make Wayland a hard build time dependency

Thomas Lübking thomas.luebking at gmail.com
Tue Aug 11 16:45:45 UTC 2015


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


Generally fine with the change, but some present oddities were exposed by it.


abstract_client.cpp (line 517)
<https://git.reviewboard.kde.org/r/124697/#comment57931>

    Why are these functions in AbstractClient itfp.? Looks like shell-client-only feature.



abstract_egl_backend.h (line 94)
<https://git.reviewboard.kde.org/r/124697/#comment57932>

    same here?
    (also applies to all inner special code - why abstracts if they contain specific code already)



abstract_egl_backend.cpp (line 59)
<https://git.reviewboard.kde.org/r/124697/#comment57933>

    eg. cleanup should likely (? doesn't sound that performance critical) be virtual and EglWaylandBackend calls the wayland specific code and then AbstractEglBackend::cleanup()
    
    I'm sure there's a reason for this, but it looks a bit like bad design and hacked in :-\


- Thomas Lübking


On Aug. 11, 2015, 12:01 nachm., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124697/
> -----------------------------------------------------------
> 
> (Updated Aug. 11, 2015, 12:01 nachm.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Repository: kwin
> 
> 
> Description
> -------
> 
> As discussed on release-team ml [1] the following dependencies are
> mandatory:
> * KF5Wayland
> * Wayland::Cursor
> * Wayland::Egl
> * xkbcommon
> 
> [1] https://mail.kde.org/pipermail/release-team/2015-July/008725.html
> 
> Drop cmakedefine HAVE_XKB
> 
> No longer needed, we always depend on xkbcommon now.
> 
> Drop cmakedefine HAVE_WAYLAND_CURSOR
> 
> Now a required build-dep.
> 
> Drop cmakedefine HAVE_WAYLAND
> 
> Now a required build dependency.
> 
> Drop cmakedefine HAVE_WAYLAND_EGL
> 
> Now a required build dependency.
> 
> Make X11_XCB a build dependency of X11 windowed backend
> 
> Let's rather not build the plugin if we don't have the dependency
> then building it without OpenGL support. Simplifies the code a bit
> and makes the backend overall more useful and goes along with e.g.
> the Wayland one which has EGL also as a hard dependency for the
> plugin.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 056801feaa40b8bc24e56b5d132a3e02e66b6782 
>   abstract_backend.cpp 28564e3e03a68f78c58d351b0a2eb1f66a088879 
>   abstract_client.cpp 45976224394a8a467836bbe72596057446c95051 
>   abstract_egl_backend.h c266521d3a870b4bd110435fa6c5f8e5d4e72e33 
>   abstract_egl_backend.cpp 5ef3adaa7216c314b56c594167805da232f6a499 
>   backends/CMakeLists.txt 809ac64edbce6df001c22395a9a991d324008d92 
>   backends/wayland/CMakeLists.txt 7f643df2e13bc40172ea3ed5140b9aea5563b0b6 
>   backends/wayland/wayland_backend.cpp 57a805eafa470e1fb06cd318bf557c0cd5c2e38c 
>   backends/x11/CMakeLists.txt 9ce72fad1a7800f7e9b0c085a363f8eb256b7d24 
>   backends/x11/x11windowed_backend.cpp 95a5b70ecfa32358af934950f2c723cd9b8a03af 
>   composite.cpp 1db4790b50d99401c175e90852f5e4958f53fc8c 
>   config-kwin.h.cmake 6128c0ccae23453e2f5cf2918dee0d733aaec4d9 
>   effects.cpp 81c6ede571f6f995eee62e999633bcbc07599914 
>   events.cpp 3ce3f917c9a7854613244dd36cf0dc27e908d559 
>   geometry.cpp f63e85165b9e6c605e5ed740bb61f1ec02acecc7 
>   input.h c9ef924737b542b9d844fcb8e5b4989e02bf812d 
>   input.cpp 3e464486aa56a7edaa36371747b6c158a03c96c2 
>   layers.cpp d8328ccafd89706eb463f5dd120b82b7288c86bb 
>   scene.h 9e41cccc2da1d0a9c54adf9d3f412aeb1eece0b3 
>   scene.cpp 09b7ec4a3d5a0af3b976657f9b035a5bfecdc8f3 
>   scene_opengl.cpp 3e9b849ea7e6884386944188d9d6f4d38d74090f 
>   scene_qpainter.cpp e6829138f00f4529bf13b3733f34b0e694056e9a 
>   screens.cpp 226a2eca05d386b7f8b778b21019dc2d976c1197 
>   scripting/scripting_model.cpp 8b595f7c2ba3132bb574c48bae6d4823d9bc0366 
>   shadow.cpp 56bc97f91c204ec14d8eb8dc6ac5a4ac253e3436 
>   thumbnailitem.cpp 8b984558720ea974afb91aa55269ae514b44479f 
>   toplevel.h eb46eb4f7571fbde8034308903cd730af2b17854 
>   toplevel.cpp 4740c8f873439dd6ce08d99de7ee8028ec52ebfe 
>   workspace.cpp 9568b83b04112c28cd99ec60d472d5944a9d7f1b 
> 
> Diff: https://git.reviewboard.kde.org/r/124697/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150811/e926ae52/attachment-0001.html>


More information about the Plasma-devel mailing list