Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KDE5 decoration plugin #1784

Closed
marmarek opened this issue Feb 26, 2016 · 32 comments
Closed

KDE5 decoration plugin #1784

marmarek opened this issue Feb 26, 2016 · 32 comments
Labels
C: desktop-linux help wanted This issue will probably not get done in a timely fashion without help from community contributors. P: minor Priority: minor. The lowest priority, below "default." release notes This issue should be mentioned in the release notes.

Comments

@marmarek
Copy link
Member

KDE 5 have major changes in window decoration plugin API, making upgrade hard. Most likely Qubes decoration plugin will need to be rewritten from scratch.

This issue blocks dom0 upgrade to something newer than Fedora 20.
We're planning to add Gnome support in dom0, which somehow makes this issue lower priority, but still an issue.

Related discussion: https://groups.google.com/d/msgid/qubes-devel/20150708225129.GN900%40mail-itl

@marmarek marmarek added C: desktop-linux P: major Priority: major. Between "default" and "critical" in severity. task labels Feb 26, 2016
@marmarek marmarek added this to the Release 4.0 milestone Feb 26, 2016
@marmarek marmarek added help wanted This issue will probably not get done in a timely fashion without help from community contributors. release notes This issue should be mentioned in the release notes. labels Feb 26, 2016
@jgriffiths
Copy link

As discussed on the list, here is a diff to the current kwin master (git://anongit.kde.org/kwin) that displays the VM title as a suffix on every window, regardless of the current decoration style.

@jgriffiths
Copy link

And here is a program to demonstrate the palette setting option. compile with gcc propset.c -lX11 -o propset and run on a KF5 box with breeze windows decorations (the default) as e.g.:

./propset _KDE_NET_WM_COLOR_SCHEME /usr/share/color-schemes/Zion.colors
./propset _KDE_NET_WM_COLOR_SCHEME /usr/share/color-schemes/Norway.colors

If we were to install schemes with label suffixes then xside.c could set this property to point to the files and no further KWin changes would be required!

@jgriffiths
Copy link

The following is a minimal color profile that will be applied by KWin:

[Colors:Window]
BackgroundNormal=255,0,0

[WM]
activeBackground=255,0,0
inactiveBackground=128,0,0

According to the KDE color scheme docs at https://docs.kde.org/trunk5/en/kde-workspace/kcontrol/colors/index.html (look for 'Window Manager Colors'), its up to the theme whether they use the window manager colors from the [WM] section or the Window colors.

Experimentation shows that Breeze uses WM/activeBackground/inactiveBackground while Oxygen uses [Colors:Window]/BackgroundNormal (Oxygen doesn't seem to dim the window if its not active). Its not clear what Plastik is using.

You can try the above with the propset test program and see that the colors are correct under Oxygen and Breeze. Given this I'm going to hack up a patch to xside.c to set this value.

@jgriffiths
Copy link

And here is a commit making gui-daemon set the palette file for KWin.

@jgriffiths
Copy link

Finally, here is a quick script to generate sample palette files for all VMs. Placed in /usr/share/qubes/color-schemes with the patch above applied they should provide window colors under both oxygen and breeze.

@marmarek
Copy link
Member Author

And here is a program to demonstrate the palette setting option.

There is a standard utility xprop.

@jgriffiths
Copy link

Fixed gui-daemon range checking, updated commit is here.

I didn't realise xprop let you set properties too, doh.

@marmarek
Copy link
Member Author

marmarek commented Mar 6, 2016

Can you post some screenshots how it looks? With custom color palette and VM name as remote machine name, different themes etc.

@marmarek
Copy link
Member Author

marmarek commented Mar 7, 2016

I've tried this on Fedora 23 Live.
The mechanism for setting border colors looks to be just fine. Technically, I think color scheme should be named like qubes-VMNAME and generated for each VM (either at VM startup, or creation). To not repeat the mistake of hardcoding color values in one more place. QubesVmLabel objects know appropriate RGB value.

But I've failed to get VM name in the window title. WM_CLIENT_MACHINE seems to be ignored, at least when set on already existing window. Does it work for you?

@jgriffiths
Copy link

The VM name will only show in the window with the kwin patch linked above applied, then _QUBES_VMNAME set via xprop. WM_CLIENT_MACHINE won't work and if it did we couldn't use it anyway I think. We have to keep all existing methods/markers of truly remote windows intact.

Setting the VM name using that patch works with all windows decorations though, and its a very small patch to carry compared to the current plugin.

I will attempt to get some screenshots up shortly.

@jgriffiths
Copy link

See http://postimg.org/image/vu4vxue49/ and http://postimg.org/image/n1sinwid5/ - these show a patched kwin5 + breeze with VM name and colors set manually using xprop.

Note that 1) the colors aren't correct (either by rgb value or label-to-color mapping) since this is just a mock up and 2) the kwin patch doesn't format the VM name in any way so I manually set it to ' [vmname]'.

However you can see that the active/inactive colors change when windows are selected which is nice I think.

@marmarek
Copy link
Member Author

marmarek commented Mar 7, 2016

Setting the VM name using that patch works with all windows decorations though, and its a very small patch to carry compared to the current plugin.

Yes, it is small, but still it requires carrying own kwin package - tracking upstream version etc. Compared to the plugin which worked almost untouched across multiple KDE and Fedora versions. But if there is no other way...

Or maybe there is: if no formatting is applied to the VM name anyway, maybe gui-daemon can simply prefix window title with "[VMNAME]"? That should be some configuration/command line option to easily switch this feature on different window managers, or when someone will implement better support for KDE.

However you can see that the active/inactive colors change when windows are selected which is nice I think.

Yes :) Apparently only title bar is changed, not the rest of the window borders, which looks a little weird, but not a problem.

