DevHeads.net

Review Request 118898: KGamma: Apply user setting at login/startup

Review request for kde-workspace, Plasma and Marcel Wiesweg.

Bugs: 218668
<a href="http://bugs.kde.org/show_bug.cgi?id=218668" title="http://bugs.kde.org/show_bug.cgi?id=218668">http://bugs.kde.org/show_bug.cgi?id=218668</a>

Repository: kgamma

Description
KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays)

This patch factors out the function init_kgamma() into its own source file (init_kgamma.cpp), adds a small executable (applykgammasettings) that just applies those settings by calling that function, and installs applykgammasettings.desktop to ${AUTOSTART_INSTALL_DIR} that runs applykgammasettings on login.

PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;)

Diffs
kcmkgamma/CMakeLists.txt 3980023
kcmkgamma/applykgammasettings.cpp PRE-CREATION
kcmkgamma/applykgammasettings.desktop PRE-CREATION
kcmkgamma/init_kgamma.cpp PRE-CREATION
kcmkgamma/kgamma.cpp 890ba99

Diff: <a href="https://git.reviewboard.kde.org/r/118898/diff/" title="https://git.reviewboard.kde.org/r/118898/diff/">https://git.reviewboard.kde.org/r/118898/diff/</a>

Testing
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.

If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default).

Thanks,

Wolfgang Bauer

Comments

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/23/2014 - 12:01

(Updated June 23, 2014, 7:01 p.m.)

Review request for kde-workspace, Plasma and Marcel Wiesweg.

Changes
Use kcminit instead of creating a separate executable and starting it via an autostart .desktop file

Bugs: 218668
<a href="http://bugs.kde.org/show_bug.cgi?id=218668" title="http://bugs.kde.org/show_bug.cgi?id=218668">http://bugs.kde.org/show_bug.cgi?id=218668</a>

Repository: kgamma

Description (updated)
KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays)

This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4.

PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;)

Diffs (updated)
kcmkgamma/kgamma.cpp 890ba99
kcmkgamma/kgamma.desktop 3d87513

Diff: <a href="https://git.reviewboard.kde.org/r/118898/diff/" title="https://git.reviewboard.kde.org/r/118898/diff/">https://git.reviewboard.kde.org/r/118898/diff/</a>

Testing
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.

If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default).

Thanks,

Wolfgang Bauer

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/24/2014 - 03:04

(Updated June 24, 2014, 10:04 a.m.)

Review request for kde-workspace, KDE Graphics, Plasma, and Marcel Wiesweg.

Changes
Added kdegraphics to the review groups as kgamma is part of kdegraphics

Bugs: 218668
<a href="http://bugs.kde.org/show_bug.cgi?id=218668" title="http://bugs.kde.org/show_bug.cgi?id=218668">http://bugs.kde.org/show_bug.cgi?id=218668</a>

Repository: kgamma

Description
KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays)

This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4.

PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;)

Diffs
kcmkgamma/kgamma.cpp 890ba99
kcmkgamma/kgamma.desktop 3d87513

Diff: <a href="https://git.reviewboard.kde.org/r/118898/diff/" title="https://git.reviewboard.kde.org/r/118898/diff/">https://git.reviewboard.kde.org/r/118898/diff/</a>

Testing
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.

If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default).

Thanks,

Wolfgang Bauer

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/24/2014 - 05:58

(Updated June 24, 2014, 10:58 a.m.)

Status
This change has been marked as submitted.

Review request for kde-workspace, KDE Graphics, Plasma, and Marcel Wiesweg.

Bugs: 218668
<a href="http://bugs.kde.org/show_bug.cgi?id=218668" title="http://bugs.kde.org/show_bug.cgi?id=218668">http://bugs.kde.org/show_bug.cgi?id=218668</a>

Repository: kgamma

