D18487: Reparse background contrast settings when colors changed

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> mart wrote in theme_p.cpp:754
> They can be supplied from the theme too (or used to be) even though most of 
> the themes don't

err, forget everything i said, i misunderstood your comment :)
 using qgray on the theme backgroundcolor instead of a magic 0.45 is an 
excellent idea

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

To: broulik, #plasma
Cc: mart, ngraham, kde-frameworks-devel, michaelh, bruns

D18487: Reparse background contrast settings when colors changed

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> broulik wrote in theme_p.cpp:754
> If the theme does not supply background contrast values, they are 
> auto-determined from the background color (dark or light), or what do you 
> mean?

They can be supplied from the theme too (or used to be) even though most of the 
themes don't

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

To: broulik, #plasma
Cc: mart, ngraham, kde-frameworks-devel, michaelh, bruns

D18487: Reparse background contrast settings when colors changed

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> mart wrote in theme_p.cpp:754
> but it wouldn't be guaranteed to be the same of the value stored in the 
> config file no?

If the theme does not supply background contrast values, they are 
auto-determined from the background color (dark or light), or what do you mean?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

To: broulik, #plasma
Cc: mart, ngraham, kde-frameworks-devel, michaelh, bruns

D18487: Reparse background contrast settings when colors changed

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> broulik wrote in theme_p.cpp:754
> An alternative approach would be to store those values and then do the 
> default fallback and `qGray(color(Plasma::Theme::BackgroundColor).rgb()) < 
> 127` stuff on the fly on demand.
> It's runtime cpu cycles vs parsing a config file once on color change (with 
> some awful copy paste)

but it wouldn't be guaranteed to be the same of the value stored in the config 
file no?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

To: broulik, #plasma
Cc: mart, ngraham, kde-frameworks-devel, michaelh, bruns

D18487: Reparse background contrast settings when colors changed

[Nathaniel Graham]

ngraham added a comment.


  Nice, it works great for me!

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

To: broulik, #plasma
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D18487: Reparse background contrast settings when colors changed

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> theme_p.cpp:754
>  }
>  backgroundContrast = cg.readEntry("contrast", _contrast);
>  backgroundIntensity = cg.readEntry("intensity", _intensity);

An alternative approach would be to store those values and then do the default 
fallback and `qGray(color(Plasma::Theme::BackgroundColor).rgb()) < 127` stuff 
on the fly on demand.
It's runtime cpu cycles vs parsing a config file once on color change (with 
some awful copy paste)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

To: broulik, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18487: Reparse background contrast settings when colors changed

[Kai Uwe Broulik]

broulik created this revision.
broulik added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  When switching between light and dark system colors, the background contrast 
might change which we didn't do at runtime leading to washed out Plasma popup 
backgrounds.
  Also, while at it, update complimentary colorscheme as well, which was 
forgotten here.
  
  BUG: 401142

TEST PLAN
  - Used Breeze Plasma theme (the one that follows system colors), switched 
from Breeze light color scheme to Breeze dark, popups immediately looked as 
they should have rather than only after a plasmashell restart

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D18487

AFFECTED FILES
  src/plasma/private/theme_p.cpp

To: broulik, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns