QtCurve/Qt5 : further KF5 adaptation
ClosedPublic

Authored by rjvbb on Mar 25 2017, 7:51 PM.

Details

Reviewers
yuyichao
Summary

This patch introduces several improvements to QtCurve's integration with KF5:

  • Inherit KStyle instead of QCommonStyle when built with KDE support
  • recognise at least KF5 systemsettings application and above all KWin5
  • use the new method of being informed about changes to the compositing mode
  • detect when KWin5 adapted its colour palette to an application's dedicated palette.

The last point addresses the issue that KWin's context (window) menus can become unreadable when they KWin5 adapts their palette to match the palette of an application set to use a palette that is different from the user's desktop colour palette (or when KWin5 itself modifies the palette of a specific application via a window or application rule).

In this case, the patch attempts to use a temporary palette for the concerned menus and menu items. Depending on how QtCurve is configured this may not give exactly the same colours as used in the application's own menus, but they ought to give readable menus with appropriate colours.

Test Plan

Tested on Linux/X11 with Qt 5.8.0, KF5 Frameworks 5.32.0, KWin5 5.9.3 as well as the latest KWin4 version.

Care has been taken not to introduce regressions when used on a Plasma4 desktop.

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
rjvbb created this revision.Mar 25 2017, 7:51 PM
rjvbb updated this revision to Diff 12817.Mar 26 2017, 8:11 AM

this version attempts to take a few settings into account for rendering the items in the special-case KWin context menus.

rjvbb added a comment.Mar 26 2017, 8:52 AM

Menu appearance with my regular light desktop palette and when only the application (oxygen-demo5) was switched to the Breeze Dark palette.

rjvbb added a comment.Mar 26 2017, 9:00 AM

KWin5 context menu when (only) the application uses the Breeze Dark palette, with and without the current patch.

rjvbb added a comment.Mar 26 2017, 9:07 AM

KWin5 context menus with an item selected, with my regular light desktop theme and the Breeze Dark palette (with and without the patch).

QtCurve is configured to render the menu backdrop with a 100% HSV-based shade lightening (the main reason why KWin's context menus become unreadable when the client application uses Breeze Dark but not KWin itself).

yuyichao accepted this revision.Mar 30 2017, 2:54 PM

Is this the new review platform?

LGTM. Hopefully I'm doing the right action here.... (phabricator always seem to have way more buttons than I need..)

This revision is now accepted and ready to land.Mar 30 2017, 2:54 PM
rjvbb added a comment.Mar 30 2017, 4:05 PM

Is this the new review platform?

Yep.

LGTM. Hopefully I'm doing the right action here.... (phabricator always seem to have way more buttons than I need..)

I think so. Phab is indeed considerably less intuitive than ReviewBoard was. It appears it's designed for software professionals (whatever that means/implies ;) ).

In order to properly close the review, please either use arcanist (suggested method) or the last line should have been:
Differential Revision: https://phabricator.kde.org/D5178

ltoscano set the repository for this revision to R626 QtCurve.Mar 30 2017, 4:17 PM
ltoscano removed a subscriber: ltoscano.
ltoscano added a subscriber: ltoscano.
rjvbb added a comment.EditedMar 30 2017, 4:48 PM

I don't understand why you had to set the repository, I'm positive I did that during the initial review creation.

Differential Revision: https://phabricator.kde.org/D5178

I suppose it would technically be possible to support REVIEW: instead of Differential Revision: on a commit's last line once RB has been retired for good? It'd be shorter and clearer (natural language vs. rather silly Phab. specific jargon).

We won't be modifying Phabricator, in part because Arcanist as shipped by upstream is designed to work with that text.
Modifying it will create maintenance burden in the long run and lead to incompatibility with the software which runs on developers systems - requiring us to maintain a fork of Arcanist as well.

rjvbb added a comment.Mar 31 2017, 8:40 AM

We won't be modifying Phabricator, in part because Arcanist as shipped by upstream is designed to work with that text.

We're talking about the trigger in commit messages made with regular git commands, right? I'm not sure I understand why you couldn't use any trigger in whatever hook is used on KDE's git server to close a particular review but I'm pretty sure I don't particular care either.
I do understand that you're not looking for extra work. Neither am I.