Re: Review Request 124697: Make Wayland a hard build time dependency
On Aug. 11, 2015, 6:45 p.m., Thomas Lübking wrote: abstract_client.cpp, line 519 https://git.reviewboard.kde.org/r/124697/diff/1/?file=393340#file393340line519 Why are these functions in AbstractClient itfp.? Looks like shell-client-only feature. No, we also need to expose the X11 Clients to the interface. The idea is that we provide all managed windows to Plasma's Wayland Taskmanager, so that Plasma doesn't need to interact with X11 when on Wayland. On Aug. 11, 2015, 6:45 p.m., Thomas Lübking wrote: abstract_egl_backend.h, line 95 https://git.reviewboard.kde.org/r/124697/diff/1/?file=393341#file393341line95 same here? (also applies to all inner special code - why abstracts if they contain specific code already) That one will become more obvious once we are able to manage Wayland clients on X11 ;-) - this was kind of the idea I had with moving it in the abstract interface: it's able to handle both X11 and Wayland applications, so that we can completely mix. (Ideally I would also like the GLX backend to be able to handle Wayland clients, but unfortunately that will be impossible :-( ). On Aug. 11, 2015, 6:45 p.m., Thomas Lübking wrote: abstract_egl_backend.cpp, line 64 https://git.reviewboard.kde.org/r/124697/diff/1/?file=393342#file393342line64 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 :-\ same as above - it should in theory also support Wayland applications on X11. Also splitting it out would mean to have it in each of the backend implementations and even more weird in the EglOnXBackend (as that's what is used in the nested Wayland compositor on X11). - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124697/#review83711 --- On Aug. 11, 2015, 2:01 p.m., 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, 2:01 p.m.) 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 9e412da1d0a9c54adf9d3f412aeb1eece0b3 scene.cpp 09b7ec4a3d5a0af3b976657f9b035a5bfecdc8f3 scene_opengl.cpp 3e9b849ea7e6884386944188d9d6f4d38d74090f scene_qpainter.cpp e6829138f00f4529bf13b3733f34b0e694056e9a screens.cpp 226a2eca05d386b7f8b778b21019dc2d976c1197 scripting/scripting_model.cpp 8b595f7c2ba3132bb574c48bae6d4823d9bc0366 shadow.cpp
Re: Review Request 124697: Make Wayland a hard build time dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124697/#review83729 --- Ship it! Ship It! - 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 9e412da1d0a9c54adf9d3f412aeb1eece0b3 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 124697: Make Wayland a hard build time dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124697/ --- (Updated Aug. 12, 2015, 9:39 a.m.) Status -- This change has been marked as submitted. Review request for kwin and Plasma. Changes --- Submitted with commit 5d946e37629b56be5b57931c1a3f0463dc8bd913 by Martin Gräßlin to branch master. 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 9e412da1d0a9c54adf9d3f412aeb1eece0b3 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Review Request 124697: Make Wayland a hard build time dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124697/ --- 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 9e412da1d0a9c54adf9d3f412aeb1eece0b3 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 124697: Make Wayland a hard build time dependency
--- 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 9e412da1d0a9c54adf9d3f412aeb1eece0b3 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 ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
