Details

Reviewers

davidedmundson

Group Reviewers

KWin

Commits

R108:2211a951e795: [kcmkwin] Split out Desktop Effects KCM

Summary

A while ago desktop effects and compositing settings lived under the
same roof

but time has passed and now those two have their own kcms. This causes
some issues:

for newcomers it's harder to find code of the Desktop Effects KCM;
git history doesn't look good, e.g. "[kcmkwin/compositing] Add some bugs to Desktop Effects KCM to fix later";
in general, the mix of two doesn't look good in the code.

This change splits out the Desktop Effects KCM. Unfortunately, in order
to have more nicer code I had to refactor EffectModel a little bit.

Before:

After:

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.

zzag created this revision.Feb 3 2019, 5:03 PM

Restricted Application added a project: KWin. · View Herald TranscriptFeb 3 2019, 5:03 PM

Restricted Application added a subscriber: kwin. · View Herald Transcript

zzag requested review of this revision.Feb 3 2019, 5:03 PM

Harbormaster completed remote builds in B7817: Diff 50783.Feb 3 2019, 5:03 PM

update

Harbormaster completed remote builds in B7823: Diff 50789.Feb 3 2019, 5:10 PM

zzag added a dependent revision: D18704: [kcmkwin] Add requestConfigure to effects model.Feb 3 2019, 8:18 PM

Why is the title smaller now?

In D18703#404670, @ngraham wrote:

Why is the title smaller now?

Probably because of KQuickAddons::ConfigModule. Virtual Desktops KCM also has smaller title.

In D18703#404672, @zzag wrote:

In D18703#404670, @ngraham wrote:

Why is the title smaller now?

Probably because of KQuickAddons::ConfigModule. Virtual Desktops KCM also has smaller title.

@mart is this another case where we need to make the title use the same metrics as Kirigami's Heading?

zzag planned changes to this revision.Feb 4 2019, 1:24 AM

Address coding style issues.

Harbormaster completed remote builds in B7842: Diff 50821.Feb 4 2019, 9:32 AM

Yet another sneaky update.

Harbormaster completed remote builds in B7843: Diff 50822.Feb 4 2019, 9:37 AM

Harbormaster completed remote builds in B7844: Diff 50823.Feb 4 2019, 9:40 AM

Harbormaster completed remote builds in B7862: Diff 50850.Feb 4 2019, 2:39 PM

Rebase on master; fix merge conflicts;
Change the title to "Configure Desktop Effects."

Harbormaster completed remote builds in B7881: Diff 50886.Feb 4 2019, 9:13 PM

zzag edited the summary of this revision. (Show Details)Feb 4 2019, 9:18 PM

@ltoscano
We're splitting a .pot file into two .pots. Is there anything we can do to make translators lives easier?

kcmkwin/kwineffects/package/contents/ui/Effect.qml
51	Can you explain why we have _toggled and both onToggled and onClicked handlers

In D18703#416650, @davidedmundson wrote:

@ltoscano
We're splitting a .pot file into two .pots. Is there anything we can do to make translators lives easier?

We can copy create a copy of the existing translation file using the new name.
Will you keep the old .pot and add a new one with a subset of strings?

Will you keep the old .pot and add a new one with a subset of strings?

Yes.

old pot is kcmkwincompositing.pot
new pots will be kcmkwincompositing.pot and kcm_kwin_effects.pot

zzag added inline comments.Feb 21 2019, 3:01 PM

kcmkwin/kwineffects/package/contents/ui/Effect.qml
51	It appears that it's not possible to uncheck a radio button in an exclusive group. One would need that, for example, to disable the zoom effect.

Retrieve proper transient parent from context.

Thank you, David.

Harbormaster completed remote builds in B8643: Diff 52227.Feb 21 2019, 6:05 PM

go back to the previous version

Harbormaster completed remote builds in B8670: Diff 52273.Feb 22 2019, 9:28 AM

As for your transients problem.

We open the configuration UI with the QQuickView as the transient. This is not a top level as it's embedded in the KCM.

In this case QtWayland does try to look for parents or transientParents of the window.
But as this QQuickView is an offscreen surface it doesn't even have a parent or transient parent. QtWayland has nothing to go on.

This comes up in multiple places, such as GHNS in every QtQuick KCM

The more interesting question isn't why wayland doen't work, but why x does work.