Description
KGamma's saved user settings are not applied on startup/login. The user has to enter the KCM to apply them.
This makes it rather useless, as not even saving the settings system-wide really works any more. (this requires an xorg.conf which normally doesn't exist nowadays)

This patch uses kcminit to apply these settings again on login. Apparently this has been forgotten when moving/porting kgamma to KDE4.

PS: As there seems to be no kgamma group and this is desktop-related, I decided to add the kde-workspace and plasma groups for review. I hope that's ok... ;)

Diffs
kcmkgamma/kgamma.cpp 890ba99
kcmkgamma/kgamma.desktop 3d87513

Diff: <a href="https://git.reviewboard.kde.org/r/118898/diff/" title="https://git.reviewboard.kde.org/r/118898/diff/">https://git.reviewboard.kde.org/r/118898/diff/</a>

Testing
Set a gamma value in the KGamma KCM, logout/login (or reboot), Gamma value
gets set correctly.

If there's no kgammarc file (or it contains no actual gamma settings), the Gamma value is not changed. It stays at what is configured for X (or its default).

Thanks,

Wolfgang Bauer

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Commit Hook at 06/24/2014 - 06:03

This review has been submitted with commit 82a264a08c2356f68c76ca6dafebe139639caf56 by Wolfgang Bauer to branch master.

- Commit Hook

On June 24, 2014, 10:58 a.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Commit Hook at 06/24/2014 - 05:58

This review has been submitted with commit abb774fa60709102cc86daeef035cc8d59a9ef09 by Wolfgang Bauer to branch KDE/4.13.

- Commit Hook

On June 24, 2014, 8:04 a.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Sebastian =?utf... at 06/24/2014 - 05:33

Ship it!

Cool, this looks much less intrusive.

- Sebastian Kügler

On June 24, 2014, 8:04 a.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Christoph Feck at 06/23/2014 - 17:41

Not sure why you added Marcel to the list of reviewers...

Anyway, if the current patch is all that is needed to restore sanity as in KDE 3, the I am all for getting it fixed.

- Christoph Feck

On June 23, 2014, 5:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/24/2014 - 02:39

- Wolfgang

On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Christoph Feck at 06/23/2014 - 17:44

(And if the same issue was the cause for the KRandR regression, then I will eat my hat).

- Christoph

On June 23, 2014, 5:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Martin =?ISO-88... at 06/23/2014 - 10:13

kcmkgamma/init_kgamma.cpp
<https://git.reviewboard.kde.org/r/118898/#comment42380>

please use a copyright header as in <a href="http://techbase.kde.org/Policies/Licensing_Policy#GPL_Header" title="http://techbase.kde.org/Policies/Licensing_Policy#GPL_Header">http://techbase.kde.org/Policies/Licensing_Policy#GPL_Header</a>

kcmkgamma/init_kgamma.cpp
<https://git.reviewboard.kde.org/r/118898/#comment42381>

why delete config? I would just use a KSharedConfig::openConfig

- Martin Gräßlin

On June 23, 2014, 3:06 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/23/2014 - 12:45

OK. I dropped this issue then.

- Wolfgang

On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Martin =?ISO-88... at 06/23/2014 - 12:18

no, it isn't. It just highlights how bad reviewboard is for copied content

- Martin

On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/23/2014 - 12:03

I just copied the license from the original source file (kgamma.cpp).
But the new source file (init_kgamma.cpp) doesn't exist any more in the updated patch.

I just copy/pasted the original code.
That is reverted as well now.

Should I change this anyway?
But that's not really related to this bugfix, I'd say...

- Wolfgang

On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Sebastian =?utf... at 06/23/2014 - 08:21

I think there's a couple of problems with this approach:

- This slows down startup for everybody, not just those who changed the setting. I'm not super-familiar with this, but isn't kcminit for this use-case?
- Changing startup procedure this late in the game (Plasma 4.x has been on LTS for almost a year, feature frozen for much longer)
- This particular KCM is dead in Plasma 5 (not really a reason to not "fix it" in 4.x, but the feature will be lost again

Again, not super-privvy of the whole picture, but isn't color correction the correct solution here?

- Sebastian Kügler

On June 23, 2014, 1:06 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/23/2014 - 12:02

Thanks for the hint!
Kcminit works just fine. I just had to add some stuff to the .desktop file and rename/export the init function, so the patch is much simpler now.
Apparently it worked the way it is in KDE3 and has been forgotten to be changed when porting/moving kgamma to KDE4?

- Wolfgang

On June 23, 2014, 7:01 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/23/2014 - 09:25

PS: Sorry, kcminit does seem to be able to apply settings on login. I seem to have confused it with something else...
I will have a look at this then.

- Wolfgang

On June 23, 2014, 3:06 p.m., Wolfgang Bauer wrote:

Re: Review Request 118898: KGamma: Apply user setting at login/s

By Wolfgang Bauer at 06/23/2014 - 09:22

applykgammasettings is only called on login for people actually installing kgamma (it's a separate tarball and separate package on openSUSE at least).
There is no change to plasma's own startup procedure, and no change at all when kgamma is not installed.

kcminit is actually used by kgamma right now (it calls the same function), but that only applies when the user enters the KCM. But the point of this setting is to be applied automatically on login/startup.

- Wolfgang

On June 23, 2014, 3:06 p.m., Wolfgang Bauer wrote: