[Marble-devel] Review Request 123181: Added Right placement for labels and placemark sorting

[Adam Dabrowski]

> On April 2, 2015, 4:25 p.m., Torsten Rahn wrote:
> > src/lib/marble/PlacemarkLayout.cpp, line 452
> > <https://git.reviewboard.kde.org/r/123181/diff/2/?file=359596#file359596line452>
> >
> >     What criterion is this sorted by?
> 
> Dennis Nienhüser wrote:
>     Just the pointer value I'd say. AFAIU the point is to have it stable wrt some viewport, not necessarily stable on a second visit.
> 
> Torsten Rahn wrote:
>     But in general the placemark rendering is sorted by popularity ...!?!?!? If not then this doesn't look right ...
> 
> Adam Dabrowski wrote:
>     Well it's better than no sorting at all. As Dennis points out, the idea for it is to be a stable order for a particular view - otherwise placemark labels "flicker" as label placement is decided based on previously computed label positions and these were pseudorandom.
>     
>     I would call this (pointer) sort an improvement.
>     
>     There can be some other kind of sorting implemented here but we have to agree on what the algorithm is (popularity, then pointer value?)
> 
> Torsten Rahn wrote:
>     As discussed with Adam on IRC the observed issue might be due to unstable sorting in case placemarks share the same popularity. This could be solved by adding a second sorting criterion (e.g. pointer address to approximate creation order) after sorting by popularity.

Moved sorting issue out of this patch. It deserves a separate patch. As far as I am concerned this one is good to go..


- Adam


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


On April 10, 2015, 7:48 a.m., Adam Dabrowski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123181/
> -----------------------------------------------------------
> 
> (Updated April 10, 2015, 7:48 a.m.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> -Added Right position for placemark labels. Up to 7 positions are available for labels if placemarks overlap.
> -Added sorting of placemarks before layouting. Without this there would be labels "jumping" around as several moving placemarks overlap (now they are drawn in the same order and there is no jumping). Cost of sorting is miniscule compared to other operations performed when layouting.
> -Refactored code for checking if layout space is already occupied
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/PlacemarkLayout.cpp 6c0af30 
>   src/lib/marble/geodata/data/GeoDataLabelStyle.h 046f7bc 
>   src/lib/marble/geodata/graphicsitem/GeoLineStringGraphicsItem.cpp 5949c7d 
> 
> Diff: https://git.reviewboard.kde.org/r/123181/diff/
> 
> 
> Testing
> -------
> 
> Displayed 1, 2, 3 and 7+ items.
> Checked for no flickering.
> 
> 
> Thanks,
> 
> Adam Dabrowski
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20150410/5d46054d/attachment.html>