Merge pull request #904 from mapforge-org/refactor_layers

Refactor layers

Author: digitaltom

.rubocop.yml

@@ -4,7 +4,13 @@ inherit_gem:
 
 inherit_from: .rubocop_todo.yml
 
+inherit_mode:
+  merge:
+    - Exclude
+
 AllCops:
+  Exclude:
+    - 'deploy/**/*'
   TargetRubyVersion: 4.0
   DisplayCopNames: true
   DisplayStyleGuide: true

app/javascript/maplibre/feature/details.js

@@ -6,7 +6,6 @@ import { featureIcon, featureTitle } from 'maplibre/feature'
 import { showElevationChart } from 'maplibre/feature/elevation'
 import { showExtrasTotals } from 'maplibre/feature/extras_totals'
 import { getFeature, getLayer } from "maplibre/layers/layers"
-import { wikipediaFeatureDescription } from 'maplibre/layers/wikipedia'
 import { marked } from 'marked'
 
 // a modal without tabs (read only map, or a feature of a non geojson layer) has no
@@ -165,8 +164,6 @@ async function featureDescription (feature) {
   } else if (feature?.properties?.onclick === 'feature' && feature?.properties?.['onclick-target']) {
     // show feature target if onclick is feature
     desc = `<p><i class="bi bi-geo-alt-fill"></i> ${feature.properties['onclick-target']}</p>`
-  } else if (feature?.properties?.wikipediaId) {
-    desc = await wikipediaFeatureDescription(feature)
   } else {
     // layers can load their description on demand, the modal shows a loading message meanwhile.
     // the search layer is imported here, a static import of it breaks the module init order

app/javascript/maplibre/layers/basemap.js

@@ -5,7 +5,7 @@ import {
   stickyFeatureHighlight
 } from 'maplibre/feature'
 import { Layer } from 'maplibre/layers/layer'
-import { overpassDescription } from 'maplibre/layers/overpass/overpass'
+import { overpassDescription } from 'maplibre/layers/osm_description'
 import { addGeoJSONSource, map, mapProperties } from 'maplibre/map'
 import { basemaps } from 'maplibre/styles/basemaps'
 import { initializeViewStyles } from 'maplibre/styles/styles'
@@ -36,7 +36,7 @@ export class BasemapLayer extends Layer {
   }
 
   /**
-   * Override to disable click handler and provide custom mousemove for basemap layers.
+   * Adds a context menu for the copy of the highlighted feature, and a custom mousemove.
    */
   setupEventHandlers() {
     this.removeEventHandlers()
@@ -46,10 +46,7 @@ export class BasemapLayer extends Layer {
     this.contextMenuHandler = (e) => {
       e.preventDefault()
 
-      const basemapSource = this.sourceId
-      const mapLayers = map.getStyle().layers
-      const queryLayerIds = mapLayers.filter(layer => layer.source === basemapSource).map(layer => layer.id)
-      const features = map.queryRenderedFeatures(e.point, { layers: queryLayerIds })
+      const features = map.queryRenderedFeatures(e.point, { layers: this.getStyleLayerIds() })
 
       // the hovered feature was stored by highlightFeatureAtPoint, with an id of its own
       if (features.length) {
@@ -133,14 +130,25 @@ export class BasemapLayer extends Layer {
     if (!features.length) { return }
 
     const feature = features[0]
-    // exit early when moving over same feature
-    if (JSON.stringify(feature.geometry) === JSON.stringify(this?.selectedFeature?.geometry)) { return }
+    if (this.sameFeature(feature, this.selectedFeature)) { return }
     this.selectedFeature = feature
     hideContextMenu()
 
     this.renderHighlight(feature)
   }
 
+  // Runs on every mouse frame, and stringifying large polygons (water, landuse) is slow, so a
+  // different id answers at once. Merged tile features can share an id, so the geometry decides
+  // the rest, and the one of the selected feature is stringified only once.
+  sameFeature(feature, selected) {
+    if (!selected || feature.id !== selected.id || feature.sourceLayer !== selected.sourceLayer) { return false }
+    if (this.selectedGeometryOf !== selected) {
+      this.selectedGeometryOf = selected
+      this.selectedGeometry = JSON.stringify(selected.geometry)
+    }
+    return JSON.stringify(feature.geometry) === this.selectedGeometry
+  }
+
   /**
    * Custom mousemove handler for basemap layer - queries basemap source layers.
    */

app/javascript/maplibre/layers/geojson.js

@@ -20,8 +20,9 @@ import {
   initializeExtrasLabelStyles,
   renderRouteExtras
 } from 'maplibre/layers/geojson/route_extras'
-import { Layer } from 'maplibre/layers/layer'
-import { getFeature, layers } from 'maplibre/layers/layers'
+import { highlightFeature } from 'maplibre/feature'
+import { Layer, queryFeaturesNear } from 'maplibre/layers/layer'
+import { frontFeature, getFeature, layers } from 'maplibre/layers/layers'
 import { addGeoJSONSource, map, mapProperties, removeGeoJSONSource } from 'maplibre/map'
 import { pruneShapeImages } from 'maplibre/styles/circle_image'
 import { prunePatternImages } from 'maplibre/styles/pattern_image'
@@ -78,6 +79,38 @@ export class GeoJSONLayer extends Layer {
     addGeoJSONSource(this.extrusionSourceId, false)
   }
 
+  setupClickHandler() {
+    super.setupClickHandler()
+
+    // Double-click opens geometry edit mode directly
+    this.dblClickHandler = (e) => {
+      if (window.gon.map_mode !== 'rw') { return }
+      if (draw && draw.getMode() !== 'simple_select') { return }
+      if (e.defaultPrevented) { return }
+
+      const feature = queryFeaturesNear(e.point, { layers: this.getStyleLayerIds() })
+        .find(f => !f.properties?.cluster)
+      if (!feature) { return }
+
+      highlightFeature(feature, true, this.sourceId)
+      requestAnimationFrame(() => frontFeature(feature))
+      window.dispatchEvent(new CustomEvent('toggle-edit-feature', { detail: { type: 'geometry' } }))
+
+      // Prevent map zoom on double-click
+      e.preventDefault()
+      e.originalEvent.stopPropagation()
+    }
+    map.on('dblclick', this.dblClickHandler)
+  }
+
+  removeEventHandlers() {
+    super.removeEventHandlers()
+    if (this.dblClickHandler) {
+      map.off('dblclick', this.dblClickHandler)
+      this.dblClickHandler = null
+    }
+  }
+
   cleanup() {
     super.cleanup()
     cleanupKmMarkerImages(this.kmMarkerSourceId)
@@ -90,7 +123,7 @@ export class GeoJSONLayer extends Layer {
 
   initialize() {
     initializeViewStyles(this.sourceId, !!this.layer.heatmap)
-    if (this.layer.cluster) { initializeClusterStyles(this.sourceId, null) }
+    if (this.clustered) { initializeClusterStyles(this.sourceId, null) }
     initializeKmMarkerStyles(this.kmMarkerSourceId)
     initializeViewStyles(this.routeExtrasSourceId)
     initializeExtrasLabelStyles(this.routeExtrasSourceId)
@@ -104,6 +137,12 @@ export class GeoJSONLayer extends Layer {
     map.setLayoutProperty(`line-layer_${this.routeExtrasSourceId}`, 'line-cap', 'butt')
 
     this.setupEventHandlers()
+    // Showing a layer runs initialize() again. The source kept its features while hidden (updates
+    // reach it through updateData), unless setStyle replaced it, so only then fetch them again.
+    if (this.renderedSource && this.renderedSource === map.getSource(this.sourceId)) {
+      this.render(true, { sourceLoaded: true })
+      return Promise.resolve(this.layer.geojson)
+    }
     return this.loadData()
   }
 
@@ -129,7 +168,7 @@ export class GeoJSONLayer extends Layer {
       }
     })
 
-    if (this.layer.cluster) {
+    if (this.clustered) {
       clusterStyles(null).forEach(style => {
         const layerId = `${style.id}_${this.sourceId}`
         if (map.getLayer(layerId)) { map.setFilter(layerId, withLevelFilter(style.filter)) }
@@ -221,6 +260,7 @@ export class GeoJSONLayer extends Layer {
       })
     }
 
+    this.renderedSource = source
     this.resetDrawFeatures(resetDraw)
     pruneShapes()
   }

app/javascript/maplibre/layers/geojson/image_overlays.js

@@ -8,6 +8,8 @@ import { defaults } from 'maplibre/styles/defaults'
 // feature gets its own companion source, keyed by the geojson layer id.
 const sourcePrefix = layerId => `image-overlay-source-${layerId}-`
 const layerIdOf = sourceId => `image-overlay-layer_${sourceId}`
+// map.getStyle() would serialize the whole style on every render just to list these
+const overlaySources = new Set()
 
 // An image source takes four corners, no more and no less, so only a quadrangle can carry one.
 // The ring repeats its first point, so four corners are five coordinates.
@@ -29,7 +31,7 @@ export function renderImageOverlays (features, layerId, visible = true) {
     wanted.add(prefix + feature.id)
     upsertImageOverlay(feature, layerId, visible)
   })
-  Object.keys(map.getStyle().sources)
+  Array.from(overlaySources)
     .filter(id => id.startsWith(prefix) && !wanted.has(id))
     .forEach(removeImageOverlay)
 }
@@ -51,6 +53,7 @@ function upsertImageOverlay (feature, layerId, visible) {
   const url = feature.properties['fill-image-url']
   const coordinates = corners(feature)
   const source = map.getSource(sourceId)
+  overlaySources.add(sourceId)
   if (!source) {
     map.addSource(sourceId, { type: 'image', url, coordinates })
     // below the polygon outline of its own layer, sortLayers() keeps it there on a re-sort
@@ -71,6 +74,7 @@ function upsertImageOverlay (feature, layerId, visible) {
 }
 
 function removeImageOverlay (sourceId) {
+  overlaySources.delete(sourceId)
   if (map.getLayer(layerIdOf(sourceId))) { map.removeLayer(layerIdOf(sourceId)) }
   if (map.getSource(sourceId)) { map.removeSource(sourceId) }
 }

app/javascript/maplibre/layers/geojson/levels.js

@@ -110,7 +110,9 @@ export class IndoorLayer extends Layer {
       e.preventDefault()
     }
 
-    map.on('click', this.getStyleLayerIds(), this.clickHandler)
+    // map.off must get this same list, and getStyleLayerIds() is empty once the style is gone
+    this.clickLayerIds = this.getStyleLayerIds()
+    map.on('click', this.clickLayerIds, this.clickHandler)
 
     this.contextMenuHandler = (e_event) => {
       e_event.preventDefault()
@@ -139,7 +141,7 @@ export class IndoorLayer extends Layer {
     // this layer registers its click handler delegated to style layer ids, which
     // map.off('click', handler) in the base class cannot remove
     if (this.clickHandler) {
-      map.off('click', this.getStyleLayerIds(), this.clickHandler)
+      map.off('click', this.clickLayerIds, this.clickHandler)
       this.clickHandler = null
     }
     super.removeEventHandlers()
@@ -221,10 +223,6 @@ export class IndoorLayer extends Layer {
 
         if (JSON.stringify(newLevels) !== JSON.stringify(this.levels)) {
           this.levels = newLevels
-          // console.log('Indoor layer: detected levels', newLevels)
-          this.updateLevelControl()
-        } else if (this.levels.length > 0 && !this.levelControl) {
-          // Recreate control if it was removed (e.g., layer was hidden then shown)
           this.updateLevelControl()
         }
       }, `indoor-level-${this.id}`, 500)
@@ -235,6 +233,8 @@ export class IndoorLayer extends Layer {
   }
 
   removeLevelDetection() {
+    // a hidden layer drops out of detectLevels(), so the next show must report its levels again
+    this.levels = []
     if (this.initialTimeout) {
       clearTimeout(this.initialTimeout)
       this.initialTimeout = null