Skip to content
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

Render building=roof differently to expose underlying features #2475

Open
BushmanK opened this issue Nov 28, 2016 · 51 comments
Open

Render building=roof differently to expose underlying features #2475

BushmanK opened this issue Nov 28, 2016 · 51 comments

Comments

@BushmanK
Copy link

BushmanK commented Nov 28, 2016

There are a couple of issues, logically (but not technically) related to this situation: #2037 and #688 , but this is a bit different thing.

Often, building=roof area covers something: a platform, a roadway, or even another building. Examples are: gas stations with offices or small stores, located under the roof, but not constructed as a single building with it; public transport platforms; covered galleries or passage ways.

Logically, building=roof should presume larger value of layer= than everything else under it. But using layer= on it explicitly seems redundant because of its nature, so it is actually rarely used. Another problem is, if (when) #688 will be resolved, it will be impossible to see objects hidden under the roofs.

Honestly, I don't know what kind of rendering to suggest, because transparency doesn't seem like a good idea (discussed in #688 ) while transparent patterns could look quite ugly and cause different issues. But I want to point at this situation anyway just in case if someone has a better idea.

@mboeringa
Copy link

mboeringa commented Nov 28, 2016

I have already shown this multiple times in the other threads, but just for the sake of keeping the discussion going, I post here two new examples of Amsterdam Centraal Station and Haarlem Station in the Netherlands, that show that a fine overlay pattern (non-transparent), can work for solving this issue.

Logically, building=roof should presume larger value of layer= than everything else under it. But using layer= on it explicitly seems redundant because of its nature, so it is actually rarely used.

And yes, I render this on top of everything else (at least above buildings), and don't require and ignore any layer=x values.

As rendered by my ArcGIS Renderer. Notice the fine grating above the platforms of the railway stations, representing the platform / station roofs. Click the images to see at 100%. To me personally, this solution feels very natural and understandable (which can not always be said of the use of transparency):

arcgis_renderer_for_openstreetmap-amsterdam_centraal_station
arcgis_renderer_for_openstreetmap-station_haarlem

@BushmanK
Copy link
Author

I agree, that crosshatching looks well as a part of this particular style. However, this kind of fine pattern has one fundamental problem: it causes moire (regular interference pattern, actually) when printed. I honestly don't know, if printability is among the considerations of developers.

Another thing is, function of the OSM Standard style is technical visualization. And it might make more sense to have similar color for all building= tags. Could you, please, render the same area using a bit tweaked style with CBBCBC color of building=roof hatching to make it similar to other buildings?

@kocio-pl
Copy link
Collaborator

I honestly don't know, if printability is among the considerations of developers.

No, it's not - we already cover wide set of goals and it's enough:
https://github.com/gravitystorm/openstreetmap-carto/blob/master/CARTOGRAPHY.md#main-goals

Brown hatching might be better. Maybe you could also prepare some code? I would be happy to test it. I also lack the roofs to be seen over other objects and this solution is acceptable for me.

@kocio-pl kocio-pl added this to the Bugs and improvements milestone Nov 29, 2016
@pnorman
Copy link
Collaborator

pnorman commented Nov 29, 2016

Logically, building=roof should presume larger value of layer= than everything else under it. But using layer= on it explicitly seems redundant because of its nature, so it is actually rarely used

Even if we assumed a layer for building=roof, it would not change how it renders relative to roads and other features, only relative to other buildings.

@BushmanK
Copy link
Author

@pnorman , as far as I understand what's going on with #688 and #2037, these issues, once resolved, will implement rendering behavior similar to assumed larger layer=.

@pnorman
Copy link
Collaborator

pnorman commented Nov 29, 2016

@pnorman , as far as I understand what's going on with #688 and #2037, these issues, once resolved, will implement rendering behavior similar to assumed larger layer=.

Those issues are both about changing where the buildings layer sits relative to other layers and have nothing to do with the layer tag.

@pnorman
Copy link
Collaborator

pnorman commented Nov 29, 2016

Also, it's worth noting that the resolution to those issues could be to do nothing. This is very common with layering issues where we're faced with tradeoffs and have to weigh one type of problem vs another

@BushmanK
Copy link
Author

BushmanK commented Nov 29, 2016

I'm talking about rendering behavior, not about layer= defaults. If the resolution will be to do nothing, than it's basically a closure, not a resolution in general meaning. So, that would be completely different story.

@pnorman
Copy link
Collaborator

pnorman commented Nov 29, 2016

It's not clear to me if you

  1. want to render buildings on top of amenities (gas stations, stores, etc) with some kind of hatching for building=roof
  2. want to render building=roof after other buildings
  3. want to move building=roof to its own layer and do something else with it

@BushmanK
Copy link
Author

In this issue, I'm originally talking about rendering building=roof in a certain manner that allows to see underlying features through it. Regarding of that, I like the idea brought by @mboeringa in his example renderings.

Here, I'm not insisting on any certain drawing order, just referring to it as described in other issues, because it seems logical to me (and I hope, it will be implemented that way). So, think about this issue as about one depending on others, requesting certain building drawing order changes. If buildings will not be rendered (at least) over the highway areas and platforms, it doesn't make any sense to change style for building=roof because it will not cover anything, so you don't need to be able to see through it.

Rendering roofs over amenities doesn't seem logical to me, so, I'm not talking about that at all.

@dieterdreist
Copy link

dieterdreist commented Nov 29, 2016 via email

@mboeringa
Copy link

On squares for example, but also more generally anytime there are roads below, you'd want to render the roof after the highway area, but before the ways atop of the roof.

@dieterdreist

What you describe is a bridge or viaduct, or parking deck of a building, not a roof.

A (building=)roof - at least IMO - is:

"A lightweight structure to keep the elements out"

not a load bearing structure designed to drive or walk on.

Just because a 5 storey car-parking building is open on all sides, doesn't make it a building=roof, you would most likely tag it an amenity=parking, parking=multi-storey, building=yes.

Admittedly, I have seen multiple examples of the kind of usage you describe, but I generally consider this a poor tagging practice, which doesn't seem to follow the initial main intend of the Wiki page of building=roof.

Also, as extension of the statement directly above, the Wiki (http://wiki.openstreetmap.org/wiki/Tag:building%3Droof) says on the first line:

"For roof features which are part of another building use building:part=roof"

@dieterdreist
Copy link

dieterdreist commented Nov 30, 2016 via email

@dieterdreist
Copy link

dieterdreist commented Nov 30, 2016 via email

@kocio-pl
Copy link
Collaborator

Is anybody willing to write this code? I would still like to test it, but have some other things to do currently.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Mar 8, 2018

@Penegal Would you like to test the brown hatching, maybe similar to #3104 or with crosshatching as proposed by @mboeringa?

@Penegal
Copy link
Contributor

Penegal commented Mar 9, 2018

@kocio-pl: just gave it a try using the same recipe than with intermittent waters, but here the transparent zones are rendered with a strange dark color:
image

I don't understand why; could someone tell me? The code is at https://github.com/Penegal/openstreetmap-carto/tree/roof

@dieterdreist
Copy link

dieterdreist commented Mar 9, 2018 via email

@Tomasz-W
Copy link

Tomasz-W commented Mar 9, 2018

I think it would be better to use a dashed outline and no fill for roofs.

We should check both ways to compare.

As with the water areas straight lines were good working, in buildings case I would like to see a 45-degree version too.

@kocio-pl
Copy link
Collaborator

kocio-pl commented Mar 9, 2018

In such cases the more testing, the better. Intermittent water area solution came after the proposed solution didn't work too well, we probably wouldn't hear about different one if we haven't failed.

@Penegal
Copy link
Contributor

Penegal commented Mar 9, 2018

Without filling:
image

With a half opacity filling:
image

With a half opacity filling plus dashed border:
image

With 45° stripes:
image
For this one, I don't understand: setting opacity to the fill colour correctly showed the background, but PNG transparency is filled with a dark grey. Ideas?

@Tomasz-W
Copy link

Tomasz-W commented Mar 9, 2018

Thank you for this work! I'm happy that someone started to do something more than commenting here ;)

Unfortunely, examples from above says not too much, because there is nothing under the roof. I think this feature should be tested on more complicated cases. Here's a list of locations which I would like to see (I don't know is it technically possible for you to do it, but if yes, it would be great):
https://www.openstreetmap.org/way/525860847
https://www.openstreetmap.org/way/261913276
https://www.openstreetmap.org/way/276725913
https://www.openstreetmap.org/way/497490695
https://www.openstreetmap.org/relation/6416391
https://www.openstreetmap.org/way/438175854

I think we can exclude 'without filling' version at the beginning - it's not intuitive and too similar to other feature, parking space (#3092)

@Penegal
Copy link
Contributor

Penegal commented Mar 11, 2018

Is there a problem with the Docker files? I got the following when trying to import data for @Tomasz-W's tests:

~/openstreetmap-carto (roof|✚3) $ docker-compose up import
openstreetmapcarto_db_1 is up-to-date
Recreating 3ab0106fcc1b_3ab0106fcc1b_3ab0106fcc1b_3ab0106fcc1b_openstreetmapcarto_import_1 ... error

ERROR: for 3ab0106fcc1b_3ab0106fcc1b_3ab0106fcc1b_3ab0106fcc1b_openstreetmapcarto_import_1  no such image: sha256:b78e2c0ea48b2f6ec0ca47950884322f90a2fbf44b36064e653e5c823a38470a: No such image: sha256:b78e2c0ea48b2f6ec0ca47950884322f90a2fbf44b36064e653e5c823a38470a

ERROR: for import  no such image: sha256:b78e2c0ea48b2f6ec0ca47950884322f90a2fbf44b36064e653e5c823a38470a: No such image: sha256:b78e2c0ea48b2f6ec0ca47950884322f90a2fbf44b36064e653e5c823a38470a
ERROR: Encountered errors while bringing up the project.

I tried to drop all images to force a full rebuild and re-dowloaded the data.osm.pbf, but that wasn't enough, the error is still here even with fresh containers and data. Do you guys know what the problem could be.

@kocio-pl
Copy link
Collaborator

"no such image" means probably that something is broken with the image files. I would try to delete the images, not just containers. I guess Docker should download missing images automatically, but maybe some parts of them were broken and maybe it works different then.

@polarbearing
Copy link
Contributor

Leaving transparency and render order aside:
The darks stripes are much too strong and give the roof too much importance.
The dashed outline works fine for me. It symbolises the roof as a building without walls.
Outlines without any fill cannot be distinguished from other lines like barriers.

@Tomasz-W
Copy link

Tomasz-W commented Mar 13, 2018

In both cases, it looks like the rendering order should be changed

I agree and I imagine it like that: move roof areas above landcovers, and make it striped (half-building color, half-transparent).
Dashed outline is not a solution for me, I think we should look for a ways to show roofs and what's under it, at the same time. As you can see in the test renderings above, with dashed outline only, you can just guess that is a roof by it's short visible parts

@Penegal
Copy link
Contributor

Penegal commented Mar 13, 2018

I could use some help to remove the dark stripes: they are not in the roof pattern I created but are introduced by another element instead of transparent stripes; perhaps a bug, but more likely by a fault in current rendering, maybe its ordering. Whatever, if someone understand what it could be, that would help me to correct it.

@matthijsmelissen
Copy link
Collaborator

It doesn't make sense to me either. Maybe you can debug by removing the layers from project.mml one by one?

@kocio-pl
Copy link
Collaborator

Could you publish the code of stripes and dashed lines?

@Penegal
Copy link
Contributor

Penegal commented Mar 28, 2018

Stripped version: Penegal@83e7cd7

Dashed outline version: Penegal@6101425

@polarbearing
Copy link
Contributor

@Penegal - please use the word 'striped' or 'hatched' since 'stripped' triggers my spam filter ;-)

