<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/130219/">https://git.reviewboard.kde.org/r/130219/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 9th, 2017, 6:24 p.m. UTC, <strong>Albert Astals Cid</strong> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for contributing to Okular :)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Why would you need a different background color?</p></pre>
</blockquote>
<p>On August 11th, 2017, 2:53 a.m. UTC, <strong>Albert Freeman</strong> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I personally much prefer black as a background color.</p></pre>
</blockquote>
<p>On August 11th, 2017, 11:08 p.m. UTC, <strong>Albert Astals Cid</strong> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Maybe you could edit your color scheme in system settings instead?</p></pre>
</blockquote>
<p>On August 18th, 2017, 3:32 a.m. UTC, <strong>Albert Freeman</strong> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 flaw
ed)? Or should I just patch this manually to okular source code just on my machine?</p></pre>
</blockquote>
<p>On August 22nd, 2017, 10 p.m. UTC, <strong>Albert Astals Cid</strong> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 ); ]</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You are right, adding another "different background" setting in system settings color schemes doesn't make sense, how would you even name it?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">At this point we have two options, either accept the code or not.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have two problems with accepting the code:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">In my mind, It is a very niche feature</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">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</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 :)</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So the code is useful for accessibility, eye strain and aesthetic reasons.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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)</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On August 20th, 2017, 2:59 p.m. UTC, Albert Freeman wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Albert Freeman.</div>
<p style="color: grey;"><em>Updated Aug. 20, 2017, 2:59 p.m.</em></p>
<div style="margin-top: 1.5em;">
<strong style="color: #575012; font-size: 10pt;">Repository: </strong>
okular
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Enable background color to be changed from settings</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(ab7ad239)</span></li>
<li>conf/dlggeneral.h <span style="color: grey">(f363d260)</span></li>
<li>conf/dlggeneral.cpp <span style="color: grey">(964a655b)</span></li>
<li>conf/dlggeneralbase.ui <span style="color: grey">(cf4ebca0)</span></li>
<li>conf/okular.kcfg <span style="color: grey">(69ea8cf6)</span></li>
<li>mobile/components/CMakeLists.txt <span style="color: grey">(f1af2602)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(3d935a2e)</span></li>
<li>ui/pageviewutils.cpp <span style="color: grey">(a57712ca)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/130219/diff/3/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>