D22418: WIP [wayland] make automatic backend selection truly automatic

[Alexey Min]

alexeymin added a comment.


  > not convinced that removing the ifdef is the solution
  
  It is, because now availability and presence of command-line options is decided at KWin **compile** time, and it should be decided at **run** time based on the actual list of available plugins. Just removing `#ifdefs` already solves the problem that I cannot pass `--hwcomposer` argument to `kwin_wayland` if KWin was built without libhybris support and hwcomposer plugin is installed later and it can be loaded perfectly (and works fine). That's why we'll have to remove `#ifdef`s anyway.
  
  I wanted to go with the minimalistic and non-invasive approach, with as little bloodshed as possible. But if you wish to see some rewrites, how about this:
  
  - Each plugin has a method to help in autodetection process (`canLoad()`) that quickly checks if the environment conditions are correct and allow loading of this plugin. Previously there was a function `automaticBackendSelection()`, that did that by just checking `qEnvironmentVariableIsSet` and so; now each plugin can do more complicated tests than just env vars checking.
  - Each plugin has a concept of "priority" for automatic loading, because we need some order among them. This was previously also in `automaticBackendSelection()`, the order of `if` statements. The priority is saved in JSON metadata, the same way as the `"input"` property is now stored and tested in `Application::initPlatform() `: https://cgit.kde.org/kwin.git/tree/main.cpp?h=Plasma/5.17#n442
  
  It is not trivial to move CLI args providing and parsing into plugins, because some common arguments like `--width` and `--height` are only used for windowed backends, for example ( https://cgit.kde.org/kwin.git/tree/main_wayland.cpp?h=Plasma/5.17#n420 ) and some plugins have specific CLI options, like `--x11-display` for x11-windowed backend or `--fb-device` for framebuffer backend, `--wayland-display` for nested wayland. But maybe it's still possible by extending base `Platform` class even more...

REPOSITORY
  R108 KWin

REVISION DETAIL
  https://phabricator.kde.org/D22418

To: alexeymin, romangg, #kwin
Cc: PureTryOut, graesslin, kwin, plasma-devel, LeGast00n, The-Feren-OS-Dev, sbergeron, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191116/2ecf56de/attachment-0001.html>