<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/124989/">https://git.reviewboard.kde.org/r/124989/</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 30th, 2015, 5:08 a.m. UTC, <b>Dennis Nienhüser</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What performance impact does this have? It might affect rendering speed.
I would rather extend drawLabelPixmap to take another argument isSelected and then cache two pixmaps, one for the selected case and one for the not selected. Then return the right one based on the value of m_selected.</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;">(Indeed, this would affect rendering performance and what you suggested sounds right, but I don't see how it could work. The problem is that m_selected is not set properly because <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">VisiblePlacemark::setSelected(bool)</strong> is never called. If <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">m_selected</strong> stores whether a placemark has focus or not, I don't think it can ever be set right due to the fact that placemark selecting is handled in the AnnotatePlugin (which deals with SceneGrapicItems). It may be something that I don't understand right...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What is not working in the first place is that a <strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">PlacemarkTextAnnotation</strong>'s (which is a SceneGraphicItem) <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">paint()</em> method (which sets the GeoDataPlacemark's label color based on the focus) is called after all the VisiblePlacemarks are constructed (drawLabelPixmap() is called here) and then "collected" to be drawn on the map inside the PlacemarkLayer.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"><em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">problem++: </em></strong> I think that changing a GeoDataPlacemark's label style property to highlight it on the map is risky. If an export to .kml (or any other format supported) is called, the selected placemark would be saved with the highlighted color and not its original one.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">PS: if m_selected can be properly set, this issue can be easily fixed by handling the label highlight solely in the VisiblePlacemark class.</p></pre>
<br />
<p>- Constantin</p>
<br />
<p>On August 30th, 2015, 12:02 a.m. UTC, Constantin Mihalache 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 Marble.</div>
<div>By Constantin Mihalache.</div>
<p style="color: grey;"><i>Updated Aug. 30, 2015, 12:02 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
marble
</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;"><strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;"> bug: </strong>When a placemark is selected (left mouse click) and then deselected, it's label color does not change accordingly (it stays highlighted). When deselecting, the color changes to the original one only if the placemark is dragged out of the screen and then brought back.</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;">the main issue is that each VisiblePlacemark's labelPixmap should be generated exactly before painting.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">this may not be an elegant fix, but it works</li>
</ul></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Created placemarks -> clicked on the icon of the placemark -> label is highlighted -> clicked on the globe -> label is not highlighted.</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>src/lib/marble/VisiblePlacemark.h <span style="color: grey">(805112d)</span></li>
<li>src/lib/marble/VisiblePlacemark.cpp <span style="color: grey">(2fdc8ec)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124989/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>