ask about barrier on intersection of barrier line and path [ready for review, again]

app/src/main/java/de/westnordost/streetcomplete/quests/QuestModule.kt

@@ -58,8 +58,7 @@ import de.westnordost.streetcomplete.quests.oneway_suspects.data.WayTrafficFlowD
 import de.westnordost.streetcomplete.quests.opening_hours.AddOpeningHours
 import de.westnordost.streetcomplete.quests.atm_operator.AddAtmOperator
 import de.westnordost.streetcomplete.quests.barrier_bicycle_barrier_type.AddBicycleBarrierType
-import de.westnordost.streetcomplete.quests.barrier_type.AddBarrierType
-import de.westnordost.streetcomplete.quests.barrier_type.AddStileType
+import de.westnordost.streetcomplete.quests.barrier_type.*
 import de.westnordost.streetcomplete.quests.traffic_calming_type.AddTrafficCalmingType
 import de.westnordost.streetcomplete.quests.bollard_type.AddBollardType
 import de.westnordost.streetcomplete.quests.bus_stop_bin.AddBinStatusOnBusStop
@@ -253,6 +252,8 @@ import javax.inject.Singleton
         AddBoardType(),
 
         AddBarrierType(), // basically any more detailed rendering and routing: OSM Carto, mapy.cz, OSMand for start
+        AddBarrierOnPath(),
+        AddBarrierOnRoad(),
         AddStileType(),
         AddBicycleBarrierType(),
 

app/src/main/java/de/westnordost/streetcomplete/quests/barrier_type/AddBarrierOnPath.kt

@@ -0,0 +1,45 @@
+package de.westnordost.streetcomplete.quests.barrier_type
+
+import de.westnordost.streetcomplete.R
+import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
+import de.westnordost.streetcomplete.data.meta.ALL_PATHS
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChangesBuilder
+import de.westnordost.streetcomplete.data.osm.mapdata.Element
+import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
+import de.westnordost.streetcomplete.data.osm.mapdata.Node
+import de.westnordost.streetcomplete.data.osm.osmquests.OsmElementQuestType
+import de.westnordost.streetcomplete.data.user.achievements.QuestTypeAchievement.PEDESTRIAN
+import de.westnordost.streetcomplete.data.user.achievements.QuestTypeAchievement.WHEELCHAIR
+import de.westnordost.streetcomplete.data.user.achievements.QuestTypeAchievement.OUTDOORS
+
+class AddBarrierOnPath: OsmElementQuestType<BarrierType> {
+
+    private val barrierFilter by lazy { """
+        ways with
+          barrier ~ wall|fence|hedge|guard_rail|retaining_wall|city_wall
+          and area != yes
+    """.toElementFilterExpression() }
+
+    private val pathsFilter by lazy { """
+        ways with
+          (highway ~ ${ALL_PATHS.joinToString("|")} and area != yes)
+          and (access !~ private|no or (foot and foot !~ private|no))
+    """.toElementFilterExpression() }
+
+    override val commitMessage = "Add how path and barrier intersect"
+    override val wikiLink = "Key:barrier"
+    override val icon = R.drawable.ic_quest_barrier_on_path
+    override val questTypeAchievements = listOf(PEDESTRIAN, WHEELCHAIR, OUTDOORS)
+
+    override fun getTitle(tags: Map<String, String>) = R.string.quest_barrier_path_intersection
+
+    override fun getApplicableElements(mapData: MapDataWithGeometry): Iterable<Element> =
+        detectWayBarrierIntersection(mapData, barrierFilter, pathsFilter)
+
+    override fun isApplicableTo(element: Element): Boolean? =
+        if (element !is Node || element.tags.isNotEmpty()) false else null
+
+    override fun createForm() = AddBarrierTypeForm()
+
+    override fun applyAnswerTo(answer: BarrierType, changes: StringMapChangesBuilder) = answer.applyTo(changes)
+}

app/src/main/java/de/westnordost/streetcomplete/quests/barrier_type/AddBarrierOnRoad.kt

