romangg added a comment.
In D29024#668884 <https://phabricator.kde.org/D29024#668884>, @ngraham wrote: > In D29024#668383 <https://phabricator.kde.org/D29024#668383>, @romangg wrote: > > > Note that I need to have this plugin system or something similar in for 5.19 or I will be forced to fork libkscreen permanently for KWinFT. I would like to avoid this and instead continue my work on libkscreen as a KDE project like I have worked on it in the last two years. > > > Nobody's forcing you to do anything. How would you call it? I obviously need output management to work and if my integration patch to libkscreen is blocked I need to keep maintaining my libkscreen fork. For that I inevitably have to build up much more project infrastructure than I currently have for what I intended to provide only for a short intermediate period. > Note that Plasma 5.19 branches in three days and merging huge things like this right before branching has bitten us before and I'd like to avoid it if possible for 5.19. So if we do go with this patch in something like its current form (I have no technical opinions on it whatsoever), I would like it merged in after branching for 5.19, not right before. This patch without header export but the very same plugin system was published for review on here already three weeks ago. I asked for feedback and later help in regards to the exported headers but received very little. The so called "high level discussion first" was David adding some arguments that are not worth repeating if one knows the internal libksceen structure, me pointing that out and him then ignoring it. He calls this high level discussion, I call it stalling tactic. I uploaded three days ago an updated version incorporating changes that were requested in the other review. Again no feedback until finally I ask today one last time for //specific// issues but instead David comes with the very same high level discussion he had more than enough time to reply to but missed out on doing in time. If he wants to tell other people how to do their code he needs to stay on top with what they are doing or delegate it. I have done large changes shortly before beta release in the past and fixed remaining issues in the beta phase without problems. That's what the beta phase is for. Since I'm online on most days there is no delay in fixing bugs if there are any. In D29024#668978 <https://phabricator.kde.org/D29024#668978>, @davidedmundson wrote: > > These are not specific issues but some general complains about the overall concept chosen here without providing an alternative solution. > > Well yes, it makes sense to do high level discussion first. You had more than enough time for that but ignored it in the last three weeks. > With the Plasma agreed "new approach" that we will be porting to we will be using QtWaylandClientExtension and removing the need for the connection thread - having that in the interface will hold up progress we want to do upstream. Show me where the documentation for this QtWaylandClientExtension is and where you documented that we "agreed" on using that in Plasma exclusively and we can talk about libkscreen using it in 5.20. Personally, I think with the current Qt situation instead of making us more dependent on Qt Company offerings we should become more independent but that's a different topic. In any case the plugin system here is independent of some "connection thread". You only hand over a normal QThread. >> here without providing an alternative solution. > > static bool isValid() in the plugin. That is made synchronous by use of roundtrip. The existing plugin gets a very tiny refactor. I know when you say "tiny refactor", "little change", it's in the end often a large one with major repercussions on the overall structure and internal logic. Why would you want a synchronous design? I explicitly opted for an asynchronous one such that we can directly select the active plugin instead of waiting for potentially multiple other ones to return without success. > (also you could just qputenv("KSCREEN_BACKEND") from your fork before launching the shell which is even less invasive) What would this help? I would still need to fork basically the whole libkscreen Wayland backend to create another plugin. And enforcing this environment variable is a hack for a problem I solved cleaner and in more generality with the plugin system presented here. How would you add a plugin for wlroots based compositors? Should they then set a different environment variable? It's telling that you call providing infrastructure for clean integration with independent projects "invasive". Plasma is an unwelcoming inward-looking project thanks to this mindset. REPOSITORY R110 KScreen Library REVISION DETAIL https://phabricator.kde.org/D29024 To: romangg, #plasma Cc: ngraham, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
