From e32c06ec271abd092aa0d139c4977a7f5a50aff8 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Mon, 29 Jun 2026 11:20:34 +0100 Subject: [PATCH 1/7] Hide snap targets when not in a draw mode --- .../beta/draw-ol/src/core/OLDrawManager.js | 3 + .../beta/draw-ol/src/snap/snapInteraction.js | 59 ++++++++++--------- plugins/beta/draw-ol/src/snap/snapManager.js | 11 +++- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/plugins/beta/draw-ol/src/core/OLDrawManager.js b/plugins/beta/draw-ol/src/core/OLDrawManager.js index 1ca587a2..e7d8b835 100644 --- a/plugins/beta/draw-ol/src/core/OLDrawManager.js +++ b/plugins/beta/draw-ol/src/core/OLDrawManager.js @@ -75,6 +75,9 @@ export class OLDrawManager { this._modeInstance = null this._mode = modeName + const isDrawMode = modeName === 'draw_polygon' || modeName === 'draw_line' || modeName === 'edit_vertex' + this.snap?.setIndicatorActive(isDrawMode) + const modeOptions = { ...options, snap: this.snap } if (modeName === 'draw_polygon' || modeName === 'draw_line') { diff --git a/plugins/beta/draw-ol/src/snap/snapInteraction.js b/plugins/beta/draw-ol/src/snap/snapInteraction.js index 25bdfd0e..67279e5b 100644 --- a/plugins/beta/draw-ol/src/snap/snapInteraction.js +++ b/plugins/beta/draw-ol/src/snap/snapInteraction.js @@ -16,38 +16,39 @@ import Interaction from 'ol/interaction/Interaction.js' const SNAP_EVENTS = new Set(['pointermove', 'pointerdrag', 'pointerdown', 'pointerup', 'singleclick', 'click']) -export const createSnapInteraction = (engine, indicator, snapRadius) => { +const processSnapEvent = (mapBrowserEvent, engine, indicator, snapRadius, isIndicatorActive) => { + const { type } = mapBrowserEvent + + if (type === 'pointerout' || type === 'pointerleave') { + indicator.hide() + return + } + + if (!SNAP_EVENTS.has(type)) { + return + } + + const result = engine.query(mapBrowserEvent.coordinate, snapRadius) + if (result) { + mapBrowserEvent.coordinate = result.coord.slice() + } + + // Only update indicator during free mouse movement — hide during drag, no-op for clicks + if (type === 'pointermove' && isIndicatorActive()) { + result ? indicator.show(result.coord, result.type) : indicator.hide() + } else if (type === 'pointerdrag') { + indicator.hide() + } else { + // no indicator update for click/down/up events + } +} + +export const createSnapInteraction = (engine, indicator, snapRadius, isIndicatorActive) => { const interaction = new Interaction({ handleEvent (mapBrowserEvent) { - if (!interaction.getActive()) { - return true - } - - const { type } = mapBrowserEvent - - if (type === 'pointerout' || type === 'pointerleave') { - indicator.hide() - return true - } - - if (!SNAP_EVENTS.has(type)) { - return true - } - - const result = engine.query(mapBrowserEvent.coordinate, snapRadius) - if (result) { - mapBrowserEvent.coordinate = result.coord.slice() + if (interaction.getActive()) { + processSnapEvent(mapBrowserEvent, engine, indicator, snapRadius, isIndicatorActive) } - - // Only show indicator during free mouse movement — hide during drag and clicks - if (type === 'pointermove') { - result ? indicator.show(result.coord, result.type) : indicator.hide() - } else if (type === 'pointerdrag') { - indicator.hide() - } else { - // No action - } - return true } }) diff --git a/plugins/beta/draw-ol/src/snap/snapManager.js b/plugins/beta/draw-ol/src/snap/snapManager.js index 57479bcf..767776ab 100644 --- a/plugins/beta/draw-ol/src/snap/snapManager.js +++ b/plugins/beta/draw-ol/src/snap/snapManager.js @@ -23,7 +23,9 @@ export const createSnapManager = (map, snapLayers, colors, snapRadius) => { const engine = createSnapEngine(map, snapLayers) const indicator = createSnapIndicator(map, colors) - const interaction = createSnapInteraction(engine, indicator, snapRadius) + + let indicatorActive = false + const interaction = createSnapInteraction(engine, indicator, snapRadius, () => indicatorActive) map.addInteraction(interaction) interaction.setActive(false) // matches reducer initial state: snap: false @@ -55,6 +57,13 @@ export const createSnapManager = (map, snapLayers, colors, snapRadius) => { indicator.hide() }, + setIndicatorActive (value) { + indicatorActive = value + if (!value) { + indicator.hide() + } + }, + setActive (value) { active = value interaction.setActive(value) From b7caea44874e9311272ce38d199d4acf15994afd Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 30 Jun 2026 11:39:33 +0100 Subject: [PATCH 2/7] Draw snap tile boundary fix --- plugins/beta/draw-ol/src/snap/snapEngine.js | 106 +++++++++++++----- plugins/beta/draw-ol/src/snap/snapGeometry.js | 69 ++++-------- 2 files changed, 98 insertions(+), 77 deletions(-) diff --git a/plugins/beta/draw-ol/src/snap/snapEngine.js b/plugins/beta/draw-ol/src/snap/snapEngine.js index b958236f..35b7f703 100644 --- a/plugins/beta/draw-ol/src/snap/snapEngine.js +++ b/plugins/beta/draw-ol/src/snap/snapEngine.js @@ -1,21 +1,68 @@ -/** - * Snap candidate engine. - * - * Accepts two kinds of entry in snapLayers: - * - string → a VectorTile style-layer name (matched via feature.get('layer')) - * All VectorTileLayers on the map are searched; only features whose - * style-layer name is in the set are tested. - * - OL VectorLayer instance → all features in that layer's source are tested. - * - * query() is synchronous and uses: - * - VectorSource.getFeaturesInExtent() for OL vector layers (internal rBush, fast) - * - map.forEachFeatureAtPixel() for VectorTile layers (rendered tile data) - */ - +import { transform as projTransform } from 'ol/proj.js' import VectorLayer from 'ol/layer/Vector.js' import VectorTileLayer from 'ol/layer/VectorTile.js' import { testOLFeature, testRenderFeature } from './snapGeometry.js' +// VectorTile features are clipped to tile extents, producing artificial straight edges at tile +// boundaries. MVT tiles also carry a buffer region (geometry from adjacent tiles extending +// past the tile boundary). Both the exact boundary and the buffer zone must be filtered. +// Standard MVT buffer is 128 out of 4096 tile coordinate units. +const TILE_BOUNDARY_EPS = 1 // source-projection units of margin beyond the buffer +const MVT_BUFFER_UNITS = 128 +const MVT_TILE_EXTENT = 4096 + +const buildTileBoundaryState = (layer, map) => { + const source = layer.getSource() + const tileGrid = source?.getTileGrid() + if (!tileGrid) { return null } + const viewProj = map.getView().getProjection() + const sourceProj = source.getProjection() ?? viewProj + const zoom = tileGrid.getZForResolution(map.getView().getResolution(), 0) + return { tileGrid, viewProj, sourceProj, zoom } +} + +const isOnTileBoundary = (state, coord) => { + if (!state) { return false } + const { tileGrid, viewProj, sourceProj, zoom } = state + const c = projTransform(coord, viewProj, sourceProj) + const tileCoord = tileGrid.getTileCoordForCoordAndZ(c, zoom) + const [minX, minY, maxX, maxY] = tileGrid.getTileCoordExtent(tileCoord) + // Buffer zone in source projection: edges within this distance of a tile boundary are + // MVT buffer clip artefacts (geometry from the adjacent tile included for rendering overlap). + const tileBuffer = (maxX - minX) * (MVT_BUFFER_UNITS / MVT_TILE_EXTENT) + const eps = tileBuffer + TILE_BOUNDARY_EPS + return ( + Math.abs(c[0] - minX) < eps || + Math.abs(c[0] - maxX) < eps || + Math.abs(c[1] - minY) < eps || + Math.abs(c[1] - maxY) < eps + ) +} + +// Two same-style fill polygons share an invisible boundary — snapping to it is confusing. +// Detect this by projecting a test point slightly past the snap position (away from the cursor) +// and checking whether the same fill layer covers that side too. +// If it does → same-style invisible boundary → skip. +// If not (empty space, road, different layer) → visible outer edge → include. +const INVISIBLE_FILL_MIN_DIST_SQ = 1 // (1m)² — below this, cursor is on the edge, skip direction check + +const isInvisibleFillBoundary = (edgeCoord, cursorCoord, layerId, map, vtLayers, resolution) => { + const dx = edgeCoord[0] - cursorCoord[0] + const dy = edgeCoord[1] - cursorCoord[1] + const distSq = dx * dx + dy * dy + if (distSq < INVISIBLE_FILL_MIN_DIST_SQ) { return false } + const dist = Math.sqrt(distSq) + const step = resolution * 4 + const testCoord = [edgeCoord[0] + (dx / dist) * step, edgeCoord[1] + (dy / dist) * step] + const testPixel = map.getPixelFromCoordinate(testCoord) + if (!testPixel) { return false } + return !!map.forEachFeatureAtPixel( + testPixel, + (f) => f.get('mapbox-layer')?.id === layerId, + { hitTolerance: 2, layerFilter: (l) => vtLayers.includes(l) } + ) +} + const pickBest = (a, b) => { if (!b) { return a } if (!a) { return b } @@ -53,17 +100,9 @@ export const createSnapEngine = (map, snapLayers = []) => { setLayers(snapLayers) - /** - * Find the nearest snap candidate to coord within radiusPx screen pixels. - * @param {number[]} coord - map coordinate [x, y] - * @param {number} radiusPx - tolerance in screen pixels - * @returns {{ type: 'vertex'|'edge', coord: number[] } | null} - */ const query = (coord, radiusPx) => { const resolution = map.getView().getResolution() - if (!resolution) { - return null - } + if (!resolution) { return null } const toleranceMapUnits = radiusPx * resolution const toleranceSq = toleranceMapUnits * toleranceMapUnits const ext = [ @@ -75,27 +114,32 @@ export const createSnapEngine = (map, snapLayers = []) => { let best = null - // --- OL VectorLayer sources --- for (const layer of olLayers) { const source = layer.getSource() - if (!source) { - continue - } + if (!source) { continue } for (const feature of source.getFeaturesInExtent(ext)) { best = pickBest(best, testOLFeature(feature, coord, toleranceSq)) } } - // --- VectorTile layers --- if (vtLayerNames.size > 0) { const vtLayers = getVTLayers(map) const pixel = vtLayers.length > 0 ? map.getPixelFromCoordinate(coord) : null if (pixel) { + const tileBoundaryStates = new Map() map.forEachFeatureAtPixel( pixel, - (feature, _layer) => { - if (!vtLayerNames.has(feature.get('mapbox-layer')?.id)) { return } - best = pickBest(best, testRenderFeature(feature, coord, toleranceSq)) + (feature, layer) => { + const mapboxLayer = feature.get('mapbox-layer') + if (!vtLayerNames.has(mapboxLayer?.id)) { return } + const candidate = testRenderFeature(feature, coord, toleranceSq) + if (!candidate) { return } + if (!tileBoundaryStates.has(layer)) { + tileBoundaryStates.set(layer, buildTileBoundaryState(layer, map)) + } + if (isOnTileBoundary(tileBoundaryStates.get(layer), candidate.coord)) { return } + if (candidate.type === 'edge' && mapboxLayer?.type === 'fill' && isInvisibleFillBoundary(candidate.coord, coord, mapboxLayer.id, map, vtLayers, resolution)) { return } + best = pickBest(best, candidate) }, { hitTolerance: radiusPx, layerFilter: (l) => vtLayers.includes(l) } ) diff --git a/plugins/beta/draw-ol/src/snap/snapGeometry.js b/plugins/beta/draw-ol/src/snap/snapGeometry.js index 14432ed6..16aecebd 100644 --- a/plugins/beta/draw-ol/src/snap/snapGeometry.js +++ b/plugins/beta/draw-ol/src/snap/snapGeometry.js @@ -1,10 +1,3 @@ -/** - * Pure geometry helpers for snap candidate testing. - * No OL imports, no side effects — only coordinate math. - * - * All coordinates are [x, y] pairs in map projection units. - */ - const dist2 = (a, b) => { const dx = a[0] - b[0] const dy = a[1] - b[1] @@ -41,11 +34,8 @@ const better = (a, b) => { return b.distSq < a.distSq } -/** - * Test all vertices and edges of a coordinate ring/line for snap candidates. - * coords: [[x,y], ...] — OL ring (first === last for closed rings) - * isClosedRing: if true, OL has duplicated the first coord as last; skip it and wrap edges - */ +// OL Vector rings duplicate the first coord as last; VectorTile rings do not. +// isClosedRing=true: skip the duplicated last vertex, wrap closing edge back to first. const testCoords = (coords, query, toleranceSq, isClosedRing) => { let best = null const n = isClosedRing && coords.length > 1 ? coords.length - 1 : coords.length @@ -72,13 +62,6 @@ const testCoords = (coords, query, toleranceSq, isClosedRing) => { return best } -/** - * Test flat coordinate array (stride 2) from a VectorTile RenderFeature. - * flat: number[] — [x0,y0,x1,y1,...] - * start/end: index range within flat - * isClosedRing: VTile polygon rings — first coord is NOT duplicated at end (unlike OL Vector) - * so treat all coords as unique vertices and add a closing edge back to first - */ const getBestEdge = (flat, start, numPairs, edgeCount, query, toleranceSq) => { let best = null for (let i = 0; i < edgeCount; i++) { @@ -95,7 +78,7 @@ const getBestEdge = (flat, start, numPairs, edgeCount, query, toleranceSq) => { return best } -const getBestPair = (flat, start, numPairs, edgeCount, query, toleranceSq) => { +const getBestVertex = (flat, start, numPairs, query, toleranceSq) => { let best = null for (let i = 0; i < numPairs; i++) { const xi = start + i * 2 @@ -105,7 +88,14 @@ const getBestPair = (flat, start, numPairs, edgeCount, query, toleranceSq) => { best = bestOf(best, { type: 'vertex', coord: v, distSq: dSq }) } } - return bestOf(best, getBestEdge(flat, start, numPairs, edgeCount, query, toleranceSq)) + return best +} + +const getBestPair = (flat, start, numPairs, edgeCount, query, toleranceSq) => { + return bestOf( + getBestVertex(flat, start, numPairs, query, toleranceSq), + getBestEdge(flat, start, numPairs, edgeCount, query, toleranceSq) + ) } const testFlatCoords = (flat, start, end, query, toleranceSq, isClosedRing) => { @@ -115,32 +105,32 @@ const testFlatCoords = (flat, start, end, query, toleranceSq, isClosedRing) => { } const olGeomHandlers = { - Point (geom, query, toleranceSq) { + point: (geom, query, toleranceSq) => { const c = geom.getCoordinates() const dSq = dist2(query, c) return dSq <= toleranceSq ? { type: 'vertex', coord: [c[0], c[1]], distSq: dSq } : null }, - LineString (geom, query, toleranceSq) { + lineString: (geom, query, toleranceSq) => { return testCoords(geom.getCoordinates(), query, toleranceSq, false) }, - LinearRing (geom, query, toleranceSq) { + linearRing: (geom, query, toleranceSq) => { return testCoords(geom.getCoordinates(), query, toleranceSq, true) }, - Polygon (geom, query, toleranceSq) { + polygon: (geom, query, toleranceSq) => { let best = null for (const ring of geom.getCoordinates()) { best = bestOf(best, testCoords(ring, query, toleranceSq, true)) } return best }, - MultiLineString (geom, query, toleranceSq) { + multiLineString: (geom, query, toleranceSq) => { let best = null for (const line of geom.getCoordinates()) { best = bestOf(best, testCoords(line, query, toleranceSq, false)) } return best }, - MultiPolygon (geom, query, toleranceSq) { + multiPolygon: (geom, query, toleranceSq) => { let best = null for (const polygon of geom.getCoordinates()) { for (const ring of polygon) { @@ -151,27 +141,14 @@ const olGeomHandlers = { } } -/** - * Test an OL Feature (from a VectorSource) against query coord. - * Handles Point, LineString, LinearRing, Polygon, MultiLineString, MultiPolygon. - * - * @returns {{ type: 'vertex'|'edge', coord: number[], distSq: number } | null} - */ export const testOLFeature = (feature, query, toleranceSq) => { const geom = feature.getGeometry() - if (!geom) { - return null - } - const handler = olGeomHandlers[geom.getType()] + if (!geom) { return null } + const rawType = geom.getType() + const handler = olGeomHandlers[rawType[0].toLowerCase() + rawType.slice(1)] return handler ? handler(geom, query, toleranceSq) : null } -/** - * Test an OL RenderFeature (from a VectorTileSource) against query coord. - * Handles Point, LineString, Polygon, MultiLineString. - * - * @returns {{ type: 'vertex'|'edge', coord: number[], distSq: number } | null} - */ export const testRenderFeature = (feature, query, toleranceSq) => { const type = feature.getType() const flat = feature.getFlatCoordinates() @@ -180,10 +157,10 @@ export const testRenderFeature = (feature, query, toleranceSq) => { if (type === 'Point') { const dSq = dist2(query, flat) if (dSq <= toleranceSq) { - best = bestOf(best, { type: 'vertex', coord: [flat[0], flat[1]], distSq: dSq }) + best = { type: 'vertex', coord: [flat[0], flat[1]], distSq: dSq } } } else if (type === 'LineString') { - best = bestOf(best, testFlatCoords(flat, 0, flat.length, query, toleranceSq, false)) + best = testFlatCoords(flat, 0, flat.length, query, toleranceSq, false) } else if (type === 'Polygon' || type === 'MultiLineString') { const ends = feature.getEnds() let start = 0 @@ -193,7 +170,7 @@ export const testRenderFeature = (feature, query, toleranceSq) => { start = end } } else { - // No action + // MultiPoint / unknown — no snap candidates } return best From 18b6cb017053b99338ab8680a44d5f6d67e09f30 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 30 Jun 2026 14:17:07 +0100 Subject: [PATCH 3/7] Closing polygon min vertecies fix --- plugins/beta/draw-ol/src/draw/DrawMode.js | 101 +++++++++++---------- plugins/beta/draw-ol/src/draw/drawInput.js | 76 +++++++++++----- 2 files changed, 103 insertions(+), 74 deletions(-) diff --git a/plugins/beta/draw-ol/src/draw/DrawMode.js b/plugins/beta/draw-ol/src/draw/DrawMode.js index 00f9d5d7..c5805ffd 100644 --- a/plugins/beta/draw-ol/src/draw/DrawMode.js +++ b/plugins/beta/draw-ol/src/draw/DrawMode.js @@ -1,18 +1,48 @@ import Draw from 'ol/interaction/Draw.js' +import { noModifierKeys } from 'ol/events/condition.js' import { createDrawInput } from './drawInput.js' import { getCoords } from '../utils/geometryHelpers.js' -/** - * Draw mode — handles draw_polygon and draw_line. - * - * OL's Draw interaction handles all pointer/mouse behaviour natively. - * drawInput.js handles touch/keyboard/button input. - * - * @returns {{ done, cancel, undo, destroy }} - */ +const SNAP_TOLERANCE_PX = 12 +const MIN_VERTICES = { Polygon: 3, LineString: 2 } + +const canFinish = (geometryType, sketchFeature) => { + if (!sketchFeature) { return false } + const geom = sketchFeature.getGeometry() + const coords = getCoords({ type: geometryType, coordinates: geom.getCoordinates() }) + // OL keeps a trailing rubber-band coordinate; subtract 1 to get real vertex count + return coords.length - 1 >= MIN_VERTICES[geometryType] +} + +// OL closes Polygon rings by appending v1: [...placed, rubber_band, v1_closing]; last placed is 3 from end. +const POLY_LAST_PLACED_OFFSET = 3 + +const getLastPlacedCoord = (geom) => { + if (geom.getType() === 'Polygon') { + const ring = geom.getCoordinates()[0] || [] + return ring.length >= POLY_LAST_PLACED_OFFSET ? ring[ring.length - POLY_LAST_PLACED_OFFSET] : null + } + const coords = geom.getCoordinates() + return coords.length >= 2 ? coords[coords.length - 2] : null +} + +const DUPLICATE_TOLERANCE_PX = 2 + +const buildCondition = (map, geometryType, getSketchFeature) => (e) => { + if (!noModifierKeys(e)) { return false } + const sf = getSketchFeature() + if (!sf || canFinish(geometryType, sf)) { return true } + const prev = getLastPlacedCoord(sf.getGeometry()) + if (!prev) { return true } + const pp = map.getPixelFromCoordinate(prev) + if (!pp) { return true } + const dx = e.pixel[0] - pp[0]; const dy = e.pixel[1] - pp[1] + return dx * dx + dy * dy > DUPLICATE_TOLERANCE_PX * DUPLICATE_TOLERANCE_PX +} + export const createDrawMode = ({ map, manager, options }) => { const { - geometryType, // 'Polygon' | 'LineString' + geometryType, featureId, properties = {}, container, @@ -22,28 +52,23 @@ export const createDrawMode = ({ map, manager, options }) => { snap } = options + let sketchFeature = null + const drawInteraction = new Draw({ type: geometryType, style: manager.styles.createSketchStyle(), stopClick: true, - // minPoints defaults: 3 for Polygon, 2 for LineString — OL handles this - // snapTolerance: how close to first point to auto-close polygon - snapTolerance: 12 + snapTolerance: SNAP_TOLERANCE_PX, + condition: buildCondition(map, geometryType, () => sketchFeature) }) map.addInteraction(drawInteraction) - // Track vertex count for the Done button enabled state - let sketchFeature = null - const updateVertexCount = () => { - if (!sketchFeature) { - return - } + if (!sketchFeature) { return } const geom = sketchFeature.getGeometry() const coords = getCoords({ type: geometryType, coordinates: geom.getCoordinates() }) // OL always keeps a trailing rubber-band coordinate; subtract 1 - const numVertices = Math.max(0, coords.length - 1) - manager.emit('vertexchange', { numVertices }) + manager.emit('vertexchange', { numVertices: Math.max(0, coords.length - 1) }) } drawInteraction.on('drawstart', (e) => { @@ -56,14 +81,11 @@ export const createDrawMode = ({ map, manager, options }) => { olFeature.setId(String(featureId)) olFeature.setProperties(properties) manager.store.source.addFeature(olFeature) - const geojson = manager.store.toGeoJSON(olFeature) - manager.emit('create', geojson) + manager.emit('create', manager.store.toGeoJSON(olFeature)) // Mode switches to disabled in events.js after receiving 'create' }) - drawInteraction.on('drawabort', () => { - manager.emit('cancel') - }) + drawInteraction.on('drawabort', () => { manager.emit('cancel') }) const input = createDrawInput({ drawInteraction, @@ -74,36 +96,17 @@ export const createDrawMode = ({ map, manager, options }) => { addVertexButtonId, mapProvider, snap, - onUndo: () => { - drawInteraction.removeLastPoint() - updateVertexCount() - } + onUndo: () => { drawInteraction.removeLastPoint(); updateVertexCount() }, + canFinish: () => canFinish(geometryType, sketchFeature) } }) return { done () { - // Validate minimum points before finishing - if (sketchFeature) { - const geom = sketchFeature.getGeometry() - const coords = getCoords({ type: geometryType, coordinates: geom.getCoordinates() }) - const min = geometryType === 'Polygon' ? 4 : 3 // +1 for rubber band - if (coords.length < min) { - return - } - } - drawInteraction.finishDrawing() + if (canFinish(geometryType, sketchFeature)) { drawInteraction.finishDrawing() } }, - - cancel () { - drawInteraction.abortDrawing() - }, - - undo () { - drawInteraction.removeLastPoint() - updateVertexCount() - }, - + cancel () { drawInteraction.abortDrawing() }, + undo () { drawInteraction.removeLastPoint(); updateVertexCount() }, destroy () { input.destroy() map.removeInteraction(drawInteraction) diff --git a/plugins/beta/draw-ol/src/draw/drawInput.js b/plugins/beta/draw-ol/src/draw/drawInput.js index cf00d059..51c93eed 100644 --- a/plugins/beta/draw-ol/src/draw/drawInput.js +++ b/plugins/beta/draw-ol/src/draw/drawInput.js @@ -1,18 +1,15 @@ -/** - * Input handling for draw mode: touch, keyboard, and button events. - * - * Mouse/pointer drawing is handled entirely by OL's Draw interaction. - * This module handles the crosshair-based input path (touch + keyboard) - * and the Done / Add Point / Cancel button wiring. - */ - import { coordToPixel, pixelDist } from '../utils/olCoords.js' const SNAP_TOLERANCE = 12 // pixels +// Minimum ring length to allow snap-to-close (placed vertices + rubber-band) +const MIN_SKETCH_COORDS = { Polygon: 4, LineString: 3 } +const DUPLICATE_TOLERANCE_PX = 2 +// OL Polygon ring layout after addToDrawing_: [...committed, rubber_band, closing_v1] +const POLY_COMMITTED_OFFSET = 3 const ARROW_KEYS = new Set(['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown']) const isCloseToFirstVertex = (map, coord, sketchCoords, geometryType) => { - if (geometryType !== 'Polygon' || sketchCoords.length < 4) { + if (geometryType !== 'Polygon' || sketchCoords.length < MIN_SKETCH_COORDS.Polygon) { return false } const firstCoord = sketchCoords[0] @@ -44,6 +41,17 @@ const applyRubberbanding = (geom, centerCoord) => { } } +// Returns the last vertex committed by OL's Draw interaction (not the rubber-band or +// the closing copy that OL appends to Polygon rings). +const getLastCommittedVertex = (geom) => { + if (geom.getType() === 'Polygon') { + const ring = geom.getCoordinates()[0] || [] + return ring.length >= POLY_COMMITTED_OFFSET ? ring[ring.length - POLY_COMMITTED_OFFSET] : null + } + const coords = geom.getCoordinates() + return coords.length >= 2 ? coords[coords.length - 2] : null +} + const wireInputEvents = ({ container, addVertexButtonId, olView, onUndo, getInterfaceType, setInterfaceType, clearLastCoord, @@ -117,14 +125,8 @@ const wireInputEvents = ({ } } -/** - * @param {object} params - * @param {import('ol/interaction/Draw').default} params.drawInteraction - * @param {object} params.options - { container, interfaceType, addVertexButtonId, mapProvider, snap } - * @returns {{ getInterfaceType: () => string, destroy: () => void }} - */ export const createDrawInput = ({ drawInteraction, options }) => { - const { container, addVertexButtonId, mapProvider, snap, onUndo } = options + const { container, addVertexButtonId, mapProvider, snap, onUndo, canFinish } = options let interfaceType = options.interfaceType let sketchFeature = null let lastPlacedCoord = null @@ -156,6 +158,38 @@ export const createDrawInput = ({ drawInteraction, options }) => { applyRubberbanding(geom, centerCoord) } + // Returns true if the vertex was handled as a close/finish attempt (caller should not append). + const tryClose = (geom, sketchCoords, coord) => { + if (lastPlacedCoord && lastPlacedCoord[0] === coord[0] && lastPlacedCoord[1] === coord[1]) { + // Same position as last placed: don't duplicate. Close only if enough real vertices exist. + if (canFinish?.()) { drawInteraction.finishDrawing() } + lastPlacedCoord = null + return true + } + if (isCloseToFirstVertex(drawInteraction.getMap(), coord, sketchCoords, geom.getType())) { + drawInteraction.finishDrawing() + return true + } + // When the add-vertex button overlays the map (touch UI), OL's native pointer handler + // and this button click handler both fire for the same tap. Detect that OL already + // committed a vertex at coord's position and skip the duplicate appendCoordinates, + // but register coord as lastPlacedCoord so a second tap at the same position can close. + const map = drawInteraction.getMap() + const lastCommitted = getLastCommittedVertex(geom) + if (lastCommitted) { + const p1 = map.getPixelFromCoordinate(lastCommitted) + const p2 = map.getPixelFromCoordinate(coord) + if (p1 && p2) { + const dx = p1[0] - p2[0]; const dy = p1[1] - p2[1] + if (dx * dx + dy * dy < DUPLICATE_TOLERANCE_PX * DUPLICATE_TOLERANCE_PX) { + lastPlacedCoord = coord + return true + } + } + } + return false + } + const placeVertex = () => { const raw = mapProvider.getCenter() const coord = (interfaceType !== 'pointer' && snap) ? snap.apply(raw) : raw @@ -164,15 +198,7 @@ export const createDrawInput = ({ drawInteraction, options }) => { const geom = sketchFeature.getGeometry() const rawCoords = geom.getCoordinates() const sketchCoords = geom.getType() === 'Polygon' ? (rawCoords[0] || []) : rawCoords - if (lastPlacedCoord && lastPlacedCoord[0] === coord[0] && lastPlacedCoord[1] === coord[1]) { - drawInteraction.finishDrawing() - lastPlacedCoord = null - return - } - if (isCloseToFirstVertex(drawInteraction.getMap(), coord, sketchCoords, geom.getType())) { - drawInteraction.finishDrawing() - return - } + if (tryClose(geom, sketchCoords, coord)) { return } } drawInteraction.appendCoordinates([coord]) lastPlacedCoord = coord From 91936c6c9329234ad4e7f95f964ae291c966de42 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 30 Jun 2026 14:43:48 +0100 Subject: [PATCH 4/7] Keyboard update while animating fix --- plugins/beta/draw-ol/src/draw/DrawMode.js | 5 +++++ plugins/beta/draw-ol/src/draw/drawInput.js | 14 +++++++++++++- plugins/beta/draw-ol/src/utils/geometryHelpers.js | 12 +++++++++--- plugins/beta/draw-ol/src/utils/olCoords.js | 4 ++-- plugins/beta/draw-ol/src/utils/spatial.js | 12 ++++++------ 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/plugins/beta/draw-ol/src/draw/DrawMode.js b/plugins/beta/draw-ol/src/draw/DrawMode.js index c5805ffd..fc12e454 100644 --- a/plugins/beta/draw-ol/src/draw/DrawMode.js +++ b/plugins/beta/draw-ol/src/draw/DrawMode.js @@ -62,6 +62,11 @@ export const createDrawMode = ({ map, manager, options }) => { condition: buildCondition(map, geometryType, () => sketchFeature) }) map.addInteraction(drawInteraction) + // OL internal: overlay_ is the private VectorLayer used for the sketch geometry. + // updateWhileAnimating_ forces per-frame redraws during view animations (keyboard pan). + // Without this, geom.setCoordinates() calls are ignored while the ANIMATING hint is set. + // Check ol/interaction/Draw.js and ol/layer/BaseVector.js if this breaks after an OL upgrade. + drawInteraction.overlay_.updateWhileAnimating_ = true const updateVertexCount = () => { if (!sketchFeature) { return } diff --git a/plugins/beta/draw-ol/src/draw/drawInput.js b/plugins/beta/draw-ol/src/draw/drawInput.js index 51c93eed..239f76a6 100644 --- a/plugins/beta/draw-ol/src/draw/drawInput.js +++ b/plugins/beta/draw-ol/src/draw/drawInput.js @@ -204,10 +204,13 @@ export const createDrawInput = ({ drawInteraction, options }) => { lastPlacedCoord = coord } + const map = drawInteraction.getMap() + const olView = map?.getView() + const events = wireInputEvents({ container, addVertexButtonId, - olView: drawInteraction.getMap()?.getView(), + olView, onUndo, getInterfaceType: () => interfaceType, setInterfaceType: (t) => { interfaceType = t }, @@ -216,10 +219,19 @@ export const createDrawInput = ({ drawInteraction, options }) => { placeVertex }) + // change:center fires once when a keyboard pan animation starts; postrender tracks each frame. + const onMapRender = () => { + if (interfaceType !== 'pointer' && olView?.getAnimating()) { + updateRubberbanding() + } + } + map?.on('postrender', onMapRender) + return { getInterfaceType: () => interfaceType, destroy () { events.destroy() + map?.un('postrender', onMapRender) } } } diff --git a/plugins/beta/draw-ol/src/utils/geometryHelpers.js b/plugins/beta/draw-ol/src/utils/geometryHelpers.js index 83241941..2ac3dc94 100644 --- a/plugins/beta/draw-ol/src/utils/geometryHelpers.js +++ b/plugins/beta/draw-ol/src/utils/geometryHelpers.js @@ -5,7 +5,9 @@ */ export const getCoords = (geom) => { - if (!geom?.coordinates) return [] + if (!geom?.coordinates) { + return [] + } switch (geom.type) { case 'LineString': return geom.coordinates case 'Polygon': return geom.coordinates.flatMap(ring => ring.slice(0, -1)) @@ -20,7 +22,9 @@ export const getCoords = (geom) => { * { start, length, path, closed } */ export const getRingSegments = (geom) => { - if (!geom?.coordinates) return [] + if (!geom?.coordinates) { + return [] + } const segments = [] let start = 0 @@ -80,7 +84,9 @@ export const getModifiableCoords = (geojsonGeometry, path) => { export const getMidpoints = (geom) => { const coords = getCoords(geom) const segments = getRingSegments(geom) - if (!coords.length || !segments.length) return [] + if (!coords.length || !segments.length) { + return [] + } const midpoints = [] for (const seg of segments) { diff --git a/plugins/beta/draw-ol/src/utils/olCoords.js b/plugins/beta/draw-ol/src/utils/olCoords.js index 7b0abbcb..de730e13 100644 --- a/plugins/beta/draw-ol/src/utils/olCoords.js +++ b/plugins/beta/draw-ol/src/utils/olCoords.js @@ -6,7 +6,7 @@ /** OL coordinate [e, n] → screen pixel { x, y } */ export const coordToPixel = (map, coord) => { const px = map.getPixelFromCoordinate(coord) - if (!px) return null + if (!px) { return null } return { x: px[0], y: px[1] } } @@ -30,6 +30,6 @@ export const pixelToArray = ({ x, y }) => [x, y] */ export const nudgeCoord = (map, coord, dx, dy) => { const px = map.getPixelFromCoordinate(coord) - if (!px) return coord + if (!px) { return coord } return map.getCoordinateFromPixel([px[0] + dx, px[1] + dy]) } diff --git a/plugins/beta/draw-ol/src/utils/spatial.js b/plugins/beta/draw-ol/src/utils/spatial.js index 60f3c8ba..35be1d0c 100644 --- a/plugins/beta/draw-ol/src/utils/spatial.js +++ b/plugins/beta/draw-ol/src/utils/spatial.js @@ -12,15 +12,15 @@ export const spatialNavigate = (start, pixels, direction) => { const dx = Math.abs(p[0] - start[0]) const dy = Math.abs(p[1] - start[1]) let inQuadrant = false - if (direction === 'ArrowUp') inQuadrant = p[1] <= start[1] && dy >= dx - else if (direction === 'ArrowDown') inQuadrant = p[1] > start[1] && dy >= dx - else if (direction === 'ArrowLeft') inQuadrant = p[0] <= start[0] && dy < dx - else if (direction === 'ArrowRight') inQuadrant = p[0] > start[0] && dy < dx - else inQuadrant = true + if (direction === 'ArrowUp') { inQuadrant = p[1] <= start[1] && dy >= dx } + else if (direction === 'ArrowDown') { inQuadrant = p[1] > start[1] && dy >= dx } + else if (direction === 'ArrowLeft') { inQuadrant = p[0] <= start[0] && dy < dx } + else if (direction === 'ArrowRight') { inQuadrant = p[0] > start[0] && dy < dx } + else { inQuadrant = true } return inQuadrant && JSON.stringify(p) !== JSON.stringify(start) }) - if (!quadrant.length) quadrant.push(start) + if (!quadrant.length) { quadrant.push(start) } const dist = (p) => Math.sqrt((start[0] - p[0]) ** 2 + (start[1] - p[1]) ** 2) const closest = quadrant.reduce((best, p) => dist(p) < dist(best) ? p : best) From f79d06d3eb8b29ad4a4fcfdc10c76206c8348059 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 30 Jun 2026 15:02:18 +0100 Subject: [PATCH 5/7] Delete vertex button disable fix --- plugins/beta/draw-ol/src/manifest.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/beta/draw-ol/src/manifest.js b/plugins/beta/draw-ol/src/manifest.js index 92ab136e..e5bcb16b 100644 --- a/plugins/beta/draw-ol/src/manifest.js +++ b/plugins/beta/draw-ol/src/manifest.js @@ -81,7 +81,11 @@ export const manifest = { id: 'drawDeletePoint', label: 'Delete point', iconId: 'trash', - enableWhen: ({ pluginState }) => pluginState.selectedVertexIndex >= 0 && pluginState.numVertices > 2, + enableWhen: ({ pluginState }) => { + if (pluginState.selectedVertexIndex < 0) { return false } + const isPolygon = pluginState.feature?.geometry?.type === 'Polygon' + return isPolygon ? pluginState.numVertices > 3 : pluginState.numVertices > 2 // NOSONAR + }, hiddenWhen: ({ pluginState }) => pluginState.mode !== 'edit_vertex' }], mobile: { slot: 'bottom-right' }, From 7e7b9e7bc228837e29c738e2a5e7754c02a949f9 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Tue, 30 Jun 2026 15:05:52 +0100 Subject: [PATCH 6/7] State vertecies reset when changing draw mode --- plugins/beta/draw-ol/src/reducer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/beta/draw-ol/src/reducer.js b/plugins/beta/draw-ol/src/reducer.js index 57afea18..52af3753 100644 --- a/plugins/beta/draw-ol/src/reducer.js +++ b/plugins/beta/draw-ol/src/reducer.js @@ -9,7 +9,7 @@ const initialState = { hasSnapLayers: false } -const setMode = (state, payload) => ({ ...state, mode: payload }) +const setMode = (state, payload) => ({ ...state, mode: payload, numVertices: null }) const setFeature = (state, payload) => ({ ...state, From 6aad91a55c6cf8b809b12f3eb41afc912e4c34f1 Mon Sep 17 00:00:00 2001 From: Dan Leech Date: Wed, 1 Jul 2026 09:47:09 +0100 Subject: [PATCH 7/7] Sonar cloud fixes --- plugins/beta/draw-ol/src/utils/spatial.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/plugins/beta/draw-ol/src/utils/spatial.js b/plugins/beta/draw-ol/src/utils/spatial.js index 35be1d0c..b9238b6a 100644 --- a/plugins/beta/draw-ol/src/utils/spatial.js +++ b/plugins/beta/draw-ol/src/utils/spatial.js @@ -12,17 +12,13 @@ export const spatialNavigate = (start, pixels, direction) => { const dx = Math.abs(p[0] - start[0]) const dy = Math.abs(p[1] - start[1]) let inQuadrant = false - if (direction === 'ArrowUp') { inQuadrant = p[1] <= start[1] && dy >= dx } - else if (direction === 'ArrowDown') { inQuadrant = p[1] > start[1] && dy >= dx } - else if (direction === 'ArrowLeft') { inQuadrant = p[0] <= start[0] && dy < dx } - else if (direction === 'ArrowRight') { inQuadrant = p[0] > start[0] && dy < dx } - else { inQuadrant = true } + if (direction === 'ArrowUp') { inQuadrant = p[1] <= start[1] && dy >= dx } else if (direction === 'ArrowDown') { inQuadrant = p[1] > start[1] && dy >= dx } else if (direction === 'ArrowLeft') { inQuadrant = p[0] <= start[0] && dy < dx } else if (direction === 'ArrowRight') { inQuadrant = p[0] > start[0] && dy < dx } else { inQuadrant = true } return inQuadrant && JSON.stringify(p) !== JSON.stringify(start) }) if (!quadrant.length) { quadrant.push(start) } - const dist = (p) => Math.sqrt((start[0] - p[0]) ** 2 + (start[1] - p[1]) ** 2) - const closest = quadrant.reduce((best, p) => dist(p) < dist(best) ? p : best) + const dist = (p) => Math.hypot(start[0] - p[0], start[1] - p[1]) + const closest = quadrant.reduce((best, p) => dist(p) < dist(best) ? p : best, quadrant[0]) return pixels.findIndex(p => JSON.stringify(p) === JSON.stringify(closest)) }