[kcmkwin/compositing] Port Effects KCM to QQC2
ClosedPublic

Authored by ngraham on Jan 2 2019, 7:34 PM.

Details

Summary

This patch ports the Effects KCM to QQC2, which yields the following benefits:

  1. General performance enhancements of only using QQC2 rather than a mix of 1 and 2
  2. Some code simplification is possible
  3. Improves the appearance of the checkable menu items in the dropdown menu, fixing https://bugs.kde.org/show_bug.cgi?id=402701
  4. Improves the appearance when using a fractional scale factor, fixing https://bugs.kde.org/show_bug.cgi?id=396725
  5. Fixes the incorrect size when opened from kcmshell5

Along the way, two visual changes are introduced as a by-product of porting:

  1. The scrollbar is inline, so it overlaps some of the buttons in the content. This is somewhat undesirable, but adopting a Kirigami scrollview would fix this.
  2. The button that displays a dropdown menu no longer has a downward-pointing arrow to indicate as such. This is not my preference, but there's a benefit to being consistent, and eventually we could can change this in one place to impeove the appearance of buttons that display dropdown menus everywhere in one fell swoop.

BUG: 396725
BUG: 402701
BUG: 396076
FIXED-IN: 5.15.0

Test Plan

All functionality still works

In System Settings, showing improved checkable menu item appearance:

In kcmshell at 1x scale:

In kcmshell at 1.5x scale:

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Jan 2 2019, 7:34 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 2 2019, 7:34 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
ngraham requested review of this revision.Jan 2 2019, 7:34 PM
ngraham edited the summary of this revision. (Show Details)Jan 2 2019, 7:37 PM
ngraham edited the test plan for this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)Jan 2 2019, 7:47 PM
davidedmundson added inline comments.
kcmkwin/kwincompositing/qml/EffectView.qml
48

good fix!

70

In RTL is this correct?

98

For the borders add this to the ScrollView

Component.onCompleted: background.visible = true;

152

I know you've just moved this, but this is messy.

You can't specify x and an horizontal anchor.

It should be fill horizontally and use Text.alignment

Though we can fix after.

zzag retitled this revision from [RFC] [Effects KCM] Port to QQC2 to [RFC] [kcmkwin/compositing] Port Effects KCM to QQC2.Jan 3 2019, 2:33 PM
ngraham updated this revision to Diff 48626.Jan 3 2019, 7:44 PM
ngraham marked 4 inline comments as done.

Address all review comments

ngraham added inline comments.Jan 3 2019, 7:44 PM
kcmkwin/kwincompositing/qml/EffectView.qml
70

Yep, works just fine when run with -reverse!

ngraham marked an inline comment as done.Jan 3 2019, 7:44 PM
ngraham retitled this revision from [RFC] [kcmkwin/compositing] Port Effects KCM to QQC2 to [kcmkwin/compositing] Port Effects KCM to QQC2.
ngraham edited the summary of this revision. (Show Details)
ngraham edited the test plan for this revision. (Show Details)
ngraham updated this revision to Diff 48629.Jan 3 2019, 7:55 PM

Rebase on master

ngraham updated this revision to Diff 48630.Jan 3 2019, 7:56 PM

Fix bad rebase

ngraham updated this revision to Diff 48632.Jan 3 2019, 7:58 PM

Fix it for real

ngraham edited the summary of this revision. (Show Details)Jan 6 2019, 6:37 AM
davidedmundson accepted this revision.Jan 6 2019, 10:39 PM
This revision is now accepted and ready to land.Jan 6 2019, 10:39 PM
This revision was automatically updated to reflect the committed changes.