<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/102540/">http://git.reviewboard.kde.org/r/102540/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 27th, 2011, 6:16 p.m., <b>Bernhard Beschow</b> 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;">Thanks for the patch, having the route colors adjustable rather than hardcoded is a very good idea. Having applied your patch and seeing the rather complicated color selection dialog, however, I wondered whether it was sufficient to just set a (single) alpha transparency value, as requested in the bug report. As an additional benefit, this would also prevent regular users to "mess" with the routing colors, which have been choosen to be distinct from the tracking etc. ones (such that they make up a "color theme").
What I suggest is therefore the following: Keep the colors "soft"-coded from the config file, such that computer-savvy users can still change the colors. For regular users, there should only be a single alpha transparency selector in the config dialog, setting a common transparency for all three routing colors.
What do you think?</pre>
</blockquote>
<p>On October 28th, 2011, 9:41 a.m., <b>Torsten Rahn</b> 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;">What is complex about the color selection? We do it the same way for the other plugins from what I can see ...
</pre>
</blockquote>
<p>On October 28th, 2011, 11:18 a.m., <b>Bernhard Beschow</b> 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;">Well, I was just expressing my personal perception of the color selection dialog. First, I have no idea what the radio buttons are about. Second, the technical term "Alpha" is used, whereas we use the IMO more user-friendly term "transparency". So, in order to change the transparency value (for a single case!), users first have to identify the correct field out of seven (!) fields. However, this is a challenge in itself, because users have to map the term "transparency" to "alpha". These might be the reasons why the color selection dialog appears complex to me.
Anyway, the appearence of the color selection dialog just made me realize that color selection has a few issues: First, it might suffice to set a single transparency value for all three cases, which may be enough to satisfy the original request. Second, selecting custom colors might break our "color theme". If there was a need to change the colors, we'd probably have an issue with our defaults.
In any case, having the colors soft-coded in the config files, power users were still able to change the colors.</pre>
</blockquote>
<p>On October 28th, 2011, 11:45 a.m., <b>Bernhard Beschow</b> 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;">I admit that we use the term "transparency" only in our discussion. Still, I find this term to be more intuitive and non-technical than "alpha".
Moreover, it might seem that I question the whole patch. However, this is not the case. I much prefer soft-coded values rather than hard-coded ones! So the approach taken for the config settings goes IMO in the right direction.</pre>
</blockquote>
<p>On October 28th, 2011, 12:30 p.m., <b>Torsten Rahn</b> 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;">Hi Bernhard. You are totally right, that QColorDialog these days is almost too much of a Cockpit UI experience. I feel that the plain palette (which currently defaults to 40 colors instead of Oxygen which I consider a bug) should be central to the user experience and the tweaking stuff (like Hue, RGB, etc.) should only appear on demand.
In earlier versions of QColorDialog the palette part was indeed on the top left and much more prominent than the rest of the dialog's features. Seems things have become worse.
However I feel that this issue is separate from this patch, since it affects QColorDialog itself and it should be resolved for all the other instances where we are using QColorDialog as well. A stripped down version of QColorDialog with some easy way to adjust alpha would be nice to have for most other use cases we have.</pre>
</blockquote>
<p>On October 28th, 2011, 12:32 p.m., <b>Torsten Rahn</b> 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;">oh yes, and of course in that user friendly color dialog we could use the less technical term "transparency" indeed. ;-)</pre>
</blockquote>
<p>On October 28th, 2011, 1:34 p.m., <b>Bernhard Beschow</b> 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;">I agree, changing QColorDialog is a separate issue. Still, I wonder whether we really need users to adjust the colors for the routes. IMO, an in-place user control to change the transparency value for all three cases at once, such as a slider, would do. A preview of the colors would be nice, though.</pre>
</blockquote>
<p>On November 1st, 2011, 11:26 a.m., <b>Torsten Rahn</b> 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;">I think allowing to change colors is something that should be possible given that different maps have different color schemes and one might want to choose a different one to match the color scheme of the preferred map (e.g. for print out).
Regarding the obstruction of the path that you are driving into: How do the other navigation solutions do it? Do they show arrows instead of the plain path? Or do they choose the color so that you can still see what is displayed behind? Maybe we should also evaluate to default to a different route visualization during the actual navigation mode.</pre>
</blockquote>
<p>On November 1st, 2011, 3:51 p.m., <b>Bernhard Beschow</b> 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;">> I think allowing to change colors is something that should be possible given that different maps have different color schemes
Ok. So what about usetting the QColorDialog::ShowAlphaChannel option of the color dialog and have a single transparency slider for all three colors on the settings page, possibly with a live preview?</pre>
</blockquote>
<p>On November 1st, 2011, 6:15 p.m., <b>Torsten Rahn</b> 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;">Bernhard: Sounds good to me. Shall we have the patch merged first before we tweak it further so that Florian feels better? ;-)</pre>
</blockquote>
<p>On November 3rd, 2011, 9:31 a.m., <b>Florian Eßer</b> 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;">Interesting discussion. (I have been away for some days...)
I can see both points. Giving the user a complete QColorDialog for each of the three colors may be much too complex (resulting in 3 * 2^32 = 12.9 billion possible settings!), but there certainly are some cases where you want to change the color (different map color scheme).
An approach would be to replace the three buttons with just a simple transparency spinbox (range? 0-100 percent or 0-255 value? Or is the alpha slider available as Qt widget?) and change the way the colors are stored in the config file from QRgb's 32bit UInt to a more human-readable (and editable) hexadecimal notation.
Additionally, there could be an "Advanced settings" button/tab where the user can switch from the simple transparency selection to the advanced color selection (without alpha, because this would still be controlled by the "transparency" setting).</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;">> Shall we have the patch merged first before we tweak it further so that Florian feels better?
What about splitting the patch in two? The first patch would turn the hardcoded colors in the routing layer into softcoded colors from the config files, and should cover both the KDE and the Qt version. In other words, this patch should be a refactoring (transparent to the user) that just makes our code more flexible. The second patch would - as a user-visible change - expose these settings via a GUI.</pre>
<br />
<p>- Bernhard</p>
<br />
<p>On October 26th, 2011, 9:41 a.m., Florian Eßer wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Marble.</div>
<div>By Florian Eßer.</div>
<p style="color: grey;"><i>Updated Oct. 26, 2011, 9:41 a.m.</i></p>
<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;">The patch from http://mail.kde.org/pipermail/marble-bugs/2011-September/001532.html
==============
Hi,
I had the same problem myself recently.
So here's a patch that adds an option "Adjust track color" in the
track's context menu just below "Export route".
Todo / Possible enhancements:
* make all the different route types (e.g. deviated from route)
configurable, not just the standard one. Right now, only the alpha
values get adjusted for all track types/colors.
* save/load to config file
** and then: button to restore the nice marble defaults if you have
experimented to much ;-)
Greetings
Florian
On 28.08.2011 09:48, Robin Seidel wrote:
> hi, thanks for the great marble!!! I have one wish: could you allow
> to adjust the transparency of the route overlay, so one can see the
> track beneath, this would be especially helpful for walking routes,
> where one may not want to take the smallest paths. Once the route is
> found one can currently not easily see what's beneath.
>
> best regards
>
> robin
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;">I haven't tested it it with the Qt-only version yet. (--> kcfg ??)</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>src/lib/routing/RoutingLayer.cpp <span style="color: grey">(77af021)</span></li>
<li>src/lib/routing/RoutingManager.h <span style="color: grey">(cbe70bf)</span></li>
<li>src/lib/routing/RoutingManager.cpp <span style="color: grey">(7cdffa1)</span></li>
<li>src/lib/routing/RoutingProfilesWidget.h <span style="color: grey">(e2d7333)</span></li>
<li>src/lib/routing/RoutingProfilesWidget.cpp <span style="color: grey">(88c880e)</span></li>
<li>src/lib/routing/RoutingSettingsWidget.ui <span style="color: grey">(27849c2)</span></li>
<li>src/marble.kcfg <span style="color: grey">(06277c8)</span></li>
<li>src/marble_part.cpp <span style="color: grey">(3f26b50)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102540/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/102540/s/318/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/10/26/screenshot_400x100.png" style="border: 1px black solid;" alt="Routing settings with new color buttons." /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>