@Tomasz-W - when we show what is under the roof (without walls) we should - with the same logic - show what is inside a building (roof with walls)?

@kocio-pl
Copy link
Collaborator

I've found that the problem is with polygon-clip variable - no matter if its value is false or true, it produces dark background. But when you remove it, it works as expected.

@talaj: I suspect that might be a Mapnik bug (tested with 3.0.20). What do you think?

polygon-clip=false/polygon-clip=true
bty fgcv

lack of polygon-clip
hwuudlj8

Styling part in XML looks like this:

and the diff shows no color changes by Carto CSS:

$ diff project.xml.false project.xml.none 
10371d10370
<     <PolygonSymbolizer clip="false" />
10380d10378
<     <PolygonSymbolizer clip="false" />
10385d10382
<     <PolygonSymbolizer fill="#d9d0c9" clip="false" />
10386a10384
>     <PolygonSymbolizer fill="#d9d0c9" />
10392c10390
<     <PolygonSymbolizer fill="#d1c6bd" clip="false" />
---
>     <PolygonSymbolizer fill="#d1c6bd" />
10396d10393
<     <PolygonSymbolizer clip="false" />
10398,10402d10394
<   </Rule>
<   <Rule>
<     <MaxScaleDenominator>100000</MaxScaleDenominator>
<     <MinScaleDenominator>25000</MinScaleDenominator>
<     <PolygonSymbolizer clip="false" />

@kocio-pl
Copy link
Collaborator

Similar problem has been observed in 2013: #93 (comment), however back then the fill color was not specified. I don't know why clip touches colors, on the other hand this option is not even documented:

https://github.com/mapnik/mapnik/wiki/PolygonSymbolizer

@Penegal
Copy link
Contributor

Penegal commented May 15, 2018

@kocio-pl: Glad you found out the issue! I like your striped version.

@Tomasz-W
Copy link

Tomasz-W commented May 15, 2018

@kocio-pl The output of discussion was to render roofs as stripped areas above landcovers. It would be way more intuitive on map (and harder to confuse with something placed underground). Can you make a test renderings with building=roof moved above landcovers? It would be also nice to see versions with wider stripes and 45 degree stripes to compare them and choose the best solution.

@talaj
Copy link

talaj commented May 15, 2018

@kocio-pl, why that diff is showing <PolygonSymbolizer clip="false" />? This will render polygon with default fill, which is RGB(128, 128, 128). I think this is where the dark color comes from.

I think I know where is the problem. You should use polygon-pattern-clip instead of polygon-clip.

@kocio-pl
Copy link
Collaborator

Can you make a test renderings with building=roof moved above landcovers?

Sure, just give me example places to see it better. However you can spot it on residential, forest or grass.

It would be also nice to see versions with wider stripes and 45 degree stripes to compare them and choose the best solution.

It's easy to test it now too - one can just remove clipping option as a temporary fix.

@Tomasz-W
Copy link

Tomasz-W commented May 15, 2018

Sure, just give me example places to see it better

Places showed above are enough for those tests, I think. What I mean is that we should use this stripped-transparency also for highway areas and platforms (landcover features). When a roof is under them, it looks more like some underground object.

@kocio-pl
Copy link
Collaborator

@talaj I think that it's at best inconsistent and confusing that it has some default value for clipping and then it works good, without adding gray filling, but when clipping is explicitly mentioned, it adds some filling, no matter if the clipping is true or false.

I consider it a bug to behave differently with default value than when this value is set by user. I also think that adding color when clipping does not require it (I believe it's geometrical feature) makes no sense.

@talaj
Copy link

talaj commented May 16, 2018

@kocio-pl, it's how CartoCSS works and it's the same case as in mapnik/mapnik#3813 (comment).

The following CartoCSS snippet

polygon-pattern-file: url('symbols/building_roof_dark.png');
polygon-clip: false;

generates two symbolizers, something like this:

<PolygonPatternSymbolizer file="symbols/building_roof_dark.png" />
<PolygonSymbolizer clip="false" />

@kocio-pl
Copy link
Collaborator

I see now... How should it look like to define clipping off with pattern and without adding gray?

@talaj
Copy link

talaj commented May 16, 2018

@kocio-pl

polygon-pattern-file: url('symbols/building_roof_dark.png');
polygon-pattern-clip: false;

@Tomasz-W
Copy link

@jragusa Can you provide test renderings of some of these locations with both hatching/ opacity versions to compare which one would fit better roofs and which one underground buildings?

https://www.openstreetmap.org/way/525860847
https://www.openstreetmap.org/way/261913276
https://www.openstreetmap.org/way/276725913
https://www.openstreetmap.org/way/497490695
https://www.openstreetmap.org/relation/6416391
https://www.openstreetmap.org/way/438175854

PS. Are you able and interested in writing PR for #688?

@jragusa
Copy link
Contributor

jragusa commented Dec 13, 2018

@Tomasz-W we need first to resolve #688 to test correctly rendering so I will work on it and once merged, we can come back to this issue.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Sep 1, 2020

Also see #4164 requesting different rendering for building=construction

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests