fix(engine): a new feature doesn't take a deleted feature's locked label

crates/engine/src/lib.rs

@@ -769,6 +769,37 @@ fn fresh_source_ids(before: &Project, after: &mut Project) {
     }
 }
 
+/// A lock names each label by its feature's id, and [`rustgis_data::Dataset::push`] hands the
+/// largest id out again once its feature is deleted: the new feature would take the lock's place
+/// without having been placed (#1135). So a command that removes features removes their entries,
+/// whichever way it removes them. Undo brings them back with the rest of the project. A streamed
+/// layer holds only the window in view, so its other features aren't gone. It compares ids, so an
+/// id a single command both removes and hands out stays locked: only restoring from an archive can.
+fn forget_locks_of_removed_features(before: &Project, after: &mut Project) {
+    for (i, m) in after.maps.iter_mut().enumerate() {
+        let was = before.maps.get(i).filter(|o| o.name == m.name);
+        let Some(lock) = m.label_lock.as_mut() else { continue };
+        for l in m.layers.iter().filter(|l| !l.is_streamed()) {
+            if was.and_then(|o| o.layers.iter().find(|o| o.id == l.id)).is_some_and(|o| o.dataset.features.shares(&l.dataset.features)) {
+                continue;
+            }
+            // The lock's ids for this layer, less those it still has: sized by the lock, not the layer.
+            let mut gone: std::collections::HashSet<usize> =
+                lock.iter().filter(|((layer, ..), _)| *layer == l.id).map(|((_, id, ..), _)| *id).collect();
+            if gone.is_empty() {
+                continue;
+            }
+            for f in l.dataset.features.iter() {
+                gone.remove(&(f.id as usize));
+                if gone.is_empty() {
+                    break;
+                }
+            }
+            lock.retain(|((layer, id, ..), _)| *layer != l.id || !gone.contains(id));
+        }
+    }
+}
+
 /// Why command `label` can't edit streamed layer `layer`, refused up front or undone after.
 fn streamed_refusal(label: &str, layer: &str) -> String {
     format!("{label}: {layer} is read in place from its file; Load All Features (layer.loadAll) to edit it")
@@ -1180,6 +1211,7 @@ impl Session {
             (Ok(_), Some(_)) if std::mem::take(&mut self.request_only) => {}
             (Ok(_), Some(mut snap)) => {
                 fresh_source_ids(&snap, &mut self.project);
+                forget_locks_of_removed_features(&snap, &mut self.project);
                 // An edit may change the dates the time slider's window filters on: bound layers
                 // whose features or fields the command changed (features are shared with the
                 // snapshot until written) take the window again.

crates/engine/src/tests.rs

@@ -2759,6 +2759,73 @@ fn locked_labels_stay_with_their_features_when_rows_move() {
     assert_eq!(draw(&mut s), ["Beta"], "and is locked in turn");
 }
 
+/// A feature added after locking has no label, even when it takes the id of a feature deleted
+/// since: the lock's entry for that id goes with the feature, and undo brings it back (#1135).
+#[test]
+fn a_new_feature_doesnt_take_the_locked_label_of_a_deleted_one() {
+    use crate::render::View;
+    use rustgis_data::{Dataset, Field, FieldType, Geometry, GeometryType, Value as V};
+    let mut s = world();
+    let mut d = Dataset::new("Places", GeometryType::Point, vec![Field::new("name", FieldType::Text)]);
+    for (name, lon) in [("Alpha", -30.0), ("Gamma", 30.0)] {
+        d.push(Some(Geometry::Point(rustgis_geom::Coord::new(lon, -40.0))), vec![V::Text(name.into())]);
+    }
+    let id = s.map_mut().add_dataset(d);
+    let gamma = s.map().layer(id).unwrap().dataset.features[1].id;
+    s.run("labels.enable", &json!({"layer": "Places", "on": true})).unwrap();
+    let draw = |s: &mut Session| -> Vec<String> {
+        let v = View { width: 800.0, height: 600.0, pixels_per_point: 1.0, camera: s.map().camera, sky: false };
+        let labels = crate::render::render(s.map_mut(), &v).unwrap().labels;
+        let mut texts: Vec<String> =
+            labels.into_iter().map(|l| l.text).filter(|t| ["Alpha", "Alpha 2", "Gamma", "Delta"].contains(&t.as_str())).collect();
+        texts.sort();
+        texts
+    };
+    s.run("labels.lock", &json!({"on": true})).unwrap();
+    assert_eq!(draw(&mut s), ["Alpha", "Gamma"]);
+
+    // An edit keeps the feature's id, so its label stays locked.
+    s.run("edit.setValue", &json!({"layer": "Places", "row": 0, "field": "name", "value": "Alpha 2"})).unwrap();
+    assert_eq!(draw(&mut s), ["Alpha 2", "Gamma"]);
+
+    s.map_mut().layer_mut(id).unwrap().selection = vec![1];
+    s.run("edit.delete", &Value::Null).unwrap();
+    let locked =
+        |s: &Session| s.map().label_lock.as_ref().is_some_and(|k| k.iter().any(|((l, f, ..), _)| *l == id && *f == gamma as usize));
+    assert!(!locked(&s), "the lock's entry goes with the feature");
+    let delta = json!({"layer": "Places", "geometry": {"type": "Point", "coordinates": [0.0, -40.0]}, "attributes": {"name": "Delta"}});
+    s.run("edit.addFeature", &delta).unwrap();
+    assert_eq!(s.map().layer(id).unwrap().dataset.features[1].id, gamma, "the new feature takes the deleted one's id");
+    assert_eq!(draw(&mut s), ["Alpha 2"], "and no label, as it wasn't placed when the lock was taken");
+
+    s.run("edit.undo", &Value::Null).unwrap();
+    s.run("edit.undo", &Value::Null).unwrap();
+    assert!(locked(&s), "undoing the delete brings the entry back");
+    assert_eq!(draw(&mut s), ["Alpha 2", "Gamma"]);
+
+    s.run("edit.redo", &Value::Null).unwrap();
+    assert!(!locked(&s), "redoing the delete takes it away again");
+    assert_eq!(draw(&mut s), ["Alpha 2"]);
+}
+
+/// A streamed layer holds only the window in view: a lock's entry for a feature outside it stays
+/// when the window changes (#1135).
+#[test]
+fn a_lock_holds_for_a_streamed_layers_features_outside_its_window() {
+    let p = line_of_points_shp("locked-window", 20);
+    let mut s = world();
+    let r = s.run("layer.addData", &json!({"path": p.display().to_string(), "stream": true})).unwrap();
+    let id = r["layer"].as_u64().unwrap() as u32;
+    s.map_mut().label_lock = Some(vec![((id, 1, 0, 0), Default::default())]);
+    let before = s.project.clone();
+    // The window moves off the feature: no longer shared with the copy, and without its id.
+    s.map_mut().layer_mut(id).unwrap().dataset.features = Vec::new().into();
+    crate::forget_locks_of_removed_features(&before, &mut s.project);
+    assert_eq!(s.map().label_lock.as_ref().map(Vec::len), Some(1));
+    drop(s);
+    remove_shp(&p);
+}
+
 /// A lock names each label by its class's place in the list: a change to the list (a class added,
 /// moved or removed) locks the labels again on the next frame, a change within a class keeps the
 /// lock (#732).
@@ -15249,9 +15316,7 @@ fn emptying_the_index_cache_keeps_indexes_in_use() {
     let layer = s.map().layer(r["layer"].as_u64().unwrap() as u32).unwrap();
     assert_eq!(layer.load_all().unwrap().features.len(), 20);
     drop(s);
-    for ext in ["shp", "shx", "dbf", "prj", "cpg"] {
-        let _ = std::fs::remove_file(p.with_extension(ext));
-    }
+    remove_shp(&p);
 }
 
 /// Emptying the index cache tells the user only what it couldn't do: nothing when all went (#1259).
@@ -15286,6 +15351,13 @@ fn line_of_points_shp(name: &str, n: usize) -> std::path::PathBuf {
     p
 }
 
+/// Removes a Shapefile written by [`line_of_points_shp`] with its sidecar files.
+fn remove_shp(p: &std::path::Path) {
+    for ext in ["shp", "shx", "dbf", "prj", "cpg"] {
+        let _ = std::fs::remove_file(p.with_extension(ext));
+    }
+}
+
 #[test]
 fn shapefiles_too_large_to_load_whole_still_open() {
     // Few records, but a .shp or .dbf past what the whole-file reader takes (#1055): opened and
@@ -15307,9 +15379,7 @@ fn shapefiles_too_large_to_load_whole_still_open() {
         let mut s = Session::new(Project::default());
         let e = s.run("layer.addData", &json!({"path": path, "stream": false})).unwrap_err();
         assert!(e.contains("too large"), "{grown}: {e}");
-        for ext in ["shp", "shx", "dbf", "prj", "cpg"] {
-            let _ = std::fs::remove_file(p.with_extension(ext));
-        }
+        remove_shp(&p);
     }
 }
 
@@ -15353,9 +15423,7 @@ fn large_shapefiles_are_read_in_place() {
     let r = u.run("layer.addData", &json!({"path": big.display().to_string(), "stream": false})).unwrap();
     assert!(!u.map().layer(r["layer"].as_u64().unwrap() as u32).unwrap().is_streamed());
     for p in [small, big] {
-        for ext in ["shp", "shx", "dbf", "prj", "cpg"] {
-            let _ = std::fs::remove_file(p.with_extension(ext));
-        }
+        remove_shp(&p);
     }
 }