Implemented outlines for GeoLineStringGraphicsItems.
Review Request #124074 - Created June 12, 2015 and discarded
This patch adds support for outlines on
GeoLineStringGraphicsItems, which represent the roads, streets, paths and ways on the OSM vector tiles. The main idea is to draw behind every
GeoLineStringGraphicsItemthe same line(implemented a
GeoLineStringGraphicsItem::copyAsOutline()function for this purpose) with the pen color(outline color) and the original line on top of that with the brush color(fill color) and a little thinner.
Painting these outlines have a small impact on the performance: cca. 10 ms(on my laptop) on a large map, which has 2000+ lines. Because of that, this feature can be disabled if map quality is set to
On the other hand, I've added some new features to
HighwayFootway, which are now rendered correctly on OSM vector tiles.
The performance impact is acceptable, the result is ok and works for me.
Great patch - but still needs quite some refinement imho ... :-)
why 0.01 and not 0.001? What are possible problems with regard to messing with the zValue directly (which is also accessible to the API user) as opposed to possibly introducing an "internalZValue" property that still maintains the same actual zValue for both the outline as well as the actual line from the outside?
If we keep it like this then we should at least make 0.01 a variable.
I don't get the intention behind this. In the Texture case setNeedsUpdate basically means that the previously rendered texture part of the viewport (which is cached as a pixmap) can not be used anymore and hence the whole viewport texture needs to be recalculated (instead of just painting the cached pixmap of the texture).
This doesn't seem to be the case here. Introducing such a method might make sense if we cache "artificial items" or cache "(p)resorted" data. But this is not the case here - here it just seems to call repaintNeeded(). repaintNeeded() on the other side is just about a "weaker" repaint that still keeps cached data intact. So introducing this method like this doesn't look right to me.
This happens on every single redraw. I'm quite sure this does have a performance impact. Any numbers for the expected performance impact e.g. on somewhat older hardware (think android ...)? What about caching these?
This gets sorted on every single repaint. How long does this take for a bigger OSM file on slow hardware? Would it make sense to cache this somewhere?
Please use qDeleteAll() ...
- Removed the
s_outlineZValuevariable instead the "magic number"
Patch looks pretty good to me. I wonder how well this enhancements "scales" in terms of API possible improvements with regard to code sharing and performance:
Could you create a prototype patch (or a real one) that adds the following feature:
For creating the Fake3D effect that can be seen in
http://www.openstreetmap.org/relation/1244004#map=18/47.50438/19.05058&layers=TN (ignore the blur there and just assume a shifted copy with a different style)
we would need a pretty similar technique that includes creating a "copy" with a positional offset and a different style (no pen being used for the "groundfloor shape" (better name?) item and one that uses a pen and a different brush for the "roof" item).
Which effect would this have on the public API? Should we then add another set of "copyAsFake3D()" and "isFake3D" or should we have a generic copyAsDecoration(flag), where flag could get values like "Fake3D", "Outline"?