Modify

Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#21068 closed enhancement (fixed)

[Patch] Flat roofs with roof:height

Reported by: Friendly_Ghost Owned by: team
Priority: normal Milestone: 21.06
Component: Core validator Version:
Keywords: roof height Cc: Klumbumbus

Description

I found that nearly 7,000 ways in the DB are tagged with both roof:height=* and roof:shape=flat. Some have the redundant value roof:height=0, but many have other values, even negative ones. This has already caused https://demo.f4map.com/ to crash when looking at the city of New York. Could this combination of tags be flagged as an error?

Attachments (1)

josm_21068.patch (5.3 KB) - added by skyper 12 months ago.
patch adding "roof:height=*" in general and a rule for "roof:shape=flat" with "roof:height=0"

Download all attachments as: .zip

Change History (13)

comment:1 Changed 12 months ago by skyper

I first thought this is easy but thinking about it again, how do you tell the height of a roof if not using roof:height=*?. A height of zero or negative values is non-sense but usually even a flat roof has a certain height of some centimeters. Looking at e.g. garages, I can understand why people add negative values as the outer walls might be higher then the actual roof but I can live with a "normal" warning for zero or negative values.

If f4maps crashes the code of this software needs to be adjusted.

Last edited 12 months ago by skyper (previous) (diff)

comment:2 Changed 12 months ago by Friendly_Ghost

The height of a roof relative to the walls of a building is more like a vertical offset than a true height. Unless we're mapping the thickness of the roof, which I assume is included in the height=* tag and the height of building levels, the true height of a supposedly flat surface (roof:shape=flat) should be assumed to be zero.

comment:3 Changed 12 months ago by skyper

Quoting the wiki:

roof:height
The height of the building (i.e. the height of the façade) is calculated as the building's total height=* minus roof:height=*.

So, yes, even a flat roof can have a small roof:height.

comment:4 Changed 12 months ago by Friendly_Ghost

If that's how it is, then I can also live with a warning for zero or negative values.

comment:5 Changed 12 months ago by skyper

Owner: changed from team to skyper

Changed 12 months ago by skyper

Attachment: josm_21068.patch added

patch adding "roof:height=*" in general and a rule for "roof:shape=flat" with "roof:height=0"

comment:6 Changed 12 months ago by skyper

Keywords: height added
Summary: Flat roofs with roof:height[Patch] Flat roofs with roof:height

josm_21068.patch adds roof:height=* in general to numeric.mapcss and an additional rule for roof:shape=flat + roof:height=0.

comment:7 Changed 12 months ago by skyper

Cc: Klumbumbus added
Milestone: 21.06

comment:8 Changed 12 months ago by skyper

Owner: changed from skyper to team

comment:9 Changed 12 months ago by Klumbumbus

Resolution: fixed
Status: newclosed

In 17953/josm:

fix #21068 - Add validator rules for roof:height (patch by skyper)

comment:10 Changed 12 months ago by Klumbumbus

(I fixed one class (.building_roof_separator_autofix) and adjusted the intendation for better comparison of the regexes)

comment:11 Changed 12 months ago by Klumbumbus

In 17954/josm:

see #21068 - Revert accidentally commited file

comment:12 in reply to:  10 Changed 12 months ago by skyper

Replying to Klumbumbus:

(I fixed one class (.building_roof_separator_autofix) and adjusted the intendation for better comparison of the regexes)

Thanks.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.