@jgriffiths
Copy link

maybe gui-daemon can simply prefix window title with "[VMNAME]"?

If this covers all the ways in which apps can set their title then it seems like a good idea as an initial implementation.

Apparently only title bar is changed, not the rest of the window borders, which looks a little weird,

Yeah, that looks like a kwin bug TBH and I'm pretty sure upstream will accept a patch for it. Until its fixed upstream, picking a smaller difference in active/inactive colors than my test color files would look better.

@marmarek
Copy link
Member Author

marmarek commented Mar 7, 2016

If this covers all the ways in which apps can set their title then it seems like a good idea as an initial implementation.

Yes, this is the only way how VM can set a window title visible in title bar (where currently it is prefixed with VM name by window manager). There are some cases where window don't have title bar at all (for example tray icons), but there is no place for VM name either.

@jgriffiths
Copy link

Great, would you like me to code this up for the gui-daemon with a command line option to enable it?

@marmarek
Copy link
Member Author

marmarek commented Mar 7, 2016

Yes :)

Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

@jgriffiths
Copy link

I've added untested/uncompiled support for this to this branch.

Compiling and testing is proving difficult due to qubes-builder breakage which I'll raise on qubes-devel. My worry currently is what happens if a program fetches its own window title and then appends something to it, will it end up with the VM name prefix twice?

@jgriffiths
Copy link

I've raised a KDE Bug for the window frame color issue, lets see what upstream have to say about it.

@jgriffiths
Copy link

OK, I've built and trivially tested the branch and it seems to work as far as the title goes.

Regarding your comments on naming the color files after the VM I don't think its a good idea, I think they should be named after the levels. I suspect if we want to support multiple VM decorations that we will end up with a set per decoration, e.g. red-breeze, red-oxygen etc (because each decoration uses different and incompatible color elements from the file).

@marmarek
Copy link
Member Author

marmarek commented Mar 8, 2016

I suspect if we want to support multiple VM decorations that we will end up with a set per decoration, e.g. red-breeze, red-oxygen etc (because each decoration uses different and incompatible color elements from the file).

Isn't it possible to have them in the same file? I think your generator does exactly that, no? Anyway, if not, we can have qubes-work-breeze, qubes-shopping-oxygen etc. That would also require adding another option to gui-daemon for choosing that suffix (regardless if that would be red-breeze or qubes-work-bereeze).

Additionally (not sure about that), it may be possible then to switch border colors on the fly, without restarting VM. Depends on KDE - if/when color profile is reloaded. Pretty minor feature, but if we'd got it for free, why not?

Anyway, thanks for the patches! I'll try get back to fc22 based dom0 somehow later this week (need to find which disk it was...). After final R3.1 release, which is building just now :)
If you want to help further with upgrading dom0, I've just added build instructions to #1807

@jgriffiths
Copy link

Isn't it possible to have them in the same file?

If you read the response to the KDE bug, the upstream view seems to be that its up to each window decoration to use whatever colors they feel like in actually drawing the window. For breeze we can make the colors look good by setting the border (Colors:Window:BackgroundNormal) to the inactive color (i.e. a slightly darker shade) so when the window activates the title brightens and it darkens when inactivated (this looks much better than the screenshots above where the border color stays bright when deactivated).

But since oxygen only uses Colors:Window:BackgroundNormal for both the border and the title, we have to set it to the actual label color when using the oxygen style. Under oxygen we get no choice to dim the color for inactive windows. We can't therefore use a single color file and have both styles looking their best. Other default styles and default theme styles can presumably be tweaked using different color override still.

we can have qubes-work-breeze, qubes-shopping-oxygen etc.

Yes this is doable and I suspect the only approach if you want per vm color files instead of per label. These could be handled in the same way as the VM icon file is from the looks of it, generated from qvm-create with an external helper that imports the color definitions and spits out color files for the styles we support into the VM directory.

If you want to help further with upgrading dom0, I've just added build instructions to #1807

Yes I am looking forward to playing with this, thanks.

I will also be looking at other areas as I get used to the code, I imagine there are a few areas I may be able help out.

