-
Notifications
You must be signed in to change notification settings - Fork 820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resolve the landcover ordering problem #15
Comments
This gets my vote. A big explanatory comment at the top of the file would help deal with any confusion. The additional down side for this though is potential for information loss - eg "military=airfield" is not obvious as "feature=airfield". But of the values actually used in landcover.mss I don't really see any that would become ambiguous when coalesced to "feature" except for perhaps "military=danger_area". It might be possible to do some SQL trickery to concatenate tag and value (eg "feature=military_airfield") - not sure what kind of performance cost that would incur. |
I've been pondering this and realised one big pitfall. Given that this is aimed to be the main style on osm.org, and for a majority of people "does it render" is a big part of their editing feedback loop, we need to be careful not to encourage incorrect tagging. A straight coalesce of values into the feature column would mean that landuse=wood, tourism=wood, natural=wood, leisure=wood, amenity=wood etc would all render with the same results. This will cause widespread confusion. I'm therefore drawn towards the concatenate-plus-coalesce suggestion. The following approach seems to do the trick. Does anyone have any comments on performance implications of needing to use the subselect approach, or some sql trickery to get the coalesce to work on the manipulated column values in just one query?
|
Good point about the confusion with rendering. A similar issue with the straight coalesce approach is that a less common tag combination can block the rendering of a more common one. For example I saw a leisure=golf_course not being rendered because it was also tagged amenity=country_club, and amenity was earlier in the coalesce function. |
I guess the "con-plus-coal" approach still has that problem, since you'll only end up with one tag ( I don't mind if two valid features could be matched (e.g. natural=wood, tourism=attraction) since we're dealing with polygon fills, and so only one colour could be shown in any case. But I don't want to find a rare and un-rendered tag (e.g. amenity=country_club) overriding a common and explicitly rendered one (leisure=golf_course). I don't have any great ideas on how to work around this. One potential could be
... in order to 'blank out' these undesired tags, but this is starting to set off excessive-complexity alarm bells! |
see my comment here - a short term workaround could be to actually use the exact style for landcover from the old XML. This should work fine since the layer is identical. |
Any sense of the performance impact of this change? |
I'm not certain on the performance. The core query (the where clause) is still identical to the original xml stylesheet version, so the data volume (in terms of number of features fetched) should be the same. Moving away from attachments should speed things up, as you detailed at #20. But I've no idea how the negative impact of all the attribute concatenation and coalescing, and the subselect, compares with the positive impact of moving away from attachments. A further improvement may be to copy the lists of known values to the where clauses, because we still have 'where landuse is not null' - so long as the benefits of selecting fewer polygons outweigh the dis-benefits of postgres doing a bucketload of "where landuse in (x, y, z)" tests. The overall aim of the commit was to fix the order-by-size problem, so I'd be willing to trade a small amount of performance for the more accurate rendering. But perhaps I should set up some benchmarking, rather than guessing! |
As a workaround for mapbox/carto#20 all the entries in landcover.mss have key-based attachments. This breaks the order-by-area clause, and also leaves the output of multiple tagging (e.g. #6 ) susceptible to changes in refactoring[1]
A different approach is needed, to make overlapping areas more likely to be drawn as intended (i.e. smallest on top), while preserving the clarity of the mss. Two choices spring to mind:
[1] since attachment draw order is based on the order its first encountered, adding or removing a rule that happens to act as the first declaration of the attachment can change the order of layers entirely.
The text was updated successfully, but these errors were encountered: