D1230: GBM remote access support for KWin

bshah added a comment. In D1230#234333 , @Kanedias wrote: > I'll take a look once I'm home I've fixed the issue on git master so don't worry. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graess

D1230: GBM remote access support for KWin

Kanedias added a comment. I'll take a look once I'm home REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: bcooksley, kossebau, jgrulich, romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, pla

D1230: GBM remote access support for KWin

bcooksley added a comment. Sorry, that was the wrong log - I intended to post https://build.kde.org/job/Plasma%20kwin%20kf5-qt5%20FreeBSDQt5.9/121/consoleText REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin C

D1230: GBM remote access support for KWin

Kanedias added a comment. @bcooksley this file was merged in D1231 REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: bcooksley, kossebau, jgrulich, romangg, ngraham, alexe

D1230: GBM remote access support for KWin

bcooksley added a comment. The introduction of this has broken the FreeBSD Build. Please see https://build.kde.org/job/Plasma%20kwin%20kf5-qt5%20FreeBSDQt5.9/120/consoleText REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson,

D1230: GBM remote access support for KWin

Kanedias added a comment. Done! Thanks for looking this through. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: kossebau, jgrulich, romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma

D1230: GBM remote access support for KWin

Kanedias closed this revision. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: kossebau, jgrulich, romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, ragreen, schernikov, iodelay,

D1230: GBM remote access support for KWin

romangg accepted this revision. romangg added a comment. This revision is now accepted and ready to land. Looks fine to me. Tested runtime together with your KWayland patch. Probably you know this, but please push as one commit only to master. INLINE COMMENTS > egl_gbm_backend.cpp:160 >

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 30189. Kanedias added a comment. Remove override fix REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=30092&id=30189 BRANCH fix-clang REVISION DETAIL https://phabricator.kde.org/D1230 AFFECTED FILES plugins

D1230: GBM remote access support for KWin

Kanedias added a comment. This makes sense. Thanks, will try with KDecoration master once I'm home REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: kossebau, jgrulich, romangg, ngraham, alexeymin, aacid, kw

D1230: GBM remote access support for KWin

kossebau added a comment. In D1230#230642 , @romangg wrote: > In D1230#230613 , @Kanedias wrote: > > > They don't override anything and compile fails for me if they are present. GCC 7.3.1. > > >

D1230: GBM remote access support for KWin

Kanedias added a comment. I don't have KDecoration checked out. The problem is much more simple. These functions don't override anything but there's override keyword present where it shouldn't be. The fix is still required for them, regardless of KDecoration status. Any other issues apa

D1230: GBM remote access support for KWin

romangg added a comment. In D1230#230613 , @Kanedias wrote: > They don't override anything and compile fails for me if they are present. GCC 7.3.1. I believe this is an unrelated regression you just ran into because of D11209

D1230: GBM remote access support for KWin

Kanedias marked an inline comment as done. Kanedias added a comment. They don't override anything and compile fails for me if they are present. GCC 7.3.1. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: j

D1230: GBM remote access support for KWin

romangg added a comment. Why did you remove the override specifier? It compiles for me also with them. And they are certainly in no connection to this patch. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc

D1230: GBM remote access support for KWin

Kanedias requested review of this revision. Kanedias marked an inline comment as done. Kanedias added inline comments. INLINE COMMENTS > romangg wrote in drm_output.h:135 > This gives me an compile error: invalid static_cast from type 'QObject*' to > type 'KWayland::Server::OutputInterface*' >

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 30092. Kanedias added a comment. - Fix clang compilation - Fix QPointer REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=29434&id=30092 BRANCH fix-clang REVISION DETAIL https://phabricator.kde.org/D1230 AFF

D1230: GBM remote access support for KWin

romangg added inline comments. INLINE COMMENTS > drm_output.h:135 > +const KWayland::Server::OutputInterface* getWaylandInterface() const { > +return m_waylandOutput.data(); > +} This gives me an compile error: invalid static_cast from type 'QObject*' to type 'KWayland::Server::

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 29434. Kanedias added a comment. Typo REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=29433&id=29434 BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 AFFECTED FILES plugins/platforms/drm/CM

