Merge pull request #11769 from ahocevar/modify-dx

Modify interaction developer experience improvements

examples/modify-icon.html

@@ -0,0 +1,10 @@
+---
+layout: example.html
+title: Icon modification
+shortdesc: Example using a Modify interaction to edit an icon.
+docs: >
+  The icon on this map can be dragged to modify its location.
+  <p>The Modify interaction can be configured with a `layer` option. With this option, hit detection will be used to determine the modification candidate.</p>
+tags: "vector, modify, icon, marker"
+---
+<div id="map" class="map"></div>

examples/modify-icon.js

@@ -0,0 +1,66 @@
+import Feature from '../src/ol/Feature.js';
+import Map from '../src/ol/Map.js';
+import Point from '../src/ol/geom/Point.js';
+import TileJSON from '../src/ol/source/TileJSON.js';
+import VectorSource from '../src/ol/source/Vector.js';
+import View from '../src/ol/View.js';
+import {Icon, Style} from '../src/ol/style.js';
+import {Modify} from '../src/ol/interaction.js';
+import {Tile as TileLayer, Vector as VectorLayer} from '../src/ol/layer.js';
+
+const iconFeature = new Feature({
+  geometry: new Point([0, 0]),
+  name: 'Null Island',
+  population: 4000,
+  rainfall: 500,
+});
+
+const iconStyle = new Style({
+  image: new Icon({
+    anchor: [0.5, 46],
+    anchorXUnits: 'fraction',
+    anchorYUnits: 'pixels',
+    src: 'data/icon.png',
+  }),
+});
+
+iconFeature.setStyle(iconStyle);
+
+const vectorSource = new VectorSource({
+  features: [iconFeature],
+});
+
+const vectorLayer = new VectorLayer({
+  source: vectorSource,
+});
+
+const rasterLayer = new TileLayer({
+  source: new TileJSON({
+    url: 'https://a.tiles.mapbox.com/v3/aj.1x1-degrees.json?secure=1',
+    crossOrigin: '',
+  }),
+});
+
+const target = document.getElementById('map');
+const map = new Map({
+  layers: [rasterLayer, vectorLayer],
+  target: target,
+  view: new View({
+    center: [0, 0],
+    zoom: 3,
+  }),
+});
+
+const modify = new Modify({
+  hitDetection: vectorLayer,
+  source: vectorSource,
+});
+modify.on(['modifystart', 'modifyend'], function (evt) {
+  target.style.cursor = evt.type === 'modifystart' ? 'grabbing' : 'pointer';
+});
+const overlaySource = modify.getOverlay().getSource();
+overlaySource.on(['addfeature', 'removefeature'], function (evt) {
+  target.style.cursor = evt.type === 'addfeature' ? 'pointer' : '';
+});
+
+map.addInteraction(modify);

src/ol/interaction/Modify.js

@@ -108,14 +108,24 @@ const ModifyEventType = {
  * @property {number} [pixelTolerance=10] Pixel tolerance for considering the
  * pointer close enough to a segment or vertex for editing.
  * @property {import("../style/Style.js").StyleLike} [style]
- * Style used for the features being modified. By default the default edit
+ * Style used for the modification point or vertex. For linestrings and polygons, this will
- * style is used (see {@link module:ol/style}).
+ * be the affected vertex, for circles a point along the circle, and for points the actual
+ * point. If not configured, the default edit style is used (see {@link module:ol/style}).
+ * When using a style function, the point feature passed to the function will have a `features`
+ * property - an array whose entries are the features that are being modified, and a `geometries`
+ * property - an array whose entries are the geometries that are being modified. Both arrays are
+ * in the same order. The `geometries` are only useful when modifying geometry collections, where
+ * the geometry will be the particular geometry from the collection that is being modified.
  * @property {VectorSource} [source] The vector source with
  * features to modify.  If a vector source is not provided, a feature collection
- * must be provided with the features option.
+ * must be provided with the `features` option.
+ * @property {boolean|import("../layer/BaseVector").default} [hitDetection] When configured, point
+ * features will be considered for modification based on their visual appearance, instead of being within
+ * the `pixelTolerance` from the pointer location. When a {@link module:ol/layer/BaseVector} is
+ * provided, only the rendered representation of the features on that layer will be considered.
  * @property {Collection<Feature>} [features]
  * The features the interaction works on.  If a feature collection is not
- * provided, a vector source must be provided with the source option.
+ * provided, a vector source must be provided with the `source` option.
  * @property {boolean} [wrapX=false] Wrap the world horizontally on the sketch
  * overlay.
  */
@@ -161,6 +171,12 @@ export class ModifyEvent extends Event {
  * the `features` option.  The interaction must be constructed with either a
  * `source` or `features` option.
  *
+ * Cartesian distance from the pointer is used to determine the features that
+ * will be modified. This means that geometries will only be considered for
+ * modification when they are within the configured `pixelTolerane`. For point
+ * geometries, the `hitDetection` option can be used to match their visual
+ * appearance.
+ *
  * By default, the interaction will allow deletion of vertices when the `alt`
  * key is pressed.  To configure the interaction with a different condition
  * for deletion, use the `deleteCondition` option.
@@ -237,10 +253,10 @@ class Modify extends PointerInteraction {
     this.ignoreNextSingleClick_ = false;
 
     /**
-     * @type {boolean}
+     * @type {Collection<Feature>}
      * @private
      */
-    this.modified_ = false;
+    this.featuresBeingModified_ = null;
 
     /**
      * Segment RTree for each layer
@@ -314,8 +330,15 @@ class Modify extends PointerInteraction {
      */
     this.source_ = null;
 
+    /**
+     * @type {boolean|import("../layer/BaseVector").default}
+     */
+    this.hitDetection_ = null;
+
     let features;
-    if (options.source) {
+    if (options.features) {
+      features = options.features;
+    } else if (options.source) {
       this.source_ = options.source;
       features = new Collection(this.source_.getFeatures());
       this.source_.addEventListener(
@@ -326,15 +349,18 @@ class Modify extends PointerInteraction {
         VectorEventType.REMOVEFEATURE,
         this.handleSourceRemove_.bind(this)
       );
-    } else {
-      features = options.features;
     }
     if (!features) {
-      throw new Error('The modify interaction requires features or a source');
+      throw new Error(
+        'The modify interaction requires features, a source or a layer'
+      );
+    }
+    if (options.hitDetection) {
+      this.hitDetection_ = options.hitDetection;
     }
 
     /**
-     * @type {Collection<Feature>}
+     * @type {Collection<import("../Feature.js").FeatureLike>}
      * @private
      */
     this.features_ = features;
@@ -354,6 +380,12 @@ class Modify extends PointerInteraction {
      * @private
      */
     this.lastPointerEvent_ = null;
+
+    /**
+     * Delta (x, y in map units) between matched rtree vertex and pointer vertex.
+     * @type {Array<number>}
+     */
+    this.delta_ = [0, 0];
   }
 
   /**
@@ -376,14 +408,27 @@ class Modify extends PointerInteraction {
   }
 
   /**
-   * @param {import("../MapBrowserEvent.js").default} evt Map browser event
+   * @param {import("../MapBrowserEvent.js").default} evt Map browser event.
+   * @param {Array<Array<SegmentData>>} segments The segments subject to modification.
    * @private
    */
-  willModifyFeatures_(evt) {
+  willModifyFeatures_(evt, segments) {
-    if (!this.modified_) {
+    if (!this.featuresBeingModified_) {
-      this.modified_ = true;
+      this.featuresBeingModified_ = new Collection();
+      const features = this.featuresBeingModified_.getArray();
+      for (let i = 0, ii = segments.length; i < ii; ++i) {
+        const feature = segments[i][0].feature;
+        if (features.indexOf(feature) === -1) {
+          this.featuresBeingModified_.push(feature);
+        }
+      }
+
       this.dispatchEvent(
-        new ModifyEvent(ModifyEventType.MODIFYSTART, this.features_, evt)
+        new ModifyEvent(
+          ModifyEventType.MODIFYSTART,
+          this.featuresBeingModified_,
+          evt
+        )
       );
     }
   }
@@ -460,7 +505,7 @@ class Modify extends PointerInteraction {
   }
 
   /**
-   * Get the overlay layer that this interaction renders sketch features to.
+   * Get the overlay layer that this interaction renders the modification point or vertex to.
    * @return {VectorLayer} Overlay layer.
    * @api
    */
@@ -725,10 +770,12 @@ class Modify extends PointerInteraction {
 
   /**
    * @param {import("../coordinate.js").Coordinate} coordinates Coordinates.
+   * @param {Array<Feature>} features The features being modified.
+   * @param {Array<import("../geom/SimpleGeometry.js").default>} geometries The geometries being modified.
    * @return {Feature} Vertex feature.
    * @private
    */
-  createOrUpdateVertexFeature_(coordinates) {
+  createOrUpdateVertexFeature_(coordinates, features, geometries) {
     let vertexFeature = this.vertexFeature_;
     if (!vertexFeature) {
       vertexFeature = new Feature(new Point(coordinates));
@@ -738,6 +785,8 @@ class Modify extends PointerInteraction {
       const geometry = vertexFeature.getGeometry();
       geometry.setCoordinates(coordinates);
     }
+    vertexFeature.set('features', features);
+    vertexFeature.set('geometries', geometries);
     return vertexFeature;
   }
 
@@ -784,14 +833,26 @@ class Modify extends PointerInteraction {
    */
   handleDragEvent(evt) {
     this.ignoreNextSingleClick_ = false;
-    this.willModifyFeatures_(evt);
+    this.willModifyFeatures_(evt, this.dragSegments_);
 
-    const vertex = evt.coordinate;
+    const vertex = [
+      evt.coordinate[0] + this.delta_[0],
+      evt.coordinate[1] + this.delta_[1],
+    ];
+    const features = [];
+    const geometries = [];
     for (let i = 0, ii = this.dragSegments_.length; i < ii; ++i) {
       const dragSegment = this.dragSegments_[i];
       const segmentData = dragSegment[0];
-      const depth = segmentData.depth;
+      const feature = segmentData.feature;
+      if (features.indexOf(feature) === -1) {
+        features.push(feature);
+      }
       const geometry = segmentData.geometry;
+      if (geometries.indexOf(geometry) === -1) {
+        geometries.push(geometry);
+      }
+      const depth = segmentData.depth;
       let coordinates;
       const segment = segmentData.segment;
       const index = dragSegment[1];
@@ -869,7 +930,7 @@ class Modify extends PointerInteraction {
         this.setGeometryCoordinates_(geometry, coordinates);
       }
     }
-    this.createOrUpdateVertexFeature_(vertex);
+    this.createOrUpdateVertexFeature_(vertex, features, geometries);
   }
 
   /**
@@ -884,7 +945,7 @@ class Modify extends PointerInteraction {
     const pixelCoordinate = evt.coordinate;
     this.handlePointerAtPixel_(evt.pixel, evt.map, pixelCoordinate);
     this.dragSegments_.length = 0;
-    this.modified_ = false;
+    this.featuresBeingModified_ = null;
     const vertexFeature = this.vertexFeature_;
     if (vertexFeature) {
       const projection = evt.map.getView().getProjection();
@@ -965,7 +1026,7 @@ class Modify extends PointerInteraction {
       }
 
       if (insertVertices.length) {
-        this.willModifyFeatures_(evt);
+        this.willModifyFeatures_(evt, [insertVertices]);
       }
 
       for (let j = insertVertices.length - 1; j >= 0; --j) {
@@ -1014,11 +1075,15 @@ class Modify extends PointerInteraction {
         this.rBush_.update(boundingExtent(segmentData.segment), segmentData);
       }
     }
-    if (this.modified_) {
+    if (this.featuresBeingModified_) {
       this.dispatchEvent(
-        new ModifyEvent(ModifyEventType.MODIFYEND, this.features_, evt)
+        new ModifyEvent(
+          ModifyEventType.MODIFYEND,
+          this.featuresBeingModified_,
+          evt
+        )
       );
-      this.modified_ = false;
+      this.featuresBeingModified_ = null;
     }
     return false;
   }
@@ -1048,36 +1113,68 @@ class Modify extends PointerInteraction {
       );
     };
 
-    const viewExtent = fromUserExtent(
+    let nodes, hitPointGeometry;
-      createExtent(pixelCoordinate, tempExtent),
+    if (this.hitDetection_) {
-      projection
+      const layerFilter =
-    );
+        typeof this.hitDetection_ === 'object'
-    const buffer = map.getView().getResolution() * this.pixelTolerance_;
+          ? (layer) => layer === this.hitDetection_
-    const box = toUserExtent(
+          : undefined;
-      bufferExtent(viewExtent, buffer, tempExtent),
+      map.forEachFeatureAtPixel(
-      projection
+        pixel,
-    );
+        (feature, layer, geometry) => {
+          geometry = geometry || feature.getGeometry();
+          if (geometry.getType() === GeometryType.POINT) {
+            hitPointGeometry = geometry;
+            const coordinate = geometry.getCoordinates();
+            nodes = [
+              {
+                feature,
+                geometry,
+                segment: [coordinate, coordinate],
+              },
+            ];
+          }
+          return true;
+        },
+        {layerFilter}
+      );
+    }
+    if (!nodes) {
+      const viewExtent = fromUserExtent(
+        createExtent(pixelCoordinate, tempExtent),
+        projection
+      );
+      const buffer = map.getView().getResolution() * this.pixelTolerance_;
+      const box = toUserExtent(
+        bufferExtent(viewExtent, buffer, tempExtent),
+        projection
+      );
+      nodes = this.rBush_.getInExtent(box);
+    }
 
-    const rBush = this.rBush_;
+    if (nodes && nodes.length > 0) {
-    const nodes = rBush.getInExtent(box);
+      const node = nodes.sort(sortByDistance)[0];
-    if (nodes.length > 0) {
-      nodes.sort(sortByDistance);
-      const node = nodes[0];
       const closestSegment = node.segment;
       let vertex = closestOnSegmentData(pixelCoordinate, node, projection);
       const vertexPixel = map.getPixelFromCoordinate(vertex);
       let dist = coordinateDistance(pixel, vertexPixel);
-      if (dist <= this.pixelTolerance_) {
+      if (hitPointGeometry || dist <= this.pixelTolerance_) {
         /** @type {Object<string, boolean>} */
         const vertexSegments = {};
         vertexSegments[getUid(closestSegment)] = true;
 
+        this.delta_[0] = vertex[0] - pixelCoordinate[0];
+        this.delta_[1] = vertex[1] - pixelCoordinate[1];
         if (
           node.geometry.getType() === GeometryType.CIRCLE &&
           node.index === CIRCLE_CIRCUMFERENCE_INDEX
         ) {
           this.snappedToVertex_ = true;
-          this.createOrUpdateVertexFeature_(vertex);
+          this.createOrUpdateVertexFeature_(
+            vertex,
+            [node.feature],
+            [node.geometry]
+          );
         } else {
           const pixel1 = map.getPixelFromCoordinate(closestSegment[0]);
           const pixel2 = map.getPixelFromCoordinate(closestSegment[1]);
@@ -1091,7 +1188,11 @@ class Modify extends PointerInteraction {
                 ? closestSegment[1]
                 : closestSegment[0];
           }
-          this.createOrUpdateVertexFeature_(vertex);
+          this.createOrUpdateVertexFeature_(
+            vertex,
+            [node.feature],
+            [node.geometry]
+          );
           const geometries = {};
           geometries[getUid(node.geometry)] = true;
           for (let i = 1, ii = nodes.length; i < ii; ++i) {
@@ -1203,12 +1304,16 @@ class Modify extends PointerInteraction {
       this.lastPointerEvent_.type != MapBrowserEventType.POINTERDRAG
     ) {
       const evt = this.lastPointerEvent_;
-      this.willModifyFeatures_(evt);
+      this.willModifyFeatures_(evt, this.dragSegments_);
       const removed = this.removeVertex_();
       this.dispatchEvent(
-        new ModifyEvent(ModifyEventType.MODIFYEND, this.features_, evt)
+        new ModifyEvent(
+          ModifyEventType.MODIFYEND,
+          this.featuresBeingModified_,
+          evt
+        )
       );
-      this.modified_ = false;
+      this.featuresBeingModified_ = null;
       return removed;
     }
     return false;

test/spec/ol/interaction/modify.test.js

@@ -1,4 +1,5 @@
 import Circle from '../../../../src/ol/geom/Circle.js';
+import CircleStyle from '../../../../src/ol/style/Circle.js';
 import Collection from '../../../../src/ol/Collection.js';
 import Event from '../../../../src/ol/events/Event.js';
 import Feature from '../../../../src/ol/Feature.js';
@@ -8,11 +9,13 @@ import Map from '../../../../src/ol/Map.js';
 import MapBrowserEvent from '../../../../src/ol/MapBrowserEvent.js';
 import Modify, {ModifyEvent} from '../../../../src/ol/interaction/Modify.js';
 import Point from '../../../../src/ol/geom/Point.js';
-import Polygon from '../../../../src/ol/geom/Polygon.js';
+import Polygon, {fromExtent} from '../../../../src/ol/geom/Polygon.js';
 import Snap from '../../../../src/ol/interaction/Snap.js';
 import VectorLayer from '../../../../src/ol/layer/Vector.js';
 import VectorSource from '../../../../src/ol/source/Vector.js';
 import View from '../../../../src/ol/View.js';
+import {Fill, Style} from '../../../../src/ol/style.js';
+import {MultiPoint} from '../../../../src/ol/geom.js';
 import {
   clearUserProjection,
   setUserProjection,
@@ -21,7 +24,7 @@ import {doubleClick} from '../../../../src/ol/events/condition.js';
 import {getValues} from '../../../../src/ol/obj.js';
 
 describe('ol.interaction.Modify', function () {
-  let target, map, source, features;
+  let target, map, layer, source, features;
 
   const width = 360;
   const height = 180;
@@ -55,7 +58,7 @@ describe('ol.interaction.Modify', function () {
       features: features,
     });
 
-    const layer = new VectorLayer({source: source});
+    layer = new VectorLayer({source: source});
 
     map = new Map({
       target: target,
@@ -194,6 +197,17 @@ describe('ol.interaction.Modify', function () {
       expect(rbushEntries.length).to.be(1);
       expect(rbushEntries[0].feature).to.be(feature);
     });
+
+    it('accepts a hitDetection option', function () {
+      const feature = new Feature(new Point([0, 0]));
+      const source = new VectorSource({features: [feature]});
+      const layer = new VectorLayer({source: source});
+      const modify = new Modify({hitDetection: layer, source: source});
+      const rbushEntries = modify.rBush_.getAll();
+      expect(rbushEntries.length).to.be(1);
+      expect(rbushEntries[0].feature).to.be(feature);
+      expect(modify.hitDetection_).to.be(layer);
+    });
   });
 
   describe('vertex deletion', function () {
@@ -484,7 +498,7 @@ describe('ol.interaction.Modify', function () {
       simulateEvent('pointerdrag', 30, -5, null, 0);
       simulateEvent('pointerup', 30, -5, null, 0);
 
-      expect(circleFeature.getGeometry().getRadius()).to.equal(25);
+      expect(circleFeature.getGeometry().getRadius()).to.roughlyEqual(25, 0.1);
       expect(circleFeature.getGeometry().getCenter()).to.eql([5, 5]);
 
       // Increase radius along y axis
@@ -494,7 +508,7 @@ describe('ol.interaction.Modify', function () {
       simulateEvent('pointerdrag', 5, -35, null, 0);
       simulateEvent('pointerup', 5, -35, null, 0);
 
-      expect(circleFeature.getGeometry().getRadius()).to.equal(30);
+      expect(circleFeature.getGeometry().getRadius()).to.roughlyEqual(30, 0.1);
       expect(circleFeature.getGeometry().getCenter()).to.eql([5, 5]);
     });
 
@@ -539,7 +553,7 @@ describe('ol.interaction.Modify', function () {
         .getGeometry()
         .clone()
         .transform(userProjection, viewProjection);
-      expect(geometry2.getRadius()).to.roughlyEqual(25, 1e-9);
+      expect(geometry2.getRadius()).to.roughlyEqual(25, 0.1);
       expect(geometry2.getCenter()).to.eql([5, 5]);
 
       // Increase radius along y axis
@@ -553,7 +567,7 @@ describe('ol.interaction.Modify', function () {
         .getGeometry()
         .clone()
         .transform(userProjection, viewProjection);
-      expect(geometry3.getRadius()).to.roughlyEqual(30, 1e-9);
+      expect(geometry3.getRadius()).to.roughlyEqual(30, 0.1);
       expect(geometry3.getCenter()).to.eql([5, 5]);
     });
   });
@@ -562,6 +576,7 @@ describe('ol.interaction.Modify', function () {
     let modify, feature, events;
 
     beforeEach(function () {
+      features.push(new Feature(new Point([12, 34])));
       modify = new Modify({
         features: new Collection(features),
       });
@@ -941,19 +956,88 @@ describe('ol.interaction.Modify', function () {
     });
   });
 
-  describe('#setActive', function () {
+  describe('Vertex feature', function () {
-    it('removes the vertexFeature of deactivation', function () {
+    it('tracks features and geometries and removes the vertexFeature on deactivation', function () {
+      const collection = new Collection(features);
       const modify = new Modify({
-        features: new Collection(features),
+        features: collection,
       });
       map.addInteraction(modify);
       expect(modify.vertexFeature_).to.be(null);
 
       simulateEvent('pointermove', 10, -20, null, 0);
       expect(modify.vertexFeature_).to.not.be(null);
+      expect(modify.vertexFeature_.get('features').length).to.be(1);
+      expect(modify.vertexFeature_.get('geometries').length).to.be(1);
 
       modify.setActive(false);
       expect(modify.vertexFeature_).to.be(null);
+      map.removeInteraction(modify);
+    });
+
+    it('tracks features and geometries - multi geometry', function () {
+      const collection = new Collection();
+      const modify = new Modify({
+        features: collection,
+      });
+      map.addInteraction(modify);
+      const feature = new Feature(
+        new MultiPoint([
+          [10, 10],
+          [10, 20],
+        ])
+      );
+      collection.push(feature);
+      simulateEvent('pointermove', 10, -20, null, 0);
+      expect(modify.vertexFeature_.get('features')[0]).to.eql(feature);
+      expect(modify.vertexFeature_.get('geometries')[0]).to.eql(
+        feature.getGeometry()
+      );
+      map.removeInteraction(modify);
+    });
+
+    it('tracks features and geometries - geometry collection', function () {
+      const collection = new Collection();
+      const modify = new Modify({
+        features: collection,
+      });
+      map.addInteraction(modify);
+      const feature = new Feature(
+        new GeometryCollection([fromExtent([0, 0, 10, 10]), new Point([5, 5])])
+      );
+      collection.push(feature);
+      simulateEvent('pointermove', 5, -5, null, 0);
+      expect(modify.vertexFeature_.get('features')[0]).to.eql(feature);
+      expect(modify.vertexFeature_.get('geometries')[0]).to.eql(
+        feature.getGeometry().getGeometriesArray()[1]
+      );
+    });
+
+    it('works with hit detection of point features', function () {
+      const modify = new Modify({
+        hitDetection: layer,
+        source: source,
+      });
+      map.addInteraction(modify);
+      source.clear();
+      const pointFeature = new Feature(new Point([0, 0]));
+      source.addFeature(pointFeature);
+      layer.setStyle(
+        new Style({
+          image: new CircleStyle({
+            radius: 30,
+            fill: new Fill({
+              color: 'fuchsia',
+            }),
+          }),
+        })
+      );
+      map.renderSync();
+      simulateEvent('pointermove', 10, -10, null, 0);
+      expect(modify.vertexFeature_.get('features')[0]).to.eql(pointFeature);
+      expect(modify.vertexFeature_.get('geometries')[0]).to.eql(
+        pointFeature.getGeometry()
+      );
     });
   });
 
@@ -1000,7 +1084,7 @@ describe('ol.interaction.Modify', function () {
       simulateEvent('pointerdrag', 30, -5, null, 0);
       simulateEvent('pointerup', 30, -5, null, 0);
 
-      expect(circleFeature.getGeometry().getRadius()).to.equal(25);
+      expect(circleFeature.getGeometry().getRadius()).to.roughlyEqual(25, 1e-9);
       expect(circleFeature.getGeometry().getCenter()).to.eql([5, 5]);
 
       // Increase radius along y axis