@m-v-b
Copy link

m-v-b commented Apr 11, 2016

As an alternative/interim solution, I customized the Breeze theme, as can be seen at marmarek/qubes-desktop-linux-kde@c57ef7d and marmarek/qubes-desktop-linux-kde#1

@marmarek marmarek added this to the Release 3.2 milestone Apr 28, 2016
marmarek added a commit to marmarek/old-qubes-core-admin that referenced this issue May 18, 2016
On KDE5 native decoration plugin is used and requires special properties
set (instead of `_QUBES_VMNAME` etc).
Special care needs to be taken when detecting environment, because
environment variables aren't good enough - this script may be running
with cleared environment (through sudo, or from systemd). So check
properties of X11 root window.

QubesOS/qubes-issues#1784
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 18, 2016
Using upstream theme with custom color palete requires even less
maintenance, so is preferred solution. Additionally this way both title
bar and borders are colored.

Initial script implemented by @jgriffiths (thanks!), later modified to
dynamically extract defined labels and color values.

QubesOS/qubes-issues#1784
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 18, 2016
Remove "Computer" tab.
kickoff.tar.gz is unmodified upstream version.

QubesOS/qubes-issues#1784
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 18, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 18, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 18, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 19, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue May 19, 2016
@marmarek
Copy link
Member Author

Additional problem: tray icons do not have colorful border, and generally are quite ugly. I'm under impression that icon there isn't actual VM window embedded into panel (as it should be according to System Tray Protocol Specification), but some icon manually extracted by KDE and drawn there (after some manipulation?). Also tooltip of that icon is a window title ([sys-net] NetworkManager Applet), not the actual tooltip of nm-applet icon (current network name, signal strength).

Created #2042 for this.

andrewdavidwong added a commit that referenced this issue Jun 7, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue Jun 7, 2016
I can't manage to remove devicenotifier using config init/update API
(https://userbase.kde.org/KDE_System_Administration/PlasmaTwoDesktopScripting),
so provide default configuration file in /etc/skel.

QubesOS/qubes-issues#1784
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue Jun 7, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue Jun 7, 2016
andrewdavidwong added a commit that referenced this issue Jun 12, 2016
marmarek added a commit to marmarek/qubes-desktop-linux-kde that referenced this issue Jun 25, 2016
There should be semicolon at the end. Also remove NoDisplay=true to make
debugging easier.

QubesOS/qubes-issues#1784
Reported by: Niels Kobschaetzki <niels@kobschaetzki.net>
marmarek added a commit to QubesOS/qubes-desktop-linux-kde that referenced this issue Jun 25, 2016
There should be semicolon at the end. Also remove NoDisplay=true to make
debugging easier.

QubesOS/qubes-issues#1784
Reported by: Niels Kobschaetzki <niels@kobschaetzki.net>
@rootkovska rootkovska added P: minor Priority: minor. The lowest priority, below "default." and removed P: blocker Priority: blocker. Prevents release or would have prevented release if known prior to release. labels Jun 30, 2016
@rootkovska rootkovska modified the milestones: Far in the future, Release 3.2 Jun 30, 2016
@andrewdavidwong
Copy link
Member

In light of the move from KDE to Xfce4 as the default in R3.2, is anyone still working on still working on this? In particular, @jgriffiths? Asking for tracking purposes.

@jgriffiths
Copy link

I'm not working on this right now, I've recently started a new job which is taking most of my time.

Also I think its time to admit that KDE has lost the plot with constant refactoring and chasing non desktop usage. It is ugly, slow and unstable. I would stop using Qubes if it forced gnome in dom0 but I can live with xfce. Especially if I can upgrade dom0 past f22 sooner as a result.

@andrewdavidwong
Copy link
Member

Understood. Thanks.

andrewdavidwong added a commit that referenced this issue Jul 31, 2016
marmarek added a commit to marmarek/old-qubes-core-admin that referenced this issue Sep 8, 2016
Commit from core2:

    commit 94d52a1

    core: adjust guid parameters when running on KDE5

    On KDE5 native decoration plugin is used and requires special properties
    set (instead of `_QUBES_VMNAME` etc).
    Special care needs to be taken when detecting environment, because
    environment variables aren't good enough - this script may be running
    with cleared environment (through sudo, or from systemd). So check
    properties of X11 root window.

    QubesOS/qubes-issues#1784
@marmarek
Copy link
Member Author

marmarek commented Mar 7, 2020

This has been implemented a long time ago.

@marmarek marmarek closed this as completed Mar 7, 2020
@andrewdavidwong andrewdavidwong removed this from the Release TBD milestone Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: desktop-linux help wanted This issue will probably not get done in a timely fashion without help from community contributors. P: minor Priority: minor. The lowest priority, below "default." release notes This issue should be mentioned in the release notes.
Projects
None yet
Development

No branches or pull requests

5 participants