D1230: GBM remote access support for KWin

Kanedias added inline comments. INLINE COMMENTS > romangg wrote in egl_gbm_backend.cpp:160 > Should be the default not directly activated remote funcitonality? And if one > wants to deactivate remote set `KWIN_NO_REMOTE` or something. There's no authentication protocol, so any malicious applica

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 29433. Kanedias marked 6 inline comments as done. Kanedias added a comment. Review fixes REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=28973&id=29433 BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.o

D1230: GBM remote access support for KWin

romangg added inline comments. INLINE COMMENTS > romangg wrote in drm_output.h:139 > Is it only a friend class to access `m_waylandOutput.data()`? In this case > better create a getter for it in DrmOutput. > > Or better do the `passBuffer` call in `DrmBackend::present` and give instead > of th

D1230: GBM remote access support for KWin

romangg added inline comments. INLINE COMMENTS > main_wayland.cpp:780 > pluginName = KWin::automaticBackendSelection(); > +std::cerr << "Selected backend " << pluginName.toStdString() << > std::endl; > } Put this after the if clause (such that it shows the selected backend

D1230: GBM remote access support for KWin

Kanedias added a comment. Please re-review this and D1231 as I'm starting to work on KRfb support for this and it will be the 3rd patch depending on this. Or suggest reviewers with more spare time so they can look this through. REPOSITORY R108 KWin BR

D1230: GBM remote access support for KWin

Kanedias marked 2 inline comments as done. Kanedias added a comment. @romangg , I had to recreate it after rebase, cause I don't have force-push. REPOSITORY R108 KWin BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kw

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 28973. Kanedias added a comment. Review comments REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=25979&id=28973 BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 AFFECTED FILES main_wayland.

D1230: GBM remote access support for KWin

romangg added a comment. Pls remove the whitespace and rebase your branch on current master (it merges without conflict). INLINE COMMENTS > drm_backend.cpp:793 > } > + rm whitespace REPOSITORY R108 KWin BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias

D1230: GBM remote access support for KWin

Kanedias added a comment. @graesslin , @davidedmundson , please approve this once again, this was updated numerous times after initial review REPOSITORY R108 KWin BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin

D1230: GBM remote access support for KWin

jgrulich added a dependent revision: D10965: Add screen cast portal. REPOSITORY R108 KWin BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: jgrulich, romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson,

D1230: GBM remote access support for KWin

Kanedias added a task: T7785: PipeWire support in remote access to KWin. REPOSITORY R108 KWin BRANCH gbm-vnc REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: jgrulich, romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmund

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 25979. Kanedias added a comment. - Merge branch 'master' into gbm-vnc - Fix compilation on Clang REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=21211&id=25979 BRANCH gbm-vnc REVISION DETAIL https://phabric

D1230: GBM remote access support for KWin

Kanedias added a comment. Hi @jgrulich! There are actually two sides of this dime. The one that I wanted to take is implementing pipewire server in kded. It will be responsible for converting GBM fds to PW video stream fds and will announce this interface via DBus like Mutter does

D1230: GBM remote access support for KWin

romangg added a comment. In https://phabricator.kde.org/D1230#192516, @Kanedias wrote: > After it is working we'll have an implicit confirmation that these patches indeed have not regressed and can be merged. I don't fully understand what this means, but in general I think it soun

D1230: GBM remote access support for KWin

jgrulich added a comment. In https://phabricator.kde.org/D1230#192516, @Kanedias wrote: > @romangg , let's go with following approach: I'll upstream all changes I have left hanging (e.g. https://phabricator.kde.org/D9708) by the end of this week and will try to come up with some PoC work

D1230: GBM remote access support for KWin

Kanedias added a comment. @romangg , let's go with following approach: I'll upstream all changes I have left hanging (e.g. https://phabricator.kde.org/D9708) by the end of this week and will try to come up with some PoC work with PipeWire solution this/next weekend. After it is working we'

D1230: GBM remote access support for KWin

romangg added a comment. I talked to @graesslin some time ago and he said, that @davidedmundson and I should do the review. My plan is to get this review going after 5.12 has landed. For Plasma 5.13 then. I would like to see at least some progress regarding PipeWire before that tho

D1230: GBM remote access support for KWin

ngraham added a comment. @graesslin, would you mind reviewing this so we can push forward with the feature? Thanks! REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: romangg, ngraham, alexeymin, aacid, kwin

D1230: GBM remote access support for KWin

Kanedias added a comment. @ngraham I'm still waiting for a review to land this REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel

D1230: GBM remote access support for KWin

ngraham added a comment. What's the status of this? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, romangg, #kwin Cc: romangg, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, iodelay, bwowk, leezu, Zr

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 21211. Kanedias marked 2 inline comments as done. Kanedias added a comment. Updated against latest master REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=18859&id=21211 REVISION DETAIL https://phabricator.kde.or

D1230: GBM remote access support for KWin

Kanedias added a comment. Fixed review comments REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, subdiff, #kwin Cc: subdiff, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, bwowk, leezu, ZrenBot, progwo

D1230: GBM remote access support for KWin

Kanedias added a comment. Ok @subdiff, I've got your idea. I'll align my patches with newest sources ASAP, check that they work, then we'll talk about what to do with KDE Daemon and Screen Recorder. I just don't want to spread my efforts too much, maintaining more than one patchset in ever

D1230: GBM remote access support for KWin

subdiff added a comment. In https://phabricator.kde.org/D1230#159026, @Kanedias wrote: > > On the other side PipeWire is pretty new and its documentation is nearly non-existent. I don't know if with PipeWire it really could work in the way I described above. Your current solution works r

D1230: GBM remote access support for KWin

Kanedias added a comment. > On the other side PipeWire is pretty new and its documentation is nearly non-existent. I don't know if with PipeWire it really could work in the way I described above. Your current solution works right now. It might still be an idea if you are interested to check

D1230: GBM remote access support for KWin

subdiff added a task: T5653: [kwin] Screen recording in Wayland session. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, subdiff, #kwin Cc: subdiff, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, bwowk, l

D1230: GBM remote access support for KWin

subdiff added a comment. In https://phabricator.kde.org/D1230#158987, @Kanedias wrote: > > The thing is I would like to see your solution also be applicable to other apps that need access to the buffer contents, like screen recording tools (SimpleScreenRecorder for example) and so on. If

D1230: GBM remote access support for KWin

Kanedias added a comment. > The thing is I would like to see your solution also be applicable to other apps that need access to the buffer contents, like screen recording tools (SimpleScreenRecorder for example) and so on. If you say KRfb does the heavy-lifting, I assume such a screen record

D1230: GBM remote access support for KWin

subdiff added a comment. In https://phabricator.kde.org/D1230#158816, @Kanedias wrote: > Well, it has `active` property in KWayland that is true only if at least one client is connected (if I remember my code from 6 months back correctly), so I can expose it and use here. Will this suffi

D1230: GBM remote access support for KWin

subdiff added a comment. In https://phabricator.kde.org/D1230#158801, @davidedmundson wrote: > > The RemoteAccessManager is active all the time. It would be nice if it would be only active if there are clients who ask for buffers. > > How is a client going to ask for a buffer if there

D1230: GBM remote access support for KWin

Kanedias added a comment. In https://phabricator.kde.org/D1230#158795, @subdiff wrote: > Hi Oleg Hi. Haven't been here in a long time. > - Please merge current master. Will do this and fix review comments on weekend (or earlier if I can). > - The RemoteAccessManager

D1230: GBM remote access support for KWin

davidedmundson added a comment. > The RemoteAccessManager is active all the time. It would be nice if it would be only active if there are clients who ask for buffers. How is a client going to ask for a buffer if there is no RemoteAccessManagerInterface? (which this owns) > Then in

D1230: GBM remote access support for KWin

subdiff added reviewers: subdiff, KWin. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson, subdiff, #kwin Cc: subdiff, ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, bwowk, leezu, ZrenBot, progwolff, lesliez

D1230: GBM remote access support for KWin

subdiff added a comment. Hi Oleg, sorry that I haven't given you feedback to your patch series until now. First some smaller points: - Please merge current master. - What about the software cursor dynamically switching? I don't see where it does that. - The RemoteAccessManager i

D1230: GBM remote access support for KWin

Kanedias added a comment. In https://phabricator.kde.org/D1230#158542, @ngraham wrote: > With https://phabricator.kde.org/D6186 merged, is this now ready to go in? Yes, just waits for @graesslin's final review :) REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.or

D1230: GBM remote access support for KWin

ngraham added a comment. With https://phabricator.kde.org/D6186 merged, is this now ready to go in? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson Cc: ngraham, alexeymin, aacid, kwin, #kwin, davidedmundson, plasma-devel, bw

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 18859. Kanedias added a comment. Fixes for review comments, rebased to latest version REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D1230?vs=15146&id=18859 REVISION DETAIL https://phabricator.kde.org/D1230 AFFECTED FI

D1230: GBM remote access support for KWin

aacid added a comment. If this indeed needs fixing, can someone please mark it as "request changes" so it doesn't show up in the list of "patches that have been accepted but have not been commited"? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, gr

D1230: GBM remote access support for KWin

Kanedias added a comment. Added auxiliary patch for mouse cursor in https://phabricator.kde.org/D6186 REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson Cc: kwin, #kwin, davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff,

D1230: GBM remote access support for KWin

Kanedias added a dependent revision: D6186: Implement software cursor in OpenGL backend . REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson Cc: kwin, #kwin, davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff,

D1230: GBM remote access support for KWin

Kanedias added a comment. Wait, cursor uses DrmDumbBuffer, which has no GBM BO in it! REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson Cc: kwin, #kwin, davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, al

D1230: GBM remote access support for KWin

Kanedias added a comment. In https://phabricator.kde.org/D1230#114175, @davidedmundson wrote: > There is one issue we need to work out how to fix - with this system if you're running VNC you won't see the client's mouse. Nice catch, I didn't notice that until you pointed that out!

D1230: GBM remote access support for KWin

davidedmundson added a comment. There is one issue we need to work out how to fix - with this system if you're running VNC you won't see the client's mouse. For VNC it's not a huge problem as you can see your own mouse, for screen recording it makes it useless. As I see it there are

D1230: GBM remote access support for KWin

davidedmundson accepted this revision. davidedmundson added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > egl_gbm_backend.cpp:158 > +{ > +bool supportRemotePresent = false; > +const QByteArray remoteOption = qgetenv("KWIN_REMOTE"); you can do bool fo

D1230: GBM remote access support for KWin

Kanedias added a dependency: D1231: Add Remote Access interface to KWayland. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D1230 To: Kanedias, graesslin, davidedmundson Cc: kwin, #kwin, davidedmundson, plasma-devel, ZrenBot, spstarr, progwolff, lesliezhai, ali-mohamed, ha

D1230: GBM remote access support for KWin

Kanedias updated this revision to Diff 15146. Kanedias edited projects, added Plasma on Wayland; removed Plasma. Kanedias added a comment. Restricted Application added a project: KWin. Restricted Application added subscribers: KWin, kwin. Rebased against latest KWin, corrected implementation R

D1230: GBM remote access support for KWin

Kanedias added inline comments. INLINE COMMENTS > davidedmundson wrote in drm_buffer.cpp:131 > If it's deferred it means someone else is doing the gbm_surface_release. > > But we still need to set m_bo to nullptr. Otherwise it's potentially left > dangling here after the RemoteAccessManager has

D1230: GBM remote access support for KWin

davidedmundson requested changes to this revision. davidedmundson added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > drm_buffer.cpp:131 > #if HAVE_GBM > +if (m_deferred) { > +return; If it's deferred it means someone else is doing the gbm_surface

[Differential] [Request, 231 lines] D1230: GBM remote access support for KWin

Kanedias created this revision. Kanedias added a reviewer: graesslin. Kanedias set the repository for this revision to rKWIN KWin. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. REVISION SUMMARY Implements a KWayland protocol to pass GBM fd from KWin to KRfb and addic