add gpx snapping

Cargo.lock

@@ -1590,7 +1590,7 @@ dependencies = [
 
 [[package]]
 name = "rs_maps"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "anyhow",
  "argon2",

Cargo.toml

@@ -1,7 +1,7 @@
 # Cargo.toml
 [package]
 name = "rs_maps"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2021"
 
 [dependencies]

frontend/app.js

@@ -50,7 +50,8 @@ const state = {
   draftLatLng: null,
   colour: DEFAULT_COLOUR,
   searchPin: null,     // transient pin for the current search hit
-  draw: { active: false, points: [], markers: [], line: null },
+  draw: { active: false, points: [], markers: [], line: null, guide: null,
+          timer: null, seq: 0, snapped: null },
 };
 
 /* ───────────────────────────── api ───────────────────────────── */
@@ -468,10 +469,17 @@ $('#marker-delete').addEventListener('click', async () => {
 
 const DRAW_COLOUR = '#4FA3D1';
 
+/* brouter.de is donated infrastructure, so previews are debounced rather than
+   fired on every click and every pixel of a drag. */
+const PREVIEW_DEBOUNCE_MS = 1000;
+
 function drawReset() {
+  if (state.draw.timer) clearTimeout(state.draw.timer);
   state.draw.markers.forEach((m) => state.map.removeLayer(m));
   if (state.draw.line) state.map.removeLayer(state.draw.line);
-  state.draw = { active: false, points: [], markers: [], line: null };
+  if (state.draw.guide) state.map.removeLayer(state.draw.guide);
+  state.draw = { active: false, points: [], markers: [], line: null, guide: null,
+                 timer: null, seq: 0, snapped: null };
 }
 
 function drawSetActive(on) {
@@ -481,44 +489,102 @@ function drawSetActive(on) {
   $('#draw-panel').hidden = !on;
   $('#draw-start').textContent = on ? 'Drawing…' : 'New route';
   $('#draw-start').disabled = on;
-  // Crosshair makes the mode obvious; without it the map looks unchanged.
   state.map.getContainer().style.cursor = on ? 'crosshair' : '';
   if (on) drawUpdate();
 }
 
-/* Straight-line distance through the waypoints. Not the routed distance — the
-   real figure is only known once brouter has answered, so this is labelled as
-   a direct measurement rather than presented as the route length. */
-function directDistance(points) {
-  let metres = 0;
-  for (let i = 1; i < points.length; i += 1) {
-    metres += state.map.distance(points[i - 1], points[i]);
-  }
-  return metres;
-}
-
 function fmtDistance(metres) {
   if (!metres) return '—';
   return metres < 1000
-    ? `${Math.round(metres)} m direct`
-    : `${(metres / 1000).toFixed(1)} km direct`;
+    ? `${Math.round(metres)} m`
+    : `${(metres / 1000).toFixed(1)} km`;
+}
+
+/* The dashed guide shows the order of waypoints. It's replaced visually by the
+   snapped line once brouter answers, but kept underneath so there's always
+   something on screen while a preview is in flight. */
+function drawGuide() {
+  const latlngs = state.draw.points.map((p) => [p.lat, p.lng]);
+  if (state.draw.guide) {
+    state.draw.guide.setLatLngs(latlngs);
+  } else {
+    state.draw.guide = L.polyline(latlngs, {
+      color: DRAW_COLOUR, weight: 1, opacity: .35, dashArray: '3 6', interactive: false,
+    }).addTo(state.map);
+  }
 }
 
 function drawUpdate() {
   const points = state.draw.points;
   $('#draw-n').textContent = points.length;
-  $('#draw-dist').textContent = fmtDistance(directDistance(points));
   $('#draw-save').disabled = points.length < 2;
   $('#draw-undo').disabled = points.length === 0;
 
-  const latlngs = points.map((p) => [p.lat, p.lng]);
-  if (state.draw.line) {
-    state.draw.line.setLatLngs(latlngs);
-  } else {
-    state.draw.line = L.polyline(latlngs, {
-      color: DRAW_COLOUR, weight: 2, dashArray: '4 5', interactive: false,
-    }).addTo(state.map);
+  drawGuide();
+  schedulePreview();
+}
+
+function setPreviewStatus(text) {
+  $('#draw-dist').textContent = text;
+}
+
+function schedulePreview() {
+  if (state.draw.timer) clearTimeout(state.draw.timer);
+
+  if (state.draw.points.length < 2) {
+    if (state.draw.line) { state.map.removeLayer(state.draw.line); state.draw.line = null; }
+    state.draw.snapped = null;
+    setPreviewStatus('—');
+    return;
   }
+
+  setPreviewStatus('snapping…');
+  state.draw.timer = setTimeout(runPreview, PREVIEW_DEBOUNCE_MS);
+}
+
+async function runPreview() {
+  const points = state.draw.points.slice();
+  // Responses can arrive out of order after a fast edit; only the newest counts.
+  const seq = (state.draw.seq += 1);
+
+  try {
+    const body = { name: 'preview.gpx', waypoints: points.map((p) => ({ lat: p.lat, lon: p.lng })) };
+    const result = await api('api/routes/preview', { method: 'POST', ...json(body) });
+
+    if (seq !== state.draw.seq || !state.draw.active) return;
+
+    state.draw.snapped = result;
+    const latlngs = result.points.map((p) => [p[0], p[1]]);
+
+    if (state.draw.line) {
+      state.draw.line.setLatLngs(latlngs);
+    } else {
+      state.draw.line = L.polyline(latlngs, {
+        color: DRAW_COLOUR, weight: 4, opacity: .9,
+      }).addTo(state.map);
+    }
+
+    const ascend = result.ascend_m ? ` · ${Math.round(result.ascend_m)} m ascent` : '';
+    setPreviewStatus(fmtDistance(result.length_m) + ascend);
+    markUnsnappable(false);
+  } catch (err) {
+    if (seq !== state.draw.seq || !state.draw.active) return;
+
+    // Keep the last good line rather than clearing the map — usually only the
+    // most recent point is the problem, and it's about to be moved or undone.
+    setPreviewStatus('no route');
+    markUnsnappable(true);
+    toast(err.message || 'Could not snap that route', true);
+  }
+}
+
+/* Flags the most recently added waypoint, which is nearly always the one that
+   can't be reached — brouter doesn't say which point failed. */
+function markUnsnappable(bad) {
+  const last = state.draw.markers[state.draw.markers.length - 1];
+  if (!last) return;
+  const el = last.getElement();
+  if (el) el.classList.toggle('wp-bad', bad);
 }
 
 function addWaypoint(latlng) {
@@ -534,12 +600,12 @@ function addWaypoint(latlng) {
     }),
   }).addTo(state.map);
 
-  // Dragging rewrites the point in place; numbering is unaffected because the
-  // marker's position in the array doesn't change.
+  // Guide follows the drag live; the snapped preview waits for the debounce.
   marker.on('drag', (e) => {
     const i = state.draw.markers.indexOf(marker);
-    if (i !== -1) { state.draw.points[i] = e.target.getLatLng(); drawUpdate(); }
+    if (i !== -1) { state.draw.points[i] = e.target.getLatLng(); drawGuide(); }
   });
+  marker.on('dragend', drawUpdate);
 
   state.draw.markers.push(marker);
   drawUpdate();

frontend/index.html

@@ -139,7 +139,7 @@
     </div>
 
     <div id="draw-panel" hidden>
-      <p class="draw-count"><span id="draw-n">0</span> waypoints · <span id="draw-dist">—</span></p>
+      <p class="draw-count"><span id="draw-n">0</span> waypoints · <span id="draw-dist">—</span> routed</p>
       <div class="row">
         <button class="ghost small" id="draw-undo">Undo point</button>
         <button class="ghost small" id="draw-cancel">Cancel</button>

frontend/style.css

@@ -569,3 +569,11 @@ button.link:hover { color: var(--paper); background: none; }
   cursor: grab;
 }
 .wp:active { cursor: grabbing; }
+
+/* Waypoint brouter couldn't reach — usually too far from any path. The line
+   keeps its last good shape, so this is the only cue that something's wrong. */
+.wp-bad .wp {
+  background: #C4443A;
+  color: #fff;
+  border-color: rgba(255,255,255,.9);
+}

src/routes.rs

@@ -102,23 +102,13 @@ fn looks_like_gpx(body: &str) -> bool {
     head.contains("<gpx")
 }
 
-async fn calculate(
-    State(state): State<AppState>,
-    CurrentUser(_): CurrentUser,
-    Json(request): Json<CalculateRequest>,
-) -> AppResult<Json<CalculateResponse>> {
-    validate(&request)?;
-
-    let mut filename = request.name.trim().to_string();
-    if !filename.to_ascii_lowercase().ends_with(".gpx") {
-        filename.push_str(".gpx");
-    }
-    // Rejects traversal and anything that isn't a plain .gpx name.
-    gpx::validate_filename(&filename)?;
-
+/// Calls brouter and returns the raw body. `format` is brouter's own parameter:
+/// "gpx" for the file we store, "geojson" for the preview (smaller, and trivial
+/// to parse for coordinates without pulling in an XML dependency).
+async fn brouter_fetch(waypoints: &[Waypoint], format: &str) -> AppResult<String> {
     let url = format!(
-        "{BROUTER_URL}?lonlats={}&profile={PROFILE}&alternativeidx=0&format=gpx",
-        lonlats(&request.waypoints)
+        "{BROUTER_URL}?lonlats={}&profile={PROFILE}&alternativeidx=0&format={format}",
+        lonlats(waypoints)
     );
 
     let client = reqwest::Client::builder()
@@ -145,19 +135,108 @@ async fn calculate(
         .map_err(|e| AppError::Other(e.into()))?;
 
     if body.len() > MAX_RESPONSE_BYTES {
-        return Err(AppError::BadGateway("Route too large to store.".into()));
+        return Err(AppError::BadGateway("Route too large to handle.".into()));
+    }
+    Ok(body)
+}
+
+/// BRouter reports routing failures as HTTP 200 with a plain-text body, so the
+/// status code alone never tells you whether it worked.
+fn routing_failure(body: &str) -> AppError {
+    let detail = body.lines().next().unwrap_or("").trim();
+    tracing::info!(detail, "brouter could not route");
+    AppError::BadRequest(if detail.is_empty() {
+        "No route could be found between those points.".into()
+    } else {
+        format!("No route found: {detail}")
+    })
+}
+
+#[derive(Serialize)]
+pub struct PreviewResponse {
+    /// [[lat, lon], …] — flipped from brouter's lon-first order, ready for Leaflet.
+    pub points: Vec<[f64; 2]>,
+    /// Routed distance in metres, as reported by brouter.
+    pub length_m: Option<f64>,
+    /// Cumulative ascent in metres, where brouter provides it.
+    pub ascend_m: Option<f64>,
+}
+
+/// brouter's geojson has one LineString feature; distance and ascent arrive as
+/// strings in its properties.
+fn parse_geojson(body: &str) -> AppResult<PreviewResponse> {
+    let parsed: serde_json::Value =
+        serde_json::from_str(body).map_err(|_| routing_failure(body))?;
+
+    let feature = parsed
+        .get("features")
+        .and_then(|f| f.get(0))
+        .ok_or_else(|| routing_failure(body))?;
+
+    let coords = feature
+        .get("geometry")
+        .and_then(|g| g.get("coordinates"))
+        .and_then(|c| c.as_array())
+        .ok_or_else(|| routing_failure(body))?;
+
+    let points = coords
+        .iter()
+        .filter_map(|pair| {
+            let pair = pair.as_array()?;
+            // Longitude first in geojson, latitude first for Leaflet.
+            Some([pair.get(1)?.as_f64()?, pair.first()?.as_f64()?])
+        })
+        .collect::<Vec<_>>();
+
+    if points.len() < 2 {
+        return Err(routing_failure(body));
     }
 
+    let number = |key: &str| {
+        feature
+            .get("properties")
+            .and_then(|p| p.get(key))
+            .and_then(|v| v.as_str())
+            .and_then(|v| v.parse::<f64>().ok())
+    };
+
+    Ok(PreviewResponse {
+        points,
+        length_m: number("track-length"),
+        ascend_m: number("filtered ascend"),
+    })
+}
+
+/// Snapped geometry without writing anything. Called repeatedly while drawing,
+/// so it does no disk I/O at all.
+async fn preview(
+    State(_): State<AppState>,
+    CurrentUser(_): CurrentUser,
+    Json(request): Json<CalculateRequest>,
+) -> AppResult<Json<PreviewResponse>> {
+    validate(&request)?;
+    let body = brouter_fetch(&request.waypoints, "geojson").await?;
+    Ok(Json(parse_geojson(&body)?))
+}
+
+async fn calculate(
+    State(state): State<AppState>,
+    CurrentUser(_): CurrentUser,
+    Json(request): Json<CalculateRequest>,
+) -> AppResult<Json<CalculateResponse>> {
+    validate(&request)?;
+
+    let mut filename = request.name.trim().to_string();
+    if !filename.to_ascii_lowercase().ends_with(".gpx") {
+        filename.push_str(".gpx");
+    }
+    // Rejects traversal and anything that isn't a plain .gpx name.
+    gpx::validate_filename(&filename)?;
+
+    let body = brouter_fetch(&request.waypoints, "gpx").await?;
+
     if !looks_like_gpx(&body) {
-        // BRouter's plain-text failures are usually "couldn't snap that point to
-        // a path" — surface a trimmed version, it's genuinely useful to the user.
-        let detail = body.lines().next().unwrap_or("").trim();
-        tracing::info!(detail, "brouter could not route");
-        return Err(AppError::BadRequest(if detail.is_empty() {
-            "No route could be found between those points.".into()
-        } else {
-            format!("No route found: {detail}")
-        }));
+        return Err(routing_failure(&body));
     }
 
     // Same atomic create_new reservation the upload path uses, so a name clash
@@ -181,7 +260,10 @@ async fn calculate(
 }
 
 pub fn routes() -> Router<AppState> {
-    Router::new().route("/calculate", post(calculate))
+    Router::new()
+        // Preview writes nothing; calculate writes into GPX_DIR.
+        .route("/preview", post(preview))
+        .route("/calculate", post(calculate))
 }
 
 #[cfg(test)]
@@ -216,6 +298,24 @@ mod tests {
         assert!(validate(&request).is_err());
     }
 
+    #[test]
+    fn parses_geojson_and_flips_coordinate_order() {
+        let body = r#"{"features":[{"geometry":{"coordinates":
+            [[-2.24,53.48,50],[-2.25,53.49,60]]},
+            "properties":{"track-length":"1234","filtered ascend":"56"}}]}"#;
+
+        let parsed = parse_geojson(body).expect("valid geojson");
+        // geojson is lon,lat; the response must be lat,lon for Leaflet.
+        assert_eq!(parsed.points[0], [53.48, -2.24]);
+        assert_eq!(parsed.length_m, Some(1234.0));
+        assert_eq!(parsed.ascend_m, Some(56.0));
+    }
+
+    #[test]
+    fn plain_text_failure_is_not_geojson() {
+        assert!(parse_geojson("position not mapped in existing datafile").is_err());
+    }
+
     #[test]
     fn detects_non_gpx_body() {
         assert!(looks_like_gpx("<?xml version=\"1.0\"?><gpx version=\"1.1\">"));