It works on X11 because dialogs become group transients (this is both a feature and a bug).

At least that transient problem is fully understood. We have to solve that somewhere else as it affects the other KCMs.
Can you file a bug report (or phab task or whatever) copying the messages above so it doesn't get lost, and then lets get this shipped.

This revision is now accepted and ready to land.Mar 17 2019, 9:41 PM

In D18703#433306, @davidedmundson wrote:

Can you file a bug report (or phab task or whatever)

Against which product/component should I file a bug report?

https://bugs.kde.org/show_bug.cgi?id=405588

Closed by commit R108:2211a951e795: [kcmkwin] Split out Desktop Effects KCM (authored by zzag). · Explain WhyMar 18 2019, 11:42 AM

This revision was automatically updated to reflect the committed changes.

		Path
M		kcmkwin/CMakeLists.txt (1 line)
M		kcmkwin/common/effectmodel.h (12 lines)
M		kcmkwin/common/effectmodel.cpp (43 lines)
M		kcmkwin/kwincompositing/CMakeLists.txt (52 lines)
D	M	kcmkwin/kwincompositing/effectconfig.h (47 lines)
D	M	kcmkwin/kwincompositing/effectconfig.cpp (115 lines)
V		kcmkwin/{kwincompositing/kcmkwineffects.desktop → kwineffects/kcm_kwin_effects.desktop}
V		kcmkwin/{kwincompositing → kwineffects}/kwineffect.knsrc
M		kcmkwin/kwincompositing/main.cpp (65 lines)
D	M	kcmkwin/kwincompositing/model.h (121 lines)
D	M	kcmkwin/kwincompositing/model.cpp (212 lines)
D	M	kcmkwin/kwincompositing/qml/Effect.qml (163 lines)
D	M	kcmkwin/kwincompositing/qml/EffectView.qml (179 lines)
V		kcmkwin/{kwincompositing/qml → kwineffects/package/contents/ui}/Video.qml
D	M	kcmkwin/kwincompositing/qml/main.qml (31 lines)
D	M	kcmkwin/kwincompositing/test/effectmodeltest.h (38 lines)
D	M	kcmkwin/kwincompositing/test/effectmodeltest.cpp (46 lines)
D	M	kcmkwin/kwincompositing/test/modeltest.h (96 lines)
D	M	kcmkwin/kwincompositing/test/modeltest.cpp (600 lines)
A	M	kcmkwin/kwineffects/CMakeLists.txt (33 lines)
A	M	kcmkwin/kwineffects/Messages.sh (2 lines)
A	M	kcmkwin/kwineffects/effectsfilterproxymodel.h (62 lines)
A	M	kcmkwin/kwineffects/effectsfilterproxymodel.cpp (104 lines)
A	M	kcmkwin/kwineffects/kcm.h (58 lines)
A	M	kcmkwin/kwineffects/kcm.cpp (192 lines)
V		kcmkwin/{kwineffects/kcm_kwin_effects.desktop ← kwincompositing/kcmkwineffects.desktop} (4 lines)
V		kcmkwin/{kwineffects ← kwincompositing}/kwineffect.knsrc
A	M	kcmkwin/kwineffects/package/contents/ui/Effect.qml (140 lines)
V		kcmkwin/{kwineffects/package/contents/ui ← kwincompositing/qml}/Video.qml
A	M	kcmkwin/kwineffects/package/contents/ui/main.qml (154 lines)
A	M	kcmkwin/kwineffects/package/metadata.desktop (102 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	50783	5ad797d		Feb 3 2019, 5:03 PM	★	★
Diff 2	50789	5ad797d	update	Feb 3 2019, 5:10 PM	★	★
Diff 3	50821	5ad797d	Address coding style issues.	Feb 4 2019, 9:32 AM	★	★
Diff 4	50822	5ad797d	Yet another sneaky update.	Feb 4 2019, 9:37 AM	★	★
Diff 5	50823	5ad797d	.	Feb 4 2019, 9:40 AM	★	★
Diff 6	50850	5ad797d	.	Feb 4 2019, 2:39 PM	★	★
Diff 7	50886	8eaf40c	* Rebase on master; fix merge conflicts;	Feb 4 2019, 9:13 PM	★	★
Diff 8	52227	be87291	Retrieve proper transient parent from context.	Feb 21 2019, 6:05 PM	★	★
Diff 9	52273	be87291	go back to the previous version	Feb 22 2019, 9:28 AM	★	★
Diff 10	54196	5eb2851	R108:2211a951e795071acf01d9a7085a09dee405dd63	Mar 18 2019, 11:08 AM	★	★

[kcmkwin] Split out Desktop Effects KCM
ClosedPublic
Actions

Details

Diff Detail

Revision Contents
Changeset List

Diff 54196

kcmkwin/CMakeLists.txt

kcmkwin/common/effectmodel.h

kcmkwin/common/effectmodel.cpp

kcmkwin/kwincompositing/CMakeLists.txt

kcmkwin/kwincompositing/effectconfig.h

kcmkwin/kwincompositing/effectconfig.cpp

kcmkwin/kwincompositing/kcmkwineffects.desktop

kcmkwin/kwincompositing/kwineffect.knsrc

kcmkwin/kwincompositing/main.cpp

kcmkwin/kwincompositing/model.h

kcmkwin/kwincompositing/model.cpp

kcmkwin/kwincompositing/qml/Effect.qml

kcmkwin/kwincompositing/qml/EffectView.qml

kcmkwin/kwincompositing/qml/Video.qml

kcmkwin/kwincompositing/qml/main.qml

kcmkwin/kwincompositing/test/effectmodeltest.h

kcmkwin/kwincompositing/test/effectmodeltest.cpp

kcmkwin/kwincompositing/test/modeltest.h

kcmkwin/kwincompositing/test/modeltest.cpp

kcmkwin/kwineffects/CMakeLists.txt

kcmkwin/kwineffects/Messages.sh

kcmkwin/kwineffects/effectsfilterproxymodel.h

kcmkwin/kwineffects/effectsfilterproxymodel.cpp

kcmkwin/kwineffects/kcm.h

kcmkwin/kwineffects/kcm.cpp

kcmkwin/kwineffects/kcm_kwin_effects.desktop

kcmkwin/kwineffects/kwineffect.knsrc

kcmkwin/kwineffects/package/contents/ui/Effect.qml

kcmkwin/kwineffects/package/contents/ui/Video.qml

kcmkwin/kwineffects/package/contents/ui/main.qml

kcmkwin/kwineffects/package/metadata.desktop

[kcmkwin] Split out Desktop Effects KCMClosedPublicActions

Details

Diff Detail

Revision ContentsChangeset List

Diff 54196

kcmkwin/CMakeLists.txt

kcmkwin/common/effectmodel.h

kcmkwin/common/effectmodel.cpp

kcmkwin/kwincompositing/CMakeLists.txt

kcmkwin/kwincompositing/effectconfig.h

kcmkwin/kwincompositing/effectconfig.cpp

kcmkwin/kwincompositing/kcmkwineffects.desktop

kcmkwin/kwincompositing/kwineffect.knsrc

kcmkwin/kwincompositing/main.cpp

kcmkwin/kwincompositing/model.h

kcmkwin/kwincompositing/model.cpp

kcmkwin/kwincompositing/qml/Effect.qml

kcmkwin/kwincompositing/qml/EffectView.qml

kcmkwin/kwincompositing/qml/Video.qml

kcmkwin/kwincompositing/qml/main.qml

kcmkwin/kwincompositing/test/effectmodeltest.h

kcmkwin/kwincompositing/test/effectmodeltest.cpp

kcmkwin/kwincompositing/test/modeltest.h

kcmkwin/kwincompositing/test/modeltest.cpp

kcmkwin/kwineffects/CMakeLists.txt

kcmkwin/kwineffects/Messages.sh

kcmkwin/kwineffects/effectsfilterproxymodel.h

kcmkwin/kwineffects/effectsfilterproxymodel.cpp

kcmkwin/kwineffects/kcm.h

kcmkwin/kwineffects/kcm.cpp

kcmkwin/kwineffects/kcm_kwin_effects.desktop

kcmkwin/kwineffects/kwineffect.knsrc

kcmkwin/kwineffects/package/contents/ui/Effect.qml

kcmkwin/kwineffects/package/contents/ui/Video.qml

kcmkwin/kwineffects/package/contents/ui/main.qml

kcmkwin/kwineffects/package/metadata.desktop

[kcmkwin] Split out Desktop Effects KCM
ClosedPublic
Actions

Revision Contents
Changeset List