Review Request 130219: Enable background color to be changed from settings

[Albert Freeman]

> On Aug. 9, 2017, 6:24 p.m., Albert Astals Cid wrote:
> > Thanks for contributing to Okular :)
> > 
> > Why would you need a different background color?
> 
> Albert Freeman wrote:
>     I personally much prefer black as a background color.
> 
> Albert Astals Cid wrote:
>     Maybe you could edit your color scheme in system settings instead?
> 
> Albert Freeman wrote:
>     Through system settings I can change the okular background color but it also affects the window color of the rest of okular and all other applications. Additionally it seems to mix whatever color is selected with grey for the final background color, so if I set it as RGB 0, 0, 0 black it will appear a darker grey and if I set it RGB 255, 255, 255 white it will appear a lighter grey. The rest of the okular window does not mix with grey howevver. Should I try to make the system settings colors more granular in whichever kde codebase is responsible AND require an okular code change to adapt to that or just allow a choice of custom background color or system color scheme in okular in a better way than my attached code does (since my attached code is flawed)? Or should I just patch this manually to okular source code just on my machine?
> 
> Albert Astals Cid wrote:
>     I do appreciate that this is very important for you, but honestly i think that following the color scheme is the right thing to do [on a side note, I don't see how we change the color, all we do is directly call QPainter:::fillRect( ..., backColor ); ]
>     
>     You are right, adding another "different background" setting in system settings color schemes doesn't make sense, how would you even name it?
>     
>     At this point we have two options, either accept the code or not.
>     
>     I have two problems with accepting the code:
>     
>     * In my mind, It is a very niche feature
>     * Once we accept the feature we have to maintain it for all of the eternity (okular codebase is more than 10 years old and has gone though Qt3 to Qt4 porting and Qt4 to Qt5 porting) so even if it's "not much code", it all adds up
>     
>     I'm open to convincing that it's not a niche feature or that it's really trivial to maintain for the next 10 years :)

Youtube video with ~100 upvotes and ~60'000 views (although it changes the pages color instead of the background color)(https://www.youtube.com/watch?v=_N7PgpkH8lM)

So the code is useful for accessibility, eye strain and aesthetic reasons.

Additionally, power consumption is minorly affected by color and depends on display technology, sometimes black uses more power, othertimes white does (https://www.quora.com/Does-a-white-background-use-more-energy-on-an-LCD-than-if-it-was-set-to-black)


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130219/#review103582
-----------------------------------------------------------


On Aug. 20, 2017, 2:59 p.m., Albert Freeman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130219/
> -----------------------------------------------------------
> 
> (Updated Aug. 20, 2017, 2:59 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> Enable background color to be changed from settings
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt ab7ad239 
>   conf/dlggeneral.h f363d260 
>   conf/dlggeneral.cpp 964a655b 
>   conf/dlggeneralbase.ui cf4ebca0 
>   conf/okular.kcfg 69ea8cf6 
>   mobile/components/CMakeLists.txt f1af2602 
>   ui/pageview.cpp 3d935a2e 
>   ui/pageviewutils.cpp a57712ca 
> 
> 
> Diff: https://git.reviewboard.kde.org/r/130219/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Albert Freeman
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20170917/6a13731b/attachment.html>