-
Notifications
You must be signed in to change notification settings - Fork 821
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
Add a building is null condition to the landcover layer #217
Conversation
I'm still pondering this over. There's one minor flaw in the logic, which is that A slightly bigger problem is when someone takes the stylesheets, and wants to turn off buildings. Then lots of amenities won't show up. I think the main problem is that what we want to do has nothing to do with buildings per se - it's just that we want a faster way to retrieve the polygons we're interested in, and it's a combination of
It's not so much that I'm still pondering what the best approach is. |
Well, the partial index reflects the queries, including the existing queries on water-areas-overlay, glaciers-text, landuse-overlay, national-park-boundaries.
True, although these are amenities that don't currently show up as amenities, only as buildings. Right now if they take the stylesheet and turn off buildings, they get additional features (any amenity that was also a building) showing up. I'm not sure if features appearing when you disable buildings or features disappearing when you disable buildings is worse. In the particular case I used as an example, I'd say that it's a tagging error and the school needs to be a bigger polygon, not just the building.
Hmm - this is also an issue for partitioning, although maybe a partial index |
This would also not render those 1% of landuses which have a building tag http://taginfo.openstreetmap.org/keys/landuse#combinations |
@pnorman
and
So is it supposed to change rendering or not? |
closed, with #565 open |
Adds a
building IS NULL
to the landcover SQL.The motivation is that this allows substantial performance increases with the right indexes and that
landcover
is not buildings, as well as fixing some subtle styling oddities.The performance gain is at least +3.9% over f5ef478 with the greatest gains shown at high zooms with +5.5% at z18 and +2.9% at z13. A partial index
WHERE building IS NULL
as described in #207 (comment) was present in all cases.Buildings are drawn on top of this layer, so there isn't a rendering impact.
Details on why this doesn't impact rendering
The old SQL filtering condition is
This pull just adds
AND building IS NULL
to it.Additionally, the
CASE
statement in the columns effectively restrictsleisure
to'swimming_pool', 'playground', 'park', 'recreation_ground', 'common', 'garden', 'golf_course'
andlanduse
toI was initially concerned that adding
building IS NULL
would impact rendering of features likeamenity=school
, but this is not so.It turns out that if there is a building tag, the building is rendered on top of the landuse layer, obscuring it, giving it only a very minor rendering effect around the edges due to anti-aliasing.
As an example, see these two buildings, one
building=yes
amenity=school
and one a normalbuilding=yes
There is a difference but it is extremely subtle. The building fill is also very subtly different.
The same thing will apply to any building - the building polygon will be drawn on top of the landcover polygon