@@ -0,0 +1,43 @@
+package de.westnordost.streetcomplete.quests.barrier_type
+
+import de.westnordost.streetcomplete.R
+import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
+import de.westnordost.streetcomplete.data.meta.ALL_ROADS
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChangesBuilder
+import de.westnordost.streetcomplete.data.osm.mapdata.Element
+import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
+import de.westnordost.streetcomplete.data.osm.mapdata.Node
+import de.westnordost.streetcomplete.data.osm.osmquests.OsmElementQuestType
+import de.westnordost.streetcomplete.data.user.achievements.QuestTypeAchievement.CAR
+
+class AddBarrierOnRoad: OsmElementQuestType<BarrierType> {
+
+    private val barrierFilter by lazy { """
+        ways with
+          barrier ~ wall|fence|hedge|guard_rail|retaining_wall|city_wall
+          and area != yes
+    """.toElementFilterExpression() }
+
+    private val pathsFilter by lazy { """
+        ways with
+          (highway ~ ${ALL_ROADS.joinToString("|")} and area != yes)
+          and (access !~ private|no or (foot and foot !~ private|no))
+    """.toElementFilterExpression() }
+
+    override val commitMessage = "Add how road and barrier intersect"
+    override val wikiLink = "Key:barrier"
+    override val icon = R.drawable.ic_quest_barrier_on_road
+    override val questTypeAchievements = listOf(CAR)
+
+    override fun getTitle(tags: Map<String, String>) = R.string.quest_barrier_road_intersection
+
+    override fun getApplicableElements(mapData: MapDataWithGeometry): Iterable<Element> =
+        detectWayBarrierIntersection(mapData, barrierFilter, pathsFilter)
+
+    override fun isApplicableTo(element: Element): Boolean? =
+        if (element !is Node || element.tags.isNotEmpty()) false else null
+
+    override fun createForm() = AddBarrierTypeForm()
+
+    override fun applyAnswerTo(answer: BarrierType, changes: StringMapChangesBuilder) = answer.applyTo(changes)
+}

app/src/main/java/de/westnordost/streetcomplete/quests/barrier_type/AddBarrierType.kt

@@ -34,24 +34,5 @@ class AddBarrierType : OsmFilterQuestType<BarrierType>() {
 
     override fun createForm() = AddBarrierTypeForm()
 
-    override fun applyAnswerTo(answer: BarrierType, changes: StringMapChangesBuilder) {
-        changes.modify("barrier", answer.osmValue)
-        when (answer) {
-            BarrierType.STILE_SQUEEZER -> {
-                changes.addOrModify("stile", "squeezer")
-            }
-            BarrierType.STILE_LADDER -> {
-                changes.addOrModify("stile", "ladder")
-            }
-            BarrierType.STILE_STEPOVER_WOODEN -> {
-                changes.addOrModify("stile", "stepover")
-                changes.addOrModify("material", "wood")
-            }
-            BarrierType.STILE_STEPOVER_STONE -> {
-                changes.addOrModify("stile", "stepover")
-                changes.addOrModify("material", "stone")
-            }
-        }
-
-    }
+    override fun applyAnswerTo(answer: BarrierType, changes: StringMapChangesBuilder) = answer.applyTo(changes)
 }

app/src/main/java/de/westnordost/streetcomplete/quests/barrier_type/BarrierType.kt

@@ -1,5 +1,7 @@
 package de.westnordost.streetcomplete.quests.barrier_type
 
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChangesBuilder
+
 enum class BarrierType(val osmValue: String) {
     PASSAGE("entrance"),
     GATE("gate"),
@@ -25,3 +27,23 @@ enum class BarrierType(val osmValue: String) {
     KISSING_GATE("kissing_gate"),
     BICYCLE_BARRIER("cycle_barrier")
 }
+
+fun BarrierType.applyTo(changes: StringMapChangesBuilder) {
+    changes.addOrModify("barrier", this.osmValue)
+    when (this) {
+        BarrierType.STILE_SQUEEZER -> {
+            changes.addOrModify("stile", "squeezer")
+        }
+        BarrierType.STILE_LADDER -> {
+            changes.addOrModify("stile", "ladder")
+        }
+        BarrierType.STILE_STEPOVER_WOODEN -> {
+            changes.addOrModify("stile", "stepover")
+            changes.addOrModify("material", "wood")
+        }
+        BarrierType.STILE_STEPOVER_STONE -> {
+            changes.addOrModify("stile", "stepover")
+            changes.addOrModify("material", "stone")
+        }
+    }
+}

app/src/main/java/de/westnordost/streetcomplete/quests/barrier_type/DetectWayBarrierIntersectionUtil.kt

@@ -0,0 +1,30 @@
+package de.westnordost.streetcomplete.quests.barrier_type
+
+import de.westnordost.streetcomplete.data.elementfilter.ElementFilterExpression
+import de.westnordost.streetcomplete.data.osm.mapdata.*
+import de.westnordost.streetcomplete.quests.findNodesAtCrossingsOf
+
+fun detectWayBarrierIntersection(mapData: MapDataWithGeometry, barrierFilter: ElementFilterExpression, pathsFilter: ElementFilterExpression): Iterable<Element> {
+    val barrierWays = mapData.ways.asSequence()
+        .filter { barrierFilter.matches(it) }
+
+    val movingWays = mapData.ways.asSequence()
+        .filter { pathsFilter.matches(it) }
+
+    /* skip all cases involving tunnels, as it is typical to map retaining wall as
+     * intersecting path, what is even kind of correct
+     *
+     * e.g. https://www.openstreetmap.org/node/5074693713
+     *
+     * For bridges it is more dubious but also in active use, see
+     * https://www.openstreetmap.org/node/78135912 + https://www.openstreetmap.org/way/417857886
+     * https://www.openstreetmap.org/node/308681095 + https://www.openstreetmap.org/way/558083525
+     */
+    val crossings = findNodesAtCrossingsOf(barrierWays, movingWays, mapData).filter {
+        it.movingWays.all { way ->
+            (!way.tags.containsKey("tunnel") || way.tags["tunnel"] == "no") &&
+            (!way.tags.containsKey("bridge") || way.tags["bridge"] == "no")
+        }
+    }
+    return crossings.map { it.node }.filter { it.tags.isEmpty() }
+}

app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossing.kt

@@ -7,6 +7,7 @@ import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapChanges
 import de.westnordost.streetcomplete.data.osm.mapdata.*
 import de.westnordost.streetcomplete.data.osm.osmquests.OsmElementQuestType
 import de.westnordost.streetcomplete.data.user.achievements.QuestTypeAchievement.PEDESTRIAN
+import de.westnordost.streetcomplete.quests.findNodesAtCrossingsOf
 import de.westnordost.streetcomplete.quests.kerb_height.AddKerbHeightForm
 import de.westnordost.streetcomplete.quests.kerb_height.KerbHeight
 import de.westnordost.streetcomplete.util.isRightOf
@@ -45,95 +46,26 @@ class AddCrossing : OsmElementQuestType<KerbHeight> {
         if(element !is Node || element.tags.isNotEmpty()) false else null
 
     override fun getApplicableElements(mapData: MapDataWithGeometry): Iterable<Element> {
-
-        val roadsByNodeId = mapData.ways.asSequence()
+        val barrierWays = mapData.ways.asSequence()
             .filter { roadsFilter.matches(it) }
-            .groupByNodeIds()
-        /* filter out nodes of roads that are the end of a road network (dead end), e.g.
-         * https://www.openstreetmap.org/node/280046349 or
-         * https://www.openstreetmap.org/node/56606744 */
-        roadsByNodeId.removeEndNodes()
+
+        val movingWays = mapData.ways.asSequence()
+            .filter { footwaysFilter.matches(it) }
+
+        var crossings = findNodesAtCrossingsOf(barrierWays, movingWays, mapData)
 
         /* require all roads at a shared node to either have no sidewalk tagging or all of them to
          * have sidewalk tagging: If the sidewalk tagging changes at that point, it may be an
          * indicator that this is the transition point between separate sidewalk mapping and
          * sidewalk mapping on road-way. F.e.:
          * https://www.openstreetmap.org/node/1839120490 */
         val anySidewalk = setOf("both","left","right")
-        roadsByNodeId.values.removeAll { ways ->
-            if (ways.any { it.tags["sidewalk"] in anySidewalk }) {
-                !ways.all { it.tags["sidewalk"] in anySidewalk }
-            } else {
-                false
-            }
-        }
 
-        val footwaysByNodeId = mapData.ways.asSequence()
-            .filter { footwaysFilter.matches(it) }
-            .groupByNodeIds()
-        /* filter out nodes of footways that are the end of a footway, e.g.
-         * https://www.openstreetmap.org/node/1449039062 or
-         * https://www.openstreetmap.org/node/56606744 */
-        footwaysByNodeId.removeEndNodes()
-
-        /* filter out all nodes that are not shared nodes of both a road and a footway */
-        roadsByNodeId.keys.retainAll(footwaysByNodeId.keys)
-        footwaysByNodeId.keys.retainAll(roadsByNodeId.keys)
-
-        /* finally, filter out all shared nodes where the footway(s) do not actually cross the road(s).
-        *  There are two situations which both need to be handled:
-        *
-        *  1. The shared node is contained in a road way and a footway way and it is not an end
-        *     node of any of the involved ways, e.g.
-        *     https://www.openstreetmap.org/node/8418974983
-        *
-        *  2. The road way or the footway way or both actually end on the shared node but are
-        *     connected to another footway / road way which continues the way after
-        *     https://www.openstreetmap.org/node/1641565064
-        *
-        *  So, for the algorithm, it should be irrelevant to which way(s) the segments around the
-        *  shared node belong, what count are the positions / angles.
-        */
-        footwaysByNodeId.entries.retainAll { (nodeId, footways) ->
-
-            val roads = roadsByNodeId.getValue(nodeId)
-            val neighbouringRoadPositions = roads
-                .flatMap { it.getNodeIdsNeighbouringNodeId(nodeId) }
-                .mapNotNull { mapData.getNode(it)?.position }
-
-            val neighbouringFootwayPositions = footways
-                .flatMap { it.getNodeIdsNeighbouringNodeId(nodeId) }
-                .mapNotNull { mapData.getNode(it)?.position }
-
-            /* So, surrounding the shared node X, in the simple case, we have
-             *
-             * 1. position A, B neighbouring the shared node position which are part of a road way
-             * 2. position P, Q neighbouring the shared node position which are part of the footway
-             *
-             * The footway crosses the road if P is on one side of the polyline spanned by A,X,B and
-             * Q is on the other side.
-             *
-             * How can a footway that has a shared node with a road not cross the latter?
-             * Imagine the road at https://www.openstreetmap.org/node/258003112 would continue to
-             * the south here, then, still none of those footways would actually cross the street
-             *  - only if it continued straight to the north, for example.
-             *
-             * The example brings us to the less simple case: What if several roads share
-             * a node at a crossing-candidate position, like it is the case at every T-intersection?
-             * Also, what if there are more than one footways involved as in the link above?
-             *
-             * We look for if there is ANY crossing, so all polylines involved are checked:
-             * For all roads a car can go through point X, it is checked if not all footways that
-             * go through X are on the same side of the road-polyline.
-             * */
-            val nodePos = mapData.getNode(nodeId)?.position
-            return@retainAll nodePos != null &&
-                neighbouringFootwayPositions.anyCrossesAnyOf(neighbouringRoadPositions, nodePos)
+        crossings = crossings.filter { crossing ->
+            crossing.barrierWays.all { it.tags["sidewalk"] in anySidewalk } ||
+            crossing.barrierWays.all { it.tags["sidewalk"] !in anySidewalk }
         }
-
-        return footwaysByNodeId.keys
-            .mapNotNull { mapData.getNode(it) }
-            .filter { it.tags.isEmpty() }
+        return crossings.map { it.node }.filter { it.tags.isEmpty() }
     }
 
     override fun createForm() = AddKerbHeightForm()
@@ -154,40 +86,3 @@ class AddCrossing : OsmElementQuestType<KerbHeight> {
         }
     }
 }
-
-/** get the node id(s) neighbouring to the given node id */
-private fun Way.getNodeIdsNeighbouringNodeId(nodeId: Long): List<Long> {
-    val idx = nodeIds.indexOf(nodeId)
-    if (idx == -1) return emptyList()
-    val prevNodeId = if (idx > 0) nodeIds[idx - 1] else null
-    val nextNodeId = if (idx < nodeIds.size - 1) nodeIds[idx + 1] else null
-    return listOfNotNull(prevNodeId, nextNodeId)
-}
-
-private fun MutableMap<Long, MutableList<Way>>.removeEndNodes() {
-    entries.removeAll { (nodeId, ways) ->
-        ways.size == 1 && (nodeId == ways[0].nodeIds.first() || nodeId == ways[0].nodeIds.last())
-    }
-}
-
-/** groups the sequence of ways to a map of node id -> list of ways */
-private fun Sequence<Way>.groupByNodeIds(): MutableMap<Long, MutableList<Way>> {
-    val result = mutableMapOf<Long, MutableList<Way>>()
-    forEach { way ->
-        way.nodeIds.forEach { nodeId ->
-            result.getOrPut(nodeId, { mutableListOf() } ).add(way)
-        }
-    }
-    return result
-}
-
-/** Returns whether any of the lines spanned by any of the points in this list through the
- *  vertex point cross any of the lines spanned by any of the points through the vertex point
- *  from the other list */
-private fun List<LatLon>.anyCrossesAnyOf(other: List<LatLon>, vertex: LatLon): Boolean =
-    (1 until size).any { i -> other.anyAreOnDifferentSidesOf(this[0], vertex, this[i]) }
-
-/** Returns whether any of the points in this list are on different sides of the line spanned
- *  by p0 and p1 and the line spanned by p1 and p2 */
-private fun List<LatLon>.anyAreOnDifferentSidesOf(p0: LatLon, p1: LatLon, p2: LatLon): Boolean =
-    map { it.isRightOf(p0, p1, p2) }.toSet().size > 1