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 Wind Turbines names, Change icon to gray, Do not render before z19 if location is rooftop #3515

Merged
merged 6 commits into from
Nov 24, 2018

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Nov 16, 2018

Fixes #2734
Also resolves #1017

Changes proposed in this pull request:

  • Start rendering names of wind turbines (power=generator, generator:source=wind)
  • Change wind turbine icon color to man-made-gray
  • Render wind turbines only at z19 if location=rooftop or =roof
  • Remove rendering for deprecated tagging power=generator with power_source=wind

Explanation:
Wind turbine name labels do not currently render, because the names for power=generator are done in landcover style without any text-dy. This PR adds text-dy: 10 for wind turbines, and the names will be rendered the same as other man_made features, starting at z17 (unless location=rooftop or roof)

The icon color is changed to match other man_made features, because it is currently black. This color is otherwise used only for religious feature icons.

Wind turbines on building rooftops are always much smaller than the large stand-alone towers common in rural areas, so they will now be rendered starting at z19. This will prevent them from blocking building names and the icons for amenities or shops in buildings at z17 and z18. The name label will also be rendered only at z19 in this case.

The tag power_source=wind is deprecated, and is now only used 200 times, vs 200,000 times for generator:source=wind, therefore it will be removed from rendering.

Test rendering with links to the example places:

Before
Blackfriars road z18 before
https://www.openstreetmap.org/#map=18/51.504541/-0.10496
blackfriars-z18

Islington Council building
https://www.openstreetmap.org/#map=18/51.54401/-0.10262
z18 before
islington-z18-before
z19 before
islington-wind-19

Strata Building
https://www.openstreetmap.org/#map=18/51.492857/-0.0995045
z18 before
strata-z18-before
z19 before
chase-wind-19

Prototype named wind turbine, before
https://www.openstreetmap.org/#map=17/53.5056716/8.5790005

Test rendering z19 before
z19-test-before

Test z18 before
z18-test-before

After
Blackfriars Road, London z18 (*Name added for test) - still renders because no location tag

AD 8-180 Prototype wind turbine, z17

Islington Council building, z18 - turbine hidden
islington-after-z18
z19 after
islington-wind-gray

Strata building, z18 - hidden
strata-after-z18
z19 after
chase-wind-grayicons

Prototype after
z17-prototyp-after

Test rendering z19

  • Named rooftop wind turbines show with label
    z19-test-after

Test z18
-rooftop wind turbines hidden, but others render with label.
-Power plants and other power generator labels still render properly as a node or polygon
z18-test-after

Wind turbines (power generator with method wind_turbine or source wind) will render at z19 if location is rooftop or roof, but continue to render at z15 otherwise. Also power_srouce=wind is removed as deprecated
Remove generator:method from wind turbines
@Adamant36
Copy link
Contributor

Does a "!=" clause not include a "null"? You'd think it would since "nothing" isn't equal to a roof, or if it was blank it just wouldnt use it as a rendering variable. I only ask because im still learning the code and think its interesting. Not because I think your wrong by doing that way or anything.

@Tomasz-W
Copy link

Tomasz-W commented Nov 16, 2018

I propose to change wind turbines icon with this occasion, current one is not that readable as it could be:

wind1 (current one)
wind2 (my proposal)

Gist link: https://gist.github.com/Tomasz-W/8fb5974a1acf6f9e55de3b718e3e3596

PS. This PR resolves also #1017 , so you can change opening post text before its ref to "fixes" or "resolves".

@boothym
Copy link
Contributor

boothym commented Nov 16, 2018

Question - what are valid wind turbine names?

Because I've looked at the ones with names in the UK, and seen the following:

Also the translated name for "Wind Turbine" i.e. Aerogenerador in Spanish.

Do these really need to be rendered? Or do we need to improve the data?

@Adamant36
Copy link
Contributor

Id say most of those aren't valid names. Id also imagine most wind turbines don't have legitimate names. Its not a problem specific to wind turbines though. For instance there's a bunch of parks named "park" out there. That being said, the best way to improve the data is to render it so people can see that its wrong and fix it. So I think its still worth rendering the names. Even if most of turn out to be junk.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 20, 2018

Re: name rendering: power generator names should currently be rendered, but because wind generators have an icon the name rendering is blocked by the icon at the same location. This PR will make wind generator names render as intended, just like they currently do for solar panels, small hydroelectric, and any other type of power generator.

I agree that most individual power generators do not have real names (usually the entire power plant is named, if there is a name), but hopefully the rendering will encourage mappers to remove incorrect names like "wind turbine" from the database.

Icon change proposal: both the old and new icon look fine to me, so I do not have a strong preference either way. I wish the 3 blades on the new icon were arranged evenly around the circle, but I think this was not possible due to the square shape of the canvas?

Test area with current icon z17:
windtest1-currenticon

Test with new icon proposal:
windtest1-newicon

London Blackfriars Road rooftop wind turbines current z18 (NOTE: I have added the "name" as "wind turbine"; this is not in the public database, fortunately)
wind-blackfriars-z18

Proposed icon:
wind-blackfriars-new-icon

German prototype current z17:
wind-protoptype-currenticon

Proposed icon:
wind-prototype-newicon

Dover wind turbine current z15:
wind-dover-z15-current

Proposed:
wind-dover-new-icon

@jeisenbe
Copy link
Collaborator Author

Any opinions on the new or old wind turbine icon?

@kocio-pl
Copy link
Collaborator

I prefer the new one, since it's more visible that it has 3 symmetric arms, old one is less clear with shape.

@Adamant36
Copy link
Contributor

Adamant36 commented Nov 22, 2018

I say the new one also. I like that it has a thing in the middle of the blades. It looks clearly that way.

@jeisenbe
Copy link
Collaborator Author

OK, I have pushed a commit with the new icon. I believe this is ready to merge

@kocio-pl
Copy link
Collaborator

Thanks, it looks OK in general, however symbols/power_wind.svg file should be removed and new icon file has too much rights (755 instead of 644).

There are also few more icons with 755 mode - it'd be good to make some cleaning, but in a separate PR.

Changed permissions for new icon
@kocio-pl
Copy link
Collaborator

Please change file rights too for symbols/generator_wind.svg, since it belongs to this change, unlike other icons.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 23, 2018 via email

@kocio-pl
Copy link
Collaborator

I don't see them on the GitHub, but I see 755 on my drive and there's really no trace of such change in your last commit: e99743d

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 23, 2018 via email

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Nov 23, 2018 via email

@kocio-pl
Copy link
Collaborator

I just wanted to ask you about it... 😄

@kocio-pl kocio-pl merged commit 8ba9c4a into gravitystorm:master Nov 24, 2018
@kocio-pl
Copy link
Collaborator

Thanks! It works OK now and I like new icon more.

@Tomasz-W Tomasz-W mentioned this pull request Nov 24, 2018
26 tasks
@jengelh
Copy link

jengelh commented Nov 24, 2018

Question - what are valid wind turbine names?

Whatever the owner/city administration decided it to be. Generally speaking, the name can be found on the owner's website (if such exists - not always a given), as a label on a transformer substation nearby (most reliable here in GER), or in news media or city council documents. A common name pattern for Germany is "WKA {locality} {number}", or "WKA {number}", implying the locality. If your UK wind turbines are called "Wind Turbine #1", I guess that's that. If it were just "Wind Turbine" though, I would be concerned. Names should have a disambiguating function, and "Wind Turbine" does not do that justice.

Because I've looked at the ones with names in the UK, and seen the following:
* wind farm names
* manufacturer names/codes (which are also tagged under manufacturer/manufacturer:type)
* a reference number (e.g. A142 or 56)
* Wind | Turbine | #1

All are valid (and different things). That "reference number" btw is generally the manufacturer's serial number.

@HolgerJeromin
Copy link
Contributor

A common name pattern for Germany is "WKA {locality} {number}", or "WKA {number}", implying the locality.

I would move this information to ref tag

@jengelh
Copy link

jengelh commented Nov 25, 2018

A common name pattern for Germany is "WKA {locality} {number}", or "WKA {number}", implying the locality.

I would move this information to ref tag

That would be a strange thing to do, similar to moving "Umspannwerk {locality}" from name to ref.

@jeisenbe jeisenbe deleted the windturbines-z19-rooftop branch November 25, 2018 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Names for wind turbines wind turbines: do not show minor ones at zooms 15/16/17
7 participants