diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e07f0ac47..73dd1d920 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -784,13 +784,13 @@ fn forget_locks_of_removed_features(before: &Project, after: &mut Project) { 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 = + let mut gone: std::collections::HashSet = 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)); + gone.remove(&f.id); if gone.is_empty() { break; } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e97b917bf..70d97d755 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2770,6 +2770,9 @@ fn a_new_feature_doesnt_take_the_locked_label_of_a_deleted_one() { 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())]); } + // The full source id must survive lock placement, pruning and undo on wasm32 too. + d.features[0].id = 1_u64 << 32; + d.features[1].id = (1_u64 << 32) + 1; 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(); @@ -2790,8 +2793,7 @@ fn a_new_feature_doesnt_take_the_locked_label_of_a_deleted_one() { 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)); + let locked = |s: &Session| s.map().label_lock.as_ref().is_some_and(|k| k.iter().any(|((l, f, ..), _)| *l == id && *f == gamma)); 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(); diff --git a/crates/map/src/symbology.rs b/crates/map/src/symbology.rs index c9bdf0a43..87f9d69c2 100644 --- a/crates/map/src/symbology.rs +++ b/crates/map/src/symbology.rs @@ -2349,8 +2349,9 @@ impl LabelClass { /// A label on the map, as (layer id, feature id, label class in `Labeling::in_order`, repeat): a /// feature has a label for each class labelling it (#732), and along a line each repeat is one of /// its own (0 for the one at the line's middle, and for every label not repeated). The feature is -/// named by its id, not its row, so a lock holds through edits that move rows (#1046). -pub type LabelKey = (u32, usize, usize, usize); +/// named by its full-width id, not its row, so a lock holds through edits that move rows (#1046) +/// and distinct 64-bit ids stay distinct on wasm32 (#1135). +pub type LabelKey = (u32, u64, usize, usize); /// A layer's label classes read against its data, in `Labeling::in_order`: every class whose /// clause takes a row labels it. diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 02584fe46..116133224 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -2299,7 +2299,7 @@ fn raise_labels(map: &Map, view: &View, candidates: &mut [Candidate]) { let cam = view.camera; let (w, h) = (view.width, view.height); // Each extruded layer's rows by feature id, the label keys naming features by id (#1046). - let mut rows: std::collections::HashMap> = std::collections::HashMap::new(); + let mut rows: std::collections::HashMap> = std::collections::HashMap::new(); for Candidate { label: l, key: (id, fid, _, _), alternatives, row: offered, .. } in candidates.iter_mut() { let Some(layer) = map.layer(*id).filter(|l| l.geometry_type() == GeometryType::Polygon) else { continue }; let Some(x) = &layer.extrusion else { continue }; @@ -2307,9 +2307,9 @@ fn raise_labels(map: &Map, view: &View, candidates: &mut [Candidate]) { // The row it was offered from while its feature is there, else looked up by id; one not // among them, as a streamed layer's, rises by the height no field gives. let row = Some(*offered) - .filter(|&r| features.get(r).is_some_and(|f| f.id as usize == *fid)) + .filter(|&r| features.get(r).is_some_and(|f| f.id == *fid)) .or_else(|| { - let by_id = rows.entry(*id).or_insert_with(|| features.iter().enumerate().map(|(r, f)| (f.id as usize, r)).collect()); + let by_id = rows.entry(*id).or_insert_with(|| features.iter().enumerate().map(|(r, f)| (f.id, r)).collect()); by_id.get(fid).copied() }) .unwrap_or(usize::MAX); @@ -3099,7 +3099,7 @@ fn render_frame(map: &mut Map, view: &View, full_res_rasters: bool, backend: Bac } let lock: Option> = map.label_lock.as_ref().map(|v| v.iter().copied().collect()); // The classes of features with a label locked (any of its repeats), to pass the rest over early. - let locked_classes: Option> = + let locked_classes: Option> = lock.as_ref().map(|k| k.keys().map(|&(l, r, c, _)| (l, r, c)).collect()); let distance = viewing_distance(map.ground_res_at(&view.camera), view.height); // Graphics text: always drawn, and other labels keep clear of it. @@ -3304,7 +3304,7 @@ fn render_frame(map: &mut Map, view: &View, full_res_rasters: bool, backend: Bac let styler = layer.styler(Some(scale)); // A label is named by its feature's id, not its row: a lock holds through edits that // move rows (#1046). - let label_id = |row: usize| layer.dataset.features.get(row).map_or(row, |f| f.id as usize); + let label_id = |row: usize| layer.dataset.features.get(row).map_or(row as u64, |f| f.id); let (labels, selection, ds_ptr) = (&layer.labels, &layer.selection, &layer.dataset); // The globe draws the lon/lat geometry itself; the flat map its Web Mercator copy. let geoms: Vec> = if globe.is_some() { @@ -3862,7 +3862,7 @@ fn render_frame(map: &mut Map, view: &View, full_res_rasters: bool, backend: Bac // Tile labels, after every layer's own. if lock.is_none() { for (i, (l, pri)) in tile_out.labels.into_iter().enumerate() { - label_candidates.push((l, pri - 1e13, (u32::MAX, i, 0, 0)).into()); + label_candidates.push((l, pri - 1e13, (u32::MAX, i as u64, 0, 0)).into()); } } if let Labels::Tilted(to, out) = &mut labels_to { @@ -3876,7 +3876,7 @@ fn render_frame(map: &mut Map, view: &View, full_res_rasters: bool, backend: Bac let (sx, sy) = to.camera.try_to_screen(m, tw, th).filter(|_| near)?; (sy as f32 > sky_y).then_some((sx as f32, sy as f32)) }; - let fixed = fixed_labels.into_iter().map(|l| Candidate::from((l, f64::MAX, (u32::MAX, usize::MAX, 0, 0)))); + let fixed = fixed_labels.into_iter().map(|l| Candidate::from((l, f64::MAX, (u32::MAX, u64::MAX, 0, 0)))); for c in fixed.chain(label_candidates) { out.extend(tilt_candidate(c, &at)); } @@ -5121,7 +5121,7 @@ mod tests { let (x, y) = ((i % 200) as f32 * 10.0, (i / 200) as f32 * 10.0); let l = PlacedLabel { x, y, ..label("ab", x, None, 0.0) }; let alternatives = if others { round.map(|(dx, dy)| (x + dx, y + dy)).to_vec() } else { Vec::new() }; - Candidate { label: l, priority: -f64::from(i), key: (u32::MAX, i as usize, 0, 0), alternatives, row: usize::MAX } + Candidate { label: l, priority: -f64::from(i), key: (u32::MAX, i as u64, 0, 0), alternatives, row: usize::MAX } }) .collect() }; @@ -5416,6 +5416,40 @@ mod tests { assert_eq!(sizes(&mut m), both); } + /// A lock keeps source ids wider than a wasm32 pointer distinct, including when a label + /// that collided at lock time later has room to draw (#1135). + #[test] + fn locked_labels_keep_full_width_feature_ids() { + let mut m = Map::new("Map"); + let mut d = Dataset::new("pts", GeometryType::Point, vec![Field::new("name", FieldType::Text)]); + for (name, lon) in [("Alpha", -0.1), ("Beta", 0.1)] { + d.push(Some(Geometry::Point(Coord::new(lon, 0.0))), vec![Value::Text(name.into())]); + } + d.features[0].id = 1; + d.features[1].id = (1_u64 << 32) + 1; + let id = m.add_dataset(d); + m.layer_mut(id).unwrap().labels.enabled = true; + m.label_display.locked = true; + let mut view = View { + width: 800.0, + height: 600.0, + pixels_per_point: 1.0, + camera: Camera { center: lonlat_to_merc(Coord::new(0.0, 0.0)), res: 5000.0, bearing: 0.0, pitch: 0.0 }, + sky: false, + }; + let texts = |m: &mut Map, view: &View| render(m, view).unwrap().labels.into_iter().map(|l| l.text).collect::>(); + assert_eq!(texts(&mut m, &view), ["Alpha"]); + view.camera.res /= 16.0; + assert_eq!(texts(&mut m, &view), ["Alpha"], "Beta was not placed when the lock was taken"); + m.label_display.locked = false; + assert_eq!(texts(&mut m, &view), ["Alpha", "Beta"]); + m.label_display.locked = true; + assert_eq!(texts(&mut m, &view), ["Alpha", "Beta"]); + let mut ids: Vec = m.label_lock.as_ref().unwrap().iter().map(|((_, f, ..), _)| *f).collect(); + ids.sort_unstable(); + assert_eq!(ids, [1, (1_u64 << 32) + 1]); + } + /// A point's label whose place another takes goes round the point when its class says so /// (#1011): off, it is left out as before; locked, it stays where it went as the map moves; /// turned and tilted views place it too.