From b169023cb37e11fe6e9025008078d83318a8d270 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Fri, 11 Sep 2026 17:52:42 -0500 Subject: [PATCH 1/9] fix: minute-long drawn-polygon latency after runs push large results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (verified with a CDP-throttled browser): drawn shapes were removed locally and only re-rendered when the server's add_polygon echo round-tripped — and that tiny echo queues on the ORDERED WebSocket behind whatever multi-MB inline-geojson result broadcasts are still in flight, plus a link kept saturated by layer-manager pollers re-pulling every asset's full geometry. On an 8 Mbps link with 6x3.2MB vector results, the polygon took ~30s to appear; server round trip on loopback is 10ms. Three fixes: 1. Optimistic pending rendering (honest, not hidden): a finished draw renders immediately as a dashed outline + faint fill, tagged with a client_id that the server now echoes back in both the add_polygon broadcast and draw_complete. The dashed shape swaps to the solid confirmed asset when the echo lands — near-instant on a healthy link, and on a congested one the dash is a truthful "not landed on the server yet" signal instead of an invisible wait. Pending shapes survive basemap switches (captureMapState) and deliberately survive session restores: a shape that never landed stays dashed rather than silently vanishing. 2. Summary mode for the asset list: GET /api/maps/{id}/assets now takes include_geojson=false, returning geojson=null plus a new precomputed bbox column (filled at create_asset; ALTER TABLE migration for existing DBs, legacy rows fall back to null). A 20k-feature 3-asset map's poll shrinks 10.0 MB -> 1.1 KB. The summary SELECT excludes the geojson column entirely so big strings never leave SQLite. 3. GeoTIFF rendering (rasterio reads, percentile stretch, PNG encode) moved off the event loop via asyncio.to_thread — it was blocking every WebSocket message, including draw echoes, while a raster processed. Verified end-to-end with Playwright (healthy + throttled): pending layers appear same-tick as the draw, persist through a 30s congested transfer, and swap to the confirmed asset; client_id round-trip covered by a new TestClient WS test (passes). Note: the pre-existing pytest suite hangs/errors beyond the first test in this environment (MCP StreamableHTTPSessionManager only tolerates one lifespan entry per process + pytest-asyncio fixture teardown crosses task boundaries) — present on main with locked deps, unrelated to this change. Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/database.py | 9 ++ server/mapcontrol_server/main.py | 94 +++++++++++++- server/mapcontrol_server/models.py | 7 +- server/mapcontrol_server/routers/assets.py | 12 +- .../services/asset_service.py | 120 ++++++++++-------- .../services/geotiff_service.py | 102 ++++++++------- server/tests/test_assets.py | 29 +++++ server/tests/test_drawing.py | 38 ++++++ 8 files changed, 303 insertions(+), 108 deletions(-) diff --git a/server/mapcontrol_server/database.py b/server/mapcontrol_server/database.py index 4be5bc5..467557e 100644 --- a/server/mapcontrol_server/database.py +++ b/server/mapcontrol_server/database.py @@ -42,6 +42,7 @@ animated INTEGER DEFAULT 0, z_index INTEGER DEFAULT 0, source_url TEXT, + bbox TEXT, created_at TEXT NOT NULL, updated_at TEXT NOT NULL ); @@ -104,6 +105,14 @@ async def init_db(config: AppConfig) -> aiosqlite.Connection: ) except aiosqlite.OperationalError: pass # column already present + # assets.bbox: precomputed [minLon, minLat, maxLon, maxLat] JSON, filled at + # create_asset. Lets the asset list serve a geojson-free summary (layer + # managers poll it every few seconds) while zoom-to-fit still works. + # Pre-existing rows keep NULL — readers fall back to the full geojson. + try: + await _db.execute("ALTER TABLE assets ADD COLUMN bbox TEXT") + except aiosqlite.OperationalError: + pass # column already present await _db.commit() return _db diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index 0ed2f36..dcd4d49 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -216,6 +216,10 @@ async def websocket_endpoint(websocket: WebSocket, map_id: str, user_session_id: draw_data = msg.get("data", {}) geojson_str = draw_data.get("geojson", "") draw_type = draw_data.get("draw_type", "polygon") + # Client-generated correlation id: the drawing browser renders + # the shape immediately in a "pending" style and swaps it for + # the confirmed asset when this id round-trips. + client_id = draw_data.get("client_id", "") asset_type = f"drawn_{draw_type}" name = draw_data.get("name", f"User drawn {draw_type}") @@ -239,13 +243,16 @@ async def websocket_endpoint(websocket: WebSocket, map_id: str, user_session_id: f"type={asset_type} map={map_id} session={user_session_id}" ) - # Broadcast the new asset to all sessions as an add_polygon event + # Broadcast the new asset to all sessions as an add_polygon + # event. client_id lets the drawing session replace its + # pending-styled shape; other sessions ignore it. broadcast_msg = { "type": "add_polygon", "data": { "asset_id": asset.asset_id, "geojson": geojson_str, "style": style.model_dump(), + "client_id": client_id, }, } await manager.broadcast_to_map(map_id, broadcast_msg) @@ -257,6 +264,7 @@ async def websocket_endpoint(websocket: WebSocket, map_id: str, user_session_id: "asset_id": asset.asset_id, "asset_type": asset_type, "draw_type": draw_type, + "client_id": client_id, }, })) except Exception as e: @@ -1700,6 +1708,52 @@ class BasemapPickerControl {{ renderDeckArcs(); }} + // ─── Pending draws (optimistic rendering with an honest style) ─── + // A finished drawing renders IMMEDIATELY in a dashed "pending" style + // and is swapped for the solid server-confirmed asset when the + // add_polygon echo (carrying our client_id) round-trips. The dash is + // deliberate signal, not decoration: the server must own the shape + // before EOGPT can operate on it, so a shape that STAYS dashed tells + // a user on a bad connection that it has not landed yet — instead of + // silently pretending it did (or, worse, the old behavior: removing + // it and showing nothing until the round trip completed). + const pendingDraws = {{}}; // client_id -> {{ srcId, layerIds }} + + function addPendingDraw(clientId, geojson) {{ + const srcId = 'pending-src-' + clientId; + const fillId = 'pending-fill-' + clientId; + const lineId = 'pending-line-' + clientId; + try {{ + map.addSource(srcId, {{ type: 'geojson', data: geojson }}); + map.addLayer({{ + id: fillId, type: 'fill', source: srcId, + filter: ['==', '$type', 'Polygon'], + paint: {{ 'fill-color': '#4264fb', 'fill-opacity': 0.08 }}, + }}); + map.addLayer({{ + id: lineId, type: 'line', source: srcId, + paint: {{ + 'line-color': '#4264fb', 'line-width': 2, + 'line-dasharray': [2, 2], 'line-opacity': 0.9, + }}, + }}); + pendingDraws[clientId] = {{ srcId, layerIds: [fillId, lineId] }}; + }} catch (e) {{ + console.warn('Could not render pending draw:', e); + }} + }} + + function removePendingDraw(clientId) {{ + if (!clientId) return; + const pending = pendingDraws[clientId]; + if (!pending) return; + for (const lid of pending.layerIds) {{ + if (map.getLayer(lid)) map.removeLayer(lid); + }} + if (map.getSource(pending.srcId)) map.removeSource(pending.srcId); + delete pendingDraws[clientId]; + }} + // ─── Event Handlers ─── const handlers = {{ // ─── Navigation: uses MapLibre native flyTo (van Wijk algorithm) ─── @@ -1733,7 +1787,12 @@ class BasemapPickerControl {{ }}, // ─── Asset creation ─── - add_polygon(data) {{ addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'polygon'); }}, + add_polygon(data) {{ + // Drawn-shape echo: drop our pending-styled copy first (no-op + // for other sessions / agent-created polygons). + removePendingDraw(data.client_id); + addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'polygon'); + }}, add_polygon_url(data) {{ addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'polygon'); }}, add_path(data) {{ addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'path'); }}, add_path_url(data) {{ addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'path'); }}, @@ -1943,10 +2002,16 @@ class BasemapPickerControl {{ }}, // ─── Drawn asset from server (add_drawn_polygon) ─── - add_drawn_polygon(data) {{ addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'drawn_polygon'); }}, + add_drawn_polygon(data) {{ + removePendingDraw(data.client_id); + addGeoJSON(data.asset_id, data.geojson, data.style, true, data.name, 'drawn_polygon'); + }}, // ─── Draw confirmation (push to undo stack) ─── draw_complete(data) {{ + // Belt-and-braces: the add_polygon broadcast normally clears + // the pending copy, but draw_complete also carries client_id. + removePendingDraw(data.client_id); if (data.asset_id) {{ drawnAssetStack.push(data.asset_id); updateUndoButton(); @@ -2106,6 +2171,10 @@ class BasemapPickerControl {{ for (const reg of Object.values(assetRegistry)) {{ if (reg && reg.srcId) userSourceIds.add(reg.srcId); }} + // Unconfirmed drawn shapes survive a basemap switch too. + for (const pending of Object.values(pendingDraws)) {{ + if (pending && pending.srcId) userSourceIds.add(pending.srcId); + }} if (map.getSource(MASK_SRC_ID)) userSourceIds.add(MASK_SRC_ID); const userSources = {{}}; const userLayers = []; @@ -2447,6 +2516,15 @@ class BasemapPickerControl {{ console.log('Geoman draw complete:', drawType, e.shape, geojson); + // Render the shape RIGHT NOW in the dashed pending style, + // then swap it for the confirmed asset when the server's + // add_polygon echo returns this client_id. On a healthy + // link the swap is near-instant; on a congested one the + // shape stays visibly dashed until it lands server-side. + const clientId = 'draw-' + Date.now().toString(36) + '-' + + Math.random().toString(36).slice(2, 8); + addPendingDraw(clientId, geojson); + // Send to server via WebSocket if (ws && ws.readyState === WebSocket.OPEN) {{ ws.send(JSON.stringify({{ @@ -2454,12 +2532,18 @@ class BasemapPickerControl {{ data: {{ geojson: geojsonStr, draw_type: drawType, + client_id: clientId, }} }})); + }} else {{ + // Socket down: the dashed shape stays as the honest + // "not landed" signal; session_restore on reconnect + // re-syncs confirmed assets. + console.warn('WebSocket not open — drawn shape kept pending'); }} - // Remove the Geoman-drawn feature from the map - // (server will broadcast back as a proper asset) + // Remove the Geoman-drawn feature (the pending layer has + // replaced it; the server broadcast will add the asset) try {{ featureData.removeGeoJson(); }} catch (err) {{ diff --git a/server/mapcontrol_server/models.py b/server/mapcontrol_server/models.py index 046d679..96c6a28 100644 --- a/server/mapcontrol_server/models.py +++ b/server/mapcontrol_server/models.py @@ -185,7 +185,12 @@ class AssetResponse(BaseModel): map_id: str name: str | None = None asset_type: str - geojson: dict[str, Any] | str + # None in summary responses (list_assets include_geojson=False) — pollers + # like layer managers don't need multi-MB geometry every few seconds. + geojson: dict[str, Any] | str | None + # [minLon, minLat, maxLon, maxLat], precomputed at creation. None for rows + # created before the bbox column existed and for empty geometries. + bbox: list[float] | None = None style: AssetStyle | None = None metadata: AssetMetadata | None = None visible: bool = True diff --git a/server/mapcontrol_server/routers/assets.py b/server/mapcontrol_server/routers/assets.py index b3bb893..b2cfcd3 100644 --- a/server/mapcontrol_server/routers/assets.py +++ b/server/mapcontrol_server/routers/assets.py @@ -11,13 +11,19 @@ @router.get("", response_model=list[AssetResponse]) -async def list_assets(map_id: str): - """List all assets for a map.""" +async def list_assets(map_id: str, include_geojson: bool = True): + """List all assets for a map. + + ``include_geojson=false`` omits the geometry (geojson=null, bbox kept) — + use it for recurring polls (layer managers) so a map holding multi-MB + vector assets doesn't ship them all on every refresh. Fetch a single + asset by id when the full geometry is actually needed. + """ info = await session_service.get_map(map_id) if info is None: raise HTTPException(status_code=404, detail="Map not found") - return await asset_service.list_assets(map_id) + return await asset_service.list_assets(map_id, include_geojson=include_geojson) @router.get("/{asset_id}", response_model=AssetResponse) diff --git a/server/mapcontrol_server/services/asset_service.py b/server/mapcontrol_server/services/asset_service.py index 8f4dd24..c4d39c6 100644 --- a/server/mapcontrol_server/services/asset_service.py +++ b/server/mapcontrol_server/services/asset_service.py @@ -12,6 +12,13 @@ from ..models import AssetResponse, AssetStyle, AssetMetadata, AssetUpdate +def _compute_bbox(geojson: str) -> list[float] | None: + """[minLon, minLat, maxLon, maxLat] of a geojson string, None if empty.""" + from .session_service import _compute_bbox_from_geojsons + + return _compute_bbox_from_geojsons([geojson]) + + async def create_asset( map_id: str, asset_type: str, @@ -31,6 +38,7 @@ async def create_asset( style_json = style.model_dump_json() if style else None metadata_json = metadata.model_dump_json() if metadata else None + bbox = _compute_bbox(geojson) # New assets stack on top: z_index = current max + 1 for this map. cursor = await db.execute( @@ -41,8 +49,8 @@ async def create_asset( z_index = row[0] if row else 0 await db.execute( - """INSERT INTO assets (id, map_id, name, asset_type, geojson, style, metadata, visible, animated, z_index, source_url, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, 1, ?, ?, ?, ?, ?)""", + """INSERT INTO assets (id, map_id, name, asset_type, geojson, style, metadata, visible, animated, z_index, source_url, bbox, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, 1, ?, ?, ?, ?, ?, ?)""", ( asset_id, map_id, @@ -54,6 +62,7 @@ async def create_asset( int(animated), z_index, source_url, + json.dumps(bbox) if bbox else None, now, now, ), @@ -66,6 +75,7 @@ async def create_asset( name=name, asset_type=asset_type, geojson=geojson, + bbox=bbox, style=style, metadata=metadata, visible=True, @@ -104,57 +114,17 @@ async def create_asset_from_url( ) -async def list_assets(map_id: str) -> list[AssetResponse]: - """List all assets for a map, bottom-most first (ascending z_index). - - Iterating the result and adding layers in order reproduces the stacking: - later rows render on top (MapLibre adds new layers above existing ones). - """ - db = await get_db() - cursor = await db.execute( - "SELECT * FROM assets WHERE map_id = ? ORDER BY z_index ASC, created_at ASC", - (map_id,), - ) - rows = await cursor.fetchall() - - assets = [] - for row in rows: - style = AssetStyle.model_validate_json(row["style"]) if row["style"] else None - metadata = ( - AssetMetadata.model_validate_json(row["metadata"]) - if row["metadata"] - else None - ) - assets.append( - AssetResponse( - asset_id=row["id"], - map_id=row["map_id"], - name=row["name"], - asset_type=row["asset_type"], - geojson=row["geojson"], - style=style, - metadata=metadata, - visible=bool(row["visible"]), - animated=bool(row["animated"]), - z_index=row["z_index"] or 0, - source_url=row["source_url"], - created_at=row["created_at"], - updated_at=row["updated_at"], - ) - ) - return assets - - -async def get_asset(map_id: str, asset_id: str) -> AssetResponse | None: - """Get a single asset.""" - db = await get_db() - cursor = await db.execute( - "SELECT * FROM assets WHERE id = ? AND map_id = ?", (asset_id, map_id) - ) - row = await cursor.fetchone() - if row is None: +def _parse_bbox(raw: str | None) -> list[float] | None: + if not raw: + return None + try: + bbox = json.loads(raw) + except (ValueError, TypeError): return None + return bbox if isinstance(bbox, list) and len(bbox) == 4 else None + +def _row_to_asset(row, geojson: str | None) -> AssetResponse: style = AssetStyle.model_validate_json(row["style"]) if row["style"] else None metadata = ( AssetMetadata.model_validate_json(row["metadata"]) @@ -166,7 +136,8 @@ async def get_asset(map_id: str, asset_id: str) -> AssetResponse | None: map_id=row["map_id"], name=row["name"], asset_type=row["asset_type"], - geojson=row["geojson"], + geojson=geojson, + bbox=_parse_bbox(row["bbox"]), style=style, metadata=metadata, visible=bool(row["visible"]), @@ -178,6 +149,51 @@ async def get_asset(map_id: str, asset_id: str) -> AssetResponse | None: ) +# Every column except geojson — the summary query must not pull multi-MB +# geometry strings out of SQLite just to throw them away. +_SUMMARY_COLUMNS = ( + "id, map_id, name, asset_type, style, metadata, visible, animated, " + "z_index, source_url, bbox, created_at, updated_at" +) + + +async def list_assets( + map_id: str, include_geojson: bool = True +) -> list[AssetResponse]: + """List all assets for a map, bottom-most first (ascending z_index). + + Iterating the result and adding layers in order reproduces the stacking: + later rows render on top (MapLibre adds new layers above existing ones). + + ``include_geojson=False`` returns a summary (geojson=None, bbox kept) — + the shape layer-manager pollers should request every few seconds, since + full geometries can be many MB per asset. + """ + db = await get_db() + columns = "*" if include_geojson else _SUMMARY_COLUMNS + cursor = await db.execute( + f"SELECT {columns} FROM assets WHERE map_id = ? ORDER BY z_index ASC, created_at ASC", + (map_id,), + ) + rows = await cursor.fetchall() + return [ + _row_to_asset(row, row["geojson"] if include_geojson else None) + for row in rows + ] + + +async def get_asset(map_id: str, asset_id: str) -> AssetResponse | None: + """Get a single asset.""" + db = await get_db() + cursor = await db.execute( + "SELECT * FROM assets WHERE id = ? AND map_id = ?", (asset_id, map_id) + ) + row = await cursor.fetchone() + if row is None: + return None + return _row_to_asset(row, row["geojson"]) + + async def reorder_assets(map_id: str, asset_ids: list[str]) -> None: """Persist a new stacking order. diff --git a/server/mapcontrol_server/services/geotiff_service.py b/server/mapcontrol_server/services/geotiff_service.py index 4285628..43cd3f5 100644 --- a/server/mapcontrol_server/services/geotiff_service.py +++ b/server/mapcontrol_server/services/geotiff_service.py @@ -13,6 +13,7 @@ from __future__ import annotations +import asyncio import io import logging import os @@ -384,28 +385,33 @@ async def process_geotiff_rgb( tif_path, size_mb = await _download_to_tempfile(source, max_bytes) is_temp = True - # Process - rgba, ds = _process_rgb(tif_path, bands, alpha, nodata) - bounds = _compute_bounds_4326(ds) - band_count = ds.count - crs_str = str(ds.crs) if ds.crs else "unknown" - ds.close() - - # Save PNG - img = Image.fromarray(rgba, "RGBA") - png_filename = f"{asset_id}.png" - png_path = file_dir / png_filename - img.save(str(png_path), "PNG") - - return GeoTIFFResult( - png_path=str(png_path), - bounds=bounds, - width=rgba.shape[1], - height=rgba.shape[0], - crs=crs_str, - band_count=band_count, - image_url=f"/api/files/{png_filename}", - ) + # Process + PNG encode are CPU-bound (rasterio reads, percentile + # stretches, PIL compression) — run off the event loop so WebSocket + # traffic (drawn-shape echoes, viewport updates) keeps flowing while + # a raster renders. + def _render() -> GeoTIFFResult: + rgba, ds = _process_rgb(tif_path, bands, alpha, nodata) + bounds = _compute_bounds_4326(ds) + band_count = ds.count + crs_str = str(ds.crs) if ds.crs else "unknown" + ds.close() + + img = Image.fromarray(rgba, "RGBA") + png_filename = f"{asset_id}.png" + png_path = file_dir / png_filename + img.save(str(png_path), "PNG") + + return GeoTIFFResult( + png_path=str(png_path), + bounds=bounds, + width=rgba.shape[1], + height=rgba.shape[0], + crs=crs_str, + band_count=band_count, + image_url=f"/api/files/{png_filename}", + ) + + return await asyncio.to_thread(_render) except ValueError as e: msg = str(e) @@ -483,31 +489,33 @@ async def process_geotiff_singleband( tif_path, size_mb = await _download_to_tempfile(source, max_bytes) is_temp = True - # Process - rgba, ds = _process_singleband( - tif_path, band, colormap, alpha, - vmin, vmax, percentile_min, percentile_max, nodata, - ) - bounds = _compute_bounds_4326(ds) - band_count = ds.count - crs_str = str(ds.crs) if ds.crs else "unknown" - ds.close() - - # Save PNG - img = Image.fromarray(rgba, "RGBA") - png_filename = f"{asset_id}.png" - png_path = file_dir / png_filename - img.save(str(png_path), "PNG") - - return GeoTIFFResult( - png_path=str(png_path), - bounds=bounds, - width=rgba.shape[1], - height=rgba.shape[0], - crs=crs_str, - band_count=band_count, - image_url=f"/api/files/{png_filename}", - ) + # CPU-bound render off the event loop — same reasoning as the RGB path. + def _render() -> GeoTIFFResult: + rgba, ds = _process_singleband( + tif_path, band, colormap, alpha, + vmin, vmax, percentile_min, percentile_max, nodata, + ) + bounds = _compute_bounds_4326(ds) + band_count = ds.count + crs_str = str(ds.crs) if ds.crs else "unknown" + ds.close() + + img = Image.fromarray(rgba, "RGBA") + png_filename = f"{asset_id}.png" + png_path = file_dir / png_filename + img.save(str(png_path), "PNG") + + return GeoTIFFResult( + png_path=str(png_path), + bounds=bounds, + width=rgba.shape[1], + height=rgba.shape[0], + crs=crs_str, + band_count=band_count, + image_url=f"/api/files/{png_filename}", + ) + + return await asyncio.to_thread(_render) except ValueError as e: msg = str(e) diff --git a/server/tests/test_assets.py b/server/tests/test_assets.py index 0f113eb..9ece604 100644 --- a/server/tests/test_assets.py +++ b/server/tests/test_assets.py @@ -56,6 +56,35 @@ async def test_get_asset(client, map_with_asset): assert resp.json()["name"] == "Test Polygon" +async def test_list_assets_summary_mode(client, map_with_asset): + """include_geojson=false omits geometry but keeps the precomputed bbox, + so layer-manager pollers can refresh + zoom-to-fit without re-downloading + potentially multi-MB geometries every few seconds.""" + map_id, asset_id = map_with_asset + resp = await client.get( + f"/api/maps/{map_id}/assets", params={"include_geojson": "false"} + ) + assert resp.status_code == 200 + assets = resp.json() + assert len(assets) == 1 + assert assets[0]["asset_id"] == asset_id + assert assets[0]["geojson"] is None + # SAMPLE_GEOJSON spans lon [-97.7, -97.6], lat [30.2, 30.3] + assert assets[0]["bbox"] == [-97.7, 30.2, -97.6, 30.3] + # Non-geometry fields are intact + assert assets[0]["name"] == "Test Polygon" + assert assets[0]["style"]["fill_color"] == "#ff0000" + + +async def test_list_assets_default_includes_geojson_and_bbox(client, map_with_asset): + map_id, _ = map_with_asset + resp = await client.get(f"/api/maps/{map_id}/assets") + assert resp.status_code == 200 + assets = resp.json() + assert assets[0]["geojson"] == SAMPLE_GEOJSON + assert assets[0]["bbox"] == [-97.7, 30.2, -97.6, 30.3] + + async def test_update_asset_visibility(client, map_with_asset): map_id, asset_id = map_with_asset resp = await client.patch(f"/api/maps/{map_id}/assets/{asset_id}", json={ diff --git a/server/tests/test_drawing.py b/server/tests/test_drawing.py index f3c2726..cae4bdc 100644 --- a/server/tests/test_drawing.py +++ b/server/tests/test_drawing.py @@ -212,3 +212,41 @@ async def test_drawn_asset_style_can_be_updated(client, map_id): assert resp.status_code == 200 data = resp.json() assert data["style"]["fill_color"] == "#ff0000" + + +def test_user_drawn_feature_echoes_client_id(): + """The WS draw round trip carries the client's correlation id back in BOTH + the add_polygon broadcast and the draw_complete confirmation, so the + drawing browser can swap its dashed pending-styled shape for the + confirmed asset.""" + import json + from starlette.testclient import TestClient + from mapcontrol_server.main import app as _app + + tmp = tempfile.mkdtemp() + os.environ["MAPCONTROL_DB_PATH"] = f"{tmp}/test.db" + os.environ["MAPCONTROL_FILE_DIR"] = f"{tmp}/files" + + with TestClient(_app) as c: + map_id = c.post("/api/maps").json()["map_id"] + session_id = c.post(f"/api/maps/{map_id}/sessions").json()["user_session_id"] + + with c.websocket_connect(f"/ws/{map_id}/{session_id}") as ws: + ws.send_text(json.dumps({ + "type": "user_drawn_feature", + "data": { + "geojson": DRAWN_POLYGON_GEOJSON, + "draw_type": "polygon", + "client_id": "draw-test-abc123", + }, + })) + broadcast = json.loads(ws.receive_text()) + confirm = json.loads(ws.receive_text()) + + assert broadcast["type"] == "add_polygon" + assert broadcast["data"]["client_id"] == "draw-test-abc123" + assert broadcast["data"]["asset_id"] + + assert confirm["type"] == "draw_complete" + assert confirm["data"]["client_id"] == "draw-test-abc123" + assert confirm["data"]["asset_id"] == broadcast["data"]["asset_id"] From 90bdcc8a48fab507c3c6796ebf72dfdfbee52039 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Fri, 11 Sep 2026 20:29:45 -0500 Subject: [PATCH 2/9] fix: hover hit-tests saturated the main thread over run-sized results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second half of the slow-polygon-tools bug — and the dominant one on fast networks. Two independent hover systems ran queryRenderedFeatures work on EVERY raw mousemove: 1. esip-contract.js peek hover: one explicit query over all asset layers per mousemove. 2. The shell's feature-state hover-brighten: a MapLibre layer-scoped mousemove listener PER fill layer, each running its own internal hit-test per event. With run-sized vector results loaded (6 assets, ~12k polygons), a single hit-test measures ~13-14ms — at 60-120 mousemoves/sec the handlers demanded ~2x real time in main-thread CPU, so the iframe froze whenever the mouse moved: draw tools crawled, WS processing starved, everything lagged regardless of network. Measured: a 120-mousemove burst cost 2129ms before, 21ms after. Hovering is a dwell interaction, so both systems now defer their single shared hit-test until the cursor settles (~90ms), do nothing while it travels, and stand down entirely while a draw/delete mode is active (new __esipInternals.isDrawing hook). The hover-brighten listeners collapse from one-per-fill-layer to one map-level handler resolving the source via a layerId->srcId map. Behavior on settle is unchanged (asset_hover events + feature-state highlight verified headless); highlights/cards simply don't chase the cursor mid-flight. Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/main.py | 68 +++++++++++++------ .../mapcontrol_server/static/esip-contract.js | 45 +++++++++--- 2 files changed, 85 insertions(+), 28 deletions(-) diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index dcd4d49..044f50a 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -1446,27 +1446,54 @@ class BasemapPickerControl {{ // Polygon fills brighten under the cursor. Uses generateId'd feature // ids on the GeoJSON source; the fill layer's fill-opacity is a // feature-state expression (see addGeoJSON). + // Feature-state hover highlighting, ONE dwell-gated hit-test shared by + // every fill layer. The previous per-layer map.on('mousemove', layerId) + // wiring made MapLibre run an internal hit-test per asset on EVERY raw + // mousemove — measured ~2ms per layer per move over run-sized results, + // which (together with the contract's own per-move query) saturated + // the main thread whenever the mouse traveled and crawled the draw + // tools. Highlighting is a dwell affordance: probe when the cursor + // settles (~90ms), do nothing while it moves or while drawing. + const hoverFillSources = new Map(); // fill layerId -> srcId function wireHoverHighlight(layerId, srcId) {{ - let hoveredId = null; - map.on('mousemove', layerId, (e) => {{ - if (!e.features || !e.features.length) return; - const fid = e.features[0].id; - if (fid === undefined) return; - if (hoveredId !== null && hoveredId !== fid) {{ - map.setFeatureState({{ source: srcId, id: hoveredId }}, {{ hover: false }}); - }} - hoveredId = fid; - map.setFeatureState({{ source: srcId, id: fid }}, {{ hover: true }}); - map.getCanvas().style.cursor = 'pointer'; - }}); - map.on('mouseleave', layerId, () => {{ - if (hoveredId !== null) {{ - map.setFeatureState({{ source: srcId, id: hoveredId }}, {{ hover: false }}); - }} - hoveredId = null; - map.getCanvas().style.cursor = ''; - }}); + hoverFillSources.set(layerId, srcId); }} + const hoverHL = {{ srcId: null, fid: null }}; + function clearHoverHL() {{ + if (hoverHL.srcId !== null && hoverHL.fid !== null) {{ + try {{ + map.setFeatureState({{ source: hoverHL.srcId, id: hoverHL.fid }}, {{ hover: false }}); + }} catch (e) {{ /* source may already be gone */ }} + }} + hoverHL.srcId = null; + hoverHL.fid = null; + map.getCanvas().style.cursor = ''; + }} + let hoverHLTimer = null; + map.on('mousemove', (e) => {{ + if (hoverHLTimer) clearTimeout(hoverHLTimer); + const pt = e.point; + hoverHLTimer = setTimeout(() => {{ + if (currentDrawMode !== null || deleteMode) {{ clearHoverHL(); return; }} + const layers = [...hoverFillSources.keys()].filter(id => map.getLayer(id)); + if (!layers.length) return; + const feats = map.queryRenderedFeatures(pt, {{ layers }}); + if (!feats.length) {{ clearHoverHL(); return; }} + const f = feats[0]; + const srcId = hoverFillSources.get(f.layer.id); + if (f.id === undefined || !srcId) return; + if (hoverHL.srcId === srcId && hoverHL.fid === f.id) return; + clearHoverHL(); + hoverHL.srcId = srcId; + hoverHL.fid = f.id; + map.setFeatureState({{ source: srcId, id: f.id }}, {{ hover: true }}); + map.getCanvas().style.cursor = 'pointer'; + }}, 90); + }}); + map.on('mouseout', () => {{ + if (hoverHLTimer) clearTimeout(hoverHLTimer); + clearHoverHL(); + }}); // ─── Add GeoJSON to map ─── function addGeoJSON(assetId, geojsonStr, style, visible, name, assetType) {{ @@ -2941,6 +2968,9 @@ class BasemapPickerControl {{ baseUrl: BASE_URL, getUserSession: __esipGetUserSession, getCurrentBasemap: function () {{ return currentBasemap; }}, + // Lets the contract suppress hover hit-tests while the user is + // actively drawing/deleting (mousemove-heavy interactions). + isDrawing: function () {{ return currentDrawMode !== null || deleteMode; }}, }}; try {{ window.dispatchEvent(new CustomEvent('esip:internals-ready')); }} catch (e) {{}} diff --git a/server/mapcontrol_server/static/esip-contract.js b/server/mapcontrol_server/static/esip-contract.js index 79e61b4..d999a3b 100644 --- a/server/mapcontrol_server/static/esip-contract.js +++ b/server/mapcontrol_server/static/esip-contract.js @@ -70,16 +70,33 @@ } // ─── Hover (peek) ───────────────────────────────────────────────────── + // The hit-test runs only after the cursor DWELLS for a beat, never on + // raw mousemove. queryRenderedFeatures over run-sized vector results + // costs ~14ms per call (measured); at 60-120 mousemoves/sec that alone + // saturated the main thread the moment the mouse moved, crawling the + // draw tools and everything else in the iframe. Hover cards are a + // dwell interaction — peek when the pointer settles, stay silent while + // it travels. Also fully suppressed while a draw/delete mode is active + // (a hover card mid-draw is noise, and draws are mousemove-heavy). var hoverId = null; - map.on("mousemove", function (e) { + var hoverTimer = null; + var HOVER_DWELL_MS = 90; + function clearHover() { + if (hoverId) { + emit("asset_hover_end", { asset_id: hoverId }); + hoverId = null; + map.getCanvas().style.cursor = ""; + } + } + function hoverProbe(point, lngLat) { + if (typeof internals.isDrawing === "function" && internals.isDrawing()) { + clearHover(); + return; + } var ids = assetLayerIds(); - var feats = ids.length ? map.queryRenderedFeatures(e.point, { layers: ids }) : []; + var feats = ids.length ? map.queryRenderedFeatures(point, { layers: ids }) : []; if (feats.length === 0) { - if (hoverId) { - emit("asset_hover_end", { asset_id: hoverId }); - hoverId = null; - map.getCanvas().style.cursor = ""; - } + clearHover(); return; } var aid = assetForLayer(feats[0].layer.id); @@ -93,10 +110,20 @@ asset_id: aid, name: reg.name || null, asset_type: reg.asset_type || null, - point: { x: e.point.x, y: e.point.y }, - lngLat: [e.lngLat.lng, e.lngLat.lat], + point: { x: point.x, y: point.y }, + lngLat: [lngLat.lng, lngLat.lat], }); } + } + map.on("mousemove", function (e) { + if (hoverTimer) clearTimeout(hoverTimer); + var point = e.point; + var lngLat = e.lngLat; + hoverTimer = setTimeout(function () { hoverProbe(point, lngLat); }, HOVER_DWELL_MS); + }); + map.on("mouseout", function () { + if (hoverTimer) clearTimeout(hoverTimer); + clearHover(); }); // ─── Click (commit) ─────────────────────────────────────────────────── From 6774c2756bb672efb4034f16a6c5f1668f3dcd0c Mon Sep 17 00:00:00 2001 From: TommySquared Date: Fri, 11 Sep 2026 21:29:41 -0500 Subject: [PATCH 3/9] feat: drawn shapes use the EOGPT earth-green accent instead of blue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pending (dashed) and confirmed drawn features now render in the theme greens (--eo-accent #7cc242 fill, #5fa830 stroke) instead of the stock map blue — user-drawn shapes read as part of the product, and the color doubles as an unmistakable is-my-local-build-live marker. Verified headless: pending line #5fa830 dashed [2,2], confirmed fill #7cc242. Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/main.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index 044f50a..fccacda 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -223,10 +223,13 @@ async def websocket_endpoint(websocket: WebSocket, map_id: str, user_session_id: asset_type = f"drawn_{draw_type}" name = draw_data.get("name", f"User drawn {draw_type}") - # Default style for drawn features (distinct blue) + # Default style for drawn features: the EOGPT earth-green + # accent (--eo-accent #7cc242, darker #5fa830 stroke) so + # user-drawn shapes read as part of the product, distinct + # from agent-added analysis layers. style = AssetStyle( - fill_color="#4264fb", - stroke_color="#4264fb", + fill_color="#7cc242", + stroke_color="#5fa830", stroke_width=2.0, ) @@ -1755,12 +1758,12 @@ class BasemapPickerControl {{ map.addLayer({{ id: fillId, type: 'fill', source: srcId, filter: ['==', '$type', 'Polygon'], - paint: {{ 'fill-color': '#4264fb', 'fill-opacity': 0.08 }}, + paint: {{ 'fill-color': '#7cc242', 'fill-opacity': 0.08 }}, }}); map.addLayer({{ id: lineId, type: 'line', source: srcId, paint: {{ - 'line-color': '#4264fb', 'line-width': 2, + 'line-color': '#5fa830', 'line-width': 2, 'line-dasharray': [2, 2], 'line-opacity': 0.9, }}, }}); From 37e57cfab909b8e16caa77e919fe44164aea5db8 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Fri, 11 Sep 2026 22:20:40 -0500 Subject: [PATCH 4/9] =?UTF-8?q?fix:=20basemap=20setStyle=20wiped=20Geoman'?= =?UTF-8?q?s=20sources=20=E2=80=94=20infinite=20error=20loop=20+=20OOM?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduced live: any heavy-path basemap/style switch (vector involved → map.setStyle) destroyed Geoman's internal gm_main/gm_temporary/ gm_internal sources, which captureMapState never preserved. The next draw interaction made Geoman's source-update-manager retry against the missing source in a self-rescheduling setTimeout loop: ~190 "There is no tile manager with ID 'gm_temporary'" errors/sec, forever. Symptoms in the field: drawing goes completely dead after the switch (shapes save server-side via the WS but nothing can render client-side until a full reload) and the tab leaks until OOM — observed 20+GB in Firefox/Zen, which retains the deeply-nested retry stacks. Fix: captureMapState now carries every gm_* source (and, via the existing source-membership pass, their layers) across setStyle, same as user asset sources. Verified headless: gm sources survive a vector→raster switch, zero gm errors through post-switch draw attempts, flat heap; full pending-draw flow (healthy + throttled) still passes. Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/main.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index fccacda..2ee90ce 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -2205,6 +2205,15 @@ class BasemapPickerControl {{ for (const pending of Object.values(pendingDraws)) {{ if (pending && pending.srcId) userSourceIds.add(pending.srcId); }} + // Geoman's internal sources (gm_main / gm_temporary / gm_internal) + // MUST survive setStyle: if they are wiped, Geoman's + // source-update-manager retries against the missing source in an + // infinite setTimeout loop — an unbounded error storm (~200/s, + // observed OOM-ing a Firefox tab at 20+GB) and drawing goes dead + // until a full page reload. + for (const id of Object.keys(style.sources || {{}})) {{ + if (id.startsWith('gm_')) userSourceIds.add(id); + }} if (map.getSource(MASK_SRC_ID)) userSourceIds.add(MASK_SRC_ID); const userSources = {{}}; const userLayers = []; From 8095d9f16eb4834b4ac372892f0497e8463e3fe4 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Fri, 11 Sep 2026 23:31:55 -0500 Subject: [PATCH 5/9] fix: session-snapshot restore wedged the renderer and stalled drawing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Snapshots arrive on EVERY WS (re)connect — including mid-run reconnects — and handleSnapshot re-applied basemap and terrain wholesale each time, with two failure modes seen live during a run: 1. Basemap: the inline raster visibility-toggle assumed raster basemap LAYERS exist. With a vector style live (EOGPT pins ?basemap=maptiler_hybrid) it fired "Cannot style non-existing layer" and desynced currentBasemap from what was displayed. 2. Terrain: setProjection+setTerrain re-ran unconditionally (config default_terrain=3d means every snapshot carries it), racing any in-flight setStyle and hitting MapLibre's terrainDepth painter crash ("shaderPreludeCode ... undefined") — the render loop then throws every frame, the map never settles, and Geoman's source updates stall into "Timeout 60 seconds" — drawn shapes only appeared after that timeout (the observed ~30-60s box delay, with the pending dash invisible because gm:create itself was what stalled). Restore is now change-only and kind-aware: basemap goes through the set_basemap handler (which also makes VECTOR restores work), and only when it differs from the current one AND no ?basemap= URL param pinned an explicit embedder choice; terrain applies only on actual change. Verified headless with the exact failing topology (vector param + raster session default + terrain 3d): zero console errors, gm sources intact, draw pending same-tick and confirmed in 0.4s. Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/main.py | 71 ++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index 2ee90ce..55b0f76 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -2305,43 +2305,52 @@ class BasemapPickerControl {{ addGeoJSON(asset.asset_id, asset.geojson, asset.style, asset.visible); }} }} - // Restore basemap. Skip vector entries — they don't have a - // MapLibre layer with their id (vector basemaps live behind - // setStyle, not as visibility-toggled layers). Snapshot restore - // currently only handles raster basemaps; restoring to a vector - // basemap from a snapshot would need to go through set_basemap's - // setStyle path, which is a deeper change. - if (snapshot.basemap && BASEMAPS[snapshot.basemap]) {{ - const target = snapshot.basemap; - const targetEntry = BASEMAPS[target]; - if ((targetEntry.kind || 'raster') === 'raster') {{ - currentBasemap = target; - for (const id of basemapIds) {{ - const e = BASEMAPS[id]; - if ((e.kind || 'raster') !== 'raster') continue; - map.setLayoutProperty(id, 'visibility', id === target ? 'visible' : 'none'); - }} - updateBasemapPickerActive(); - }} - }} - // Restore theme (map-level; light/dark/auto). + // Restore basemap — ONLY when it actually differs, and always + // through the kind-aware set_basemap handler. Snapshots arrive on + // every WS (re)connect, so this path must be a no-op when nothing + // changed: the old inline raster-toggle ran unconditionally and + // assumed raster basemap LAYERS exist — false whenever the live + // style is a vector basemap (setLayoutProperty then fires + // "Cannot style non-existing layer" and currentBasemap went out + // of sync with what is actually displayed). Routing through the + // handler also makes vector-basemap restores work (heavy + // setStyle path with user-asset + gm_* preservation). + // A ?basemap= URL param is an explicit embedder choice (EOGPT pins + // the theme-matched hybrid); a stale session-stored basemap must + // not override it on (re)connect. Without the param, the stored + // basemap is the truth to restore. + const urlPinnedBasemap = new URLSearchParams(window.location.search).has('basemap'); + if (snapshot.basemap && BASEMAPS[snapshot.basemap] + && snapshot.basemap !== currentBasemap + && !urlPinnedBasemap) {{ + handlers.set_basemap({{ basemap: snapshot.basemap }}); + }} + // Restore theme (map-level; light/dark/auto). CSS-only, cheap. if (snapshot.theme) {{ applyTheme(snapshot.theme); }} // Restore terrain mode BEFORE viewport — setProjection('globe') can // disrupt center/zoom, so we apply terrain first, then set viewport // on the next animation frame once the projection has settled. - if (snapshot.terrain && snapshot.terrain !== '2d') {{ - currentTerrain = snapshot.terrain; - try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} - ensureTerrainSource(); - map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); - enableSky(); - }} else if (snapshot.terrain === '2d') {{ - currentTerrain = '2d'; - try {{ map.setProjection({{ type: 'mercator' }}); }} catch(e) {{}} - map.setTerrain(null); - disableSky(); + // Change-only for the same reason as basemap: redundantly re-running + // setProjection + setTerrain on every reconnect races any in-flight + // style switch and can wedge the renderer (terrain + globe mid-flux + // hits MapLibre's terrainDepth painter crash — the map then never + // settles, and Geoman's source updates stall into 60s timeouts). + const snapTerrain = snapshot.terrain; + if (snapTerrain && snapTerrain !== currentTerrain) {{ + if (snapTerrain !== '2d') {{ + currentTerrain = snapTerrain; + try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} + ensureTerrainSource(); + map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); + enableSky(); + }} else {{ + currentTerrain = '2d'; + try {{ map.setProjection({{ type: 'mercator' }}); }} catch(e) {{}} + map.setTerrain(null); + disableSky(); + }} }} // Terrain restore may have changed projection — re-arbitrate // deck-ribbon vs flat-line for any restored arcs. From a511b7d564863fbae19bc0d354483a3bf027c412 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Sat, 12 Sep 2026 03:19:42 -0500 Subject: [PATCH 6/9] fix: basemap setStyle broke rendering with terrain attached and skipped restore for raster styles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more layers of the mid-run drawing failure, both reproduced in headless Firefox (Chromium tolerates them — which is why earlier verification passed while Zen/Firefox users still hit it): 1. Generated run code calls map_session.set_basemap("satellite") while the map boots with terrain 3d. setStyle with terrain attached corrupts MapLibre's shader cache → per-frame painter crash ("shaderPreludeCode ... undefined" in terrainDepth) → map never settles → Geoman source updates stall into literal 60s timeouts: drawn shapes appear only when those expire, all at once, and the pending dash is invisible because gm:create itself is what stalls. Fix: detach terrain before setStyle; restoreMapState re-attaches it with retries (immediate → style-settle races reject setTerrain → retry on idle + timed backstop; terrain before projection, since setTerrain issued under an already-globe projection is dropped). 2. set_basemap registered its 'style.load' restore handler AFTER calling setStyle. Inline object styles (all raster basemaps) can fire style.load synchronously inside setStyle, so the handler missed it and the entire user-asset/gm/terrain restore silently never ran on raster switches in Firefox. Fix: register before switching. Drive-by: handleSnapshot now passes asset name/type through to addGeoJSON/addImageOverlay — restores previously produced unnamed "vector" rows in hover cards and layer managers. Verified (Firefox headless, vector param + terrain 3d + mid-run satellite switch): draw pending same-tick, confirmed 0.27s, terrain re-attached after both switch directions, gm sources intact, zero console errors. Pre-fix baseline: shader crash + 60s Geoman timeouts + shape never registered within 70s. Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/main.py | 62 ++++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 11 deletions(-) diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index 55b0f76..bb58164 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -1971,11 +1971,20 @@ class BasemapPickerControl {{ // Heavy path: setStyle wipes the style, so save/restore user assets const userState = captureMapState(); - if (nextKind === 'vector') {{ - map.setStyle(entry.url); - }} else {{ - map.setStyle(buildRasterStyle(name)); - }} + // Detach terrain BEFORE setStyle. Swapping styles with terrain + // attached corrupts MapLibre's shader cache — the painter then + // crashes every frame ("shaderPreludeCode ... undefined" in + // terrainDepth, reproduced in Firefox), the map never settles, + // and Geoman's source updates stall into literal 60-second + // timeouts (drawn shapes appear only when those expire, all at + // once). restoreMapState re-attaches terrain from the captured + // state once the new style has loaded. + try {{ map.setTerrain(null); }} catch (e) {{}} + // Register the restore handler BEFORE setStyle: inline object + // styles (all raster basemaps) can fire 'style.load' + // SYNCHRONOUSLY inside setStyle, so a handler registered on + // the next line misses it and the whole user-asset/terrain + // restore silently never runs (observed in Firefox). map.once('style.load', () => {{ currentBasemap = name; currentBasemapKind = nextKind; @@ -1983,6 +1992,11 @@ class BasemapPickerControl {{ updateBasemapPickerActive(); sendViewportUpdate(); }}); + if (nextKind === 'vector') {{ + map.setStyle(entry.url); + }} else {{ + map.setStyle(buildRasterStyle(name)); + }} }}, // ─── Terrain (2D/3D toggle) ─── @@ -2258,10 +2272,34 @@ class BasemapPickerControl {{ // ensureTerrainSource re-adds it. terrainSourceAdded = false; if (snap.terrain && snap.terrain !== '2d') {{ - try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} - ensureTerrainSource(); - try {{ map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); }} catch(e) {{}} - enableSky(); + // Re-attach terrain (the switch detached it — see set_basemap — + // to avoid MapLibre's terrain shader-cache crash). setTerrain + // right at style.load is silently rejected while the style is + // still settling, so: try now, and if it didn't stick retry on + // idle plus a timed backstop. Idempotent — first success wins. + const reattachTerrain = () => {{ + if (map.getTerrain()) return true; + try {{ + if (!map.getSource('terrain-dem')) {{ + terrainSourceAdded = false; // keep flag truthful + ensureTerrainSource(); + }} + // Terrain BEFORE projection: setTerrain issued under an + // already-globe (vertical-perspective) projection gets + // dropped; attached first, it survives the flip. + map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); + map.setProjection({{ type: 'globe' }}); + enableSky(); + }} catch (e) {{ + console.warn('Terrain re-attach attempt failed:', e.message); + return false; + }} + return !!map.getTerrain(); + }}; + if (!reattachTerrain()) {{ + map.once('idle', reattachTerrain); + setTimeout(reattachTerrain, 3000); + }} }} try {{ map.jumpTo({{ @@ -2296,13 +2334,15 @@ class BasemapPickerControl {{ const lons = coords.map(c => c[0]); const lats = coords.map(c => c[1]); const bounds = [Math.min(...lons), Math.min(...lats), Math.max(...lons), Math.max(...lats)]; - addImageOverlay(asset.asset_id, imageUrl, bounds, 1.0, asset.visible); + addImageOverlay(asset.asset_id, imageUrl, bounds, 1.0, asset.visible, asset.name, asset.asset_type); }} else if (asset.asset_type === 'arc') {{ // 3D deck arc: endpoints recovered from the stored // FeatureCollection's Point features inside addArc. addArc(asset.asset_id, asset.geojson, asset.style, asset.visible, asset.name); }} else {{ - addGeoJSON(asset.asset_id, asset.geojson, asset.style, asset.visible); + // Pass name + type through — losing them on restore left + // hover cards and layer managers with unnamed "vector" rows. + addGeoJSON(asset.asset_id, asset.geojson, asset.style, asset.visible, asset.name, asset.asset_type); }} }} // Restore basemap — ONLY when it actually differs, and always From b915303ecf35dbf5d012c01fcda02d060fbf52e6 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Sat, 12 Sep 2026 14:21:19 -0500 Subject: [PATCH 7/9] =?UTF-8?q?refactor:=20one=20permanent=20style=20?= =?UTF-8?q?=E2=80=94=20basemap=20switching=20by=20visibility=20toggles;=20?= =?UTF-8?q?3d=20mode=20is=20globe-only=20(no=20MapLibre=20terrain)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Structural fix for the setStyle bug class (Geoman source wipes, shader crashes, missed style.load restores, dead draw previews after a run switches basemaps — each previously patched individually): The style loaded at init is now PERMANENT. Every raster basemap is injected into it (hidden) right after style load — with namespaced source ids, since vector styles ship colliding source names (MapTiler hybrid has a source literally called "satellite") — and switching among the loaded vector basemap plus any raster basemap is pure visibility toggling. setStyle survives only for "switch to a vector style that is NOT the loaded one" (rare; still covered by the earlier hardening, and the new style then becomes the permanent one with rasters re-injected). Geoman, user assets, terrain state, and the deck overlay are simply never destroyed on the common paths — including the run's own set_basemap("satellite"). 3d mode is now globe projection + sky ONLY: MapLibre raster-DEM terrain is retired (globe+terrain is explicitly half-supported upstream and was the renderer-crash trigger; real 3D terrain is the Babylon/3D-Tiles initiative's job). The DEM source machinery is deleted. The default view mode is also applied at style.load instead of 'load' — flipping mercator→globe seconds after first paint read as the map "resetting and zooming out" on fresh pages and left vector labels unplaced until the next interaction. Snapshot viewport restore is now change-only for the same reason. Verified headless (Firefox): vector init injects 5 hidden rasters; satellite switch and back are zero-rebuild (style.load count stays 0), layer count constant, gm sources intact, draw pending same-tick and confirmed in 0.26s right after switching; raster-init → osm toggle, → hybrid one legacy rebuild then toggles thereafter; getTerrain() null everywhere; zero console errors across all scenarios. Co-Authored-By: Claude Fable 5 --- server/config.toml | 5 +- server/mapcontrol_server/main.py | 278 +++++++++++++++++-------------- 2 files changed, 160 insertions(+), 123 deletions(-) diff --git a/server/config.toml b/server/config.toml index e97ca0a..a26e9ea 100644 --- a/server/config.toml +++ b/server/config.toml @@ -21,7 +21,10 @@ default_basemap = "satellite" # basemap was explicitly chosen (create_map arg, set_basemap, or ?basemap=). # Keyless Carto Dark Matter so it works without provider API keys. default_dark_basemap = "carto_dark" -# Default terrain mode: "2d" or "3d" (3d = globe with terrain tiles + sky) +# Default view mode: "2d" (flat mercator) or "3d" (globe projection + sky). +# 3d no longer enables MapLibre raster-DEM terrain — globe+terrain is the +# half-supported combination behind a renderer-crash class; real 3D terrain +# belongs to the (separate) Babylon/3D-Tiles view. default_terrain = "3d" # ─── Basemap providers ──────────────────────────────────────────────────── diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index bb58164..b333ae3 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -912,7 +912,13 @@ class BasemapPickerControl {{ // If the default basemap is vector, init with its style.json URL // directly. Otherwise build a raster-only style covering every - // raster basemap (vector entries get loaded later via setStyle). + // raster basemap. Either way the style loaded here is PERMANENT: + // every raster basemap is injected into it (hidden) right after + // load, and switching among the loaded vector basemap plus any + // raster basemap is pure visibility toggling — see set_basemap. setStyle + // (a destructive full-style rebuild that wipes Geoman's internals + // and user assets) only remains for the rare "switch to a vector + // style that is NOT the loaded one" case. const _defaultEntry = BASEMAPS[DEFAULTS.basemap] || {{}}; const _initialStyle = (_defaultEntry.kind === 'vector') ? _defaultEntry.url @@ -939,6 +945,64 @@ class BasemapPickerControl {{ }} const map = new maplibregl.Map(_mapInit); + // ─── Merged-style basemap bookkeeping ─── + // Which vector basemap's layers live in the permanent style (null on + // raster-init), and the ids of those layers so switches can toggle + // the whole group. + let loadedVectorBasemap = (_defaultEntry.kind === 'vector') ? DEFAULTS.basemap : null; + let vectorLayerIds = []; + + function injectRasterBasemaps() {{ + // Record which layers belong to the loaded vector style BEFORE + // injecting anything. Runs at style load (and after a legacy + // vector rebuild) — before Geoman init and asset restore — so + // everything present that isn't a raster basemap is the vector + // basemap's. gm_* filtered defensively anyway. + if (loadedVectorBasemap) {{ + vectorLayerIds = map.getStyle().layers.map(l => l.id) + .filter(id => !basemapIds.includes(id) && !id.startsWith('gm')); + }} + const firstId = (map.getStyle().layers[0] || {{}}).id; + for (const [id, entry] of Object.entries(BASEMAPS)) {{ + if ((entry.kind || 'raster') !== 'raster') continue; + // Guard on OUR layer id, and namespace the source id: vector + // styles can ship sources whose ids collide with basemap ids + // (MapTiler hybrid has a source literally named "satellite"), + // which silently blocked that basemap's injection. + if (map.getLayer(id)) continue; // raster-init already has them + const srcId = 'bm-src-' + id; + if (!map.getSource(srcId)) {{ + map.addSource(srcId, {{ + type: 'raster', + tiles: [entry.url], + tileSize: entry.tile_size || 256, + attribution: entry.attribution || '', + maxzoom: entry.max_zoom || 22, + }}); + }} + // Below everything else: user assets and vector labels + // render above an active raster basemap. + map.addLayer({{ + id: id, type: 'raster', source: srcId, + layout: {{ visibility: currentBasemap === id ? 'visible' : 'none' }}, + }}, firstId); + }} + }} + + map.once('style.load', () => {{ + injectRasterBasemaps(); + // Apply the default view mode NOW, before the first settled + // paint, not at 'load' (which waits for tiles): flipping + // mercator→globe seconds after first paint read as the map + // "resetting and zooming out", and left vector labels unplaced + // until the next interaction. + if (currentTerrain && currentTerrain !== '2d') {{ + try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} + enableSky(); + map.jumpTo({{ pitch: 0, bearing: 0 }}); + }} + }}); + // ?ui=none — naked canvas: skip every human control (basemap picker, // Geoman draw tools). The page still renders assets, replays events, // and emits the interaction contract; it just grows no chrome. @@ -1956,47 +2020,66 @@ class BasemapPickerControl {{ }} const nextKind = entry.kind || 'raster'; - if (currentBasemapKind === 'raster' && nextKind === 'raster') {{ - // Cheap path: just toggle visibility on the raster layers - currentBasemap = name; + const showVectorGroup = (visible) => {{ + for (const id of vectorLayerIds) {{ + if (map.getLayer(id)) {{ + map.setLayoutProperty(id, 'visibility', visible ? 'visible' : 'none'); + }} + }} + }}; + const showRasterLayer = (visibleId) => {{ for (const id of basemapIds) {{ const e = BASEMAPS[id]; if ((e.kind || 'raster') !== 'raster') continue; - map.setLayoutProperty(id, 'visibility', id === name ? 'visible' : 'none'); + if (map.getLayer(id)) {{ + map.setLayoutProperty(id, 'visibility', id === visibleId ? 'visible' : 'none'); + }} }} + }}; + + // Toggle-only switching within the permanent merged style: + // any raster target, or returning to the vector basemap the + // style was loaded with. Nothing is destroyed — Geoman, + // assets, and view state are untouched. + if (nextKind === 'raster') {{ + showRasterLayer(name); + showVectorGroup(false); + currentBasemap = name; + currentBasemapKind = 'raster'; + updateBasemapPickerActive(); + sendViewportUpdate(); + return; + }} + if (name === loadedVectorBasemap) {{ + showRasterLayer(null); + showVectorGroup(true); + currentBasemap = name; + currentBasemapKind = 'vector'; updateBasemapPickerActive(); sendViewportUpdate(); return; }} - // Heavy path: setStyle wipes the style, so save/restore user assets + // Legacy heavy path — ONLY for a vector style that is not the + // loaded one: setStyle wipes the world, so save/restore user + // assets. The new vector style then becomes the permanent one + // and the raster basemaps are re-injected into it. const userState = captureMapState(); - // Detach terrain BEFORE setStyle. Swapping styles with terrain - // attached corrupts MapLibre's shader cache — the painter then - // crashes every frame ("shaderPreludeCode ... undefined" in - // terrainDepth, reproduced in Firefox), the map never settles, - // and Geoman's source updates stall into literal 60-second - // timeouts (drawn shapes appear only when those expire, all at - // once). restoreMapState re-attaches terrain from the captured - // state once the new style has loaded. - try {{ map.setTerrain(null); }} catch (e) {{}} // Register the restore handler BEFORE setStyle: inline object - // styles (all raster basemaps) can fire 'style.load' - // SYNCHRONOUSLY inside setStyle, so a handler registered on - // the next line misses it and the whole user-asset/terrain + // styles can fire 'style.load' SYNCHRONOUSLY inside setStyle, + // so a handler registered on the next line misses it and the // restore silently never runs (observed in Firefox). map.once('style.load', () => {{ currentBasemap = name; - currentBasemapKind = nextKind; + currentBasemapKind = 'vector'; + loadedVectorBasemap = name; + vectorLayerIds = []; + injectRasterBasemaps(); // before restore: user layers must not be misclassified restoreMapState(userState); updateBasemapPickerActive(); sendViewportUpdate(); }}); - if (nextKind === 'vector') {{ - map.setStyle(entry.url); - }} else {{ - map.setStyle(buildRasterStyle(name)); - }} + map.setStyle(entry.url); }}, // ─── Terrain (2D/3D toggle) ─── @@ -2268,38 +2351,13 @@ class BasemapPickerControl {{ }} }} }} - // Terrain DEM source was wiped by setStyle — reset the flag so - // ensureTerrainSource re-adds it. - terrainSourceAdded = false; + // 3d mode is globe projection + sky ONLY — MapLibre's raster-DEM + // terrain is retired here (globe+terrain is the half-supported + // combination behind the shader-cache crash class; real 3D is the + // Babylon/3D-Tiles view's job). if (snap.terrain && snap.terrain !== '2d') {{ - // Re-attach terrain (the switch detached it — see set_basemap — - // to avoid MapLibre's terrain shader-cache crash). setTerrain - // right at style.load is silently rejected while the style is - // still settling, so: try now, and if it didn't stick retry on - // idle plus a timed backstop. Idempotent — first success wins. - const reattachTerrain = () => {{ - if (map.getTerrain()) return true; - try {{ - if (!map.getSource('terrain-dem')) {{ - terrainSourceAdded = false; // keep flag truthful - ensureTerrainSource(); - }} - // Terrain BEFORE projection: setTerrain issued under an - // already-globe (vertical-perspective) projection gets - // dropped; attached first, it survives the flip. - map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); - map.setProjection({{ type: 'globe' }}); - enableSky(); - }} catch (e) {{ - console.warn('Terrain re-attach attempt failed:', e.message); - return false; - }} - return !!map.getTerrain(); - }}; - if (!reattachTerrain()) {{ - map.once('idle', reattachTerrain); - setTimeout(reattachTerrain, 3000); - }} + try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} + enableSky(); }} try {{ map.jumpTo({{ @@ -2369,47 +2427,53 @@ class BasemapPickerControl {{ if (snapshot.theme) {{ applyTheme(snapshot.theme); }} - // Restore terrain mode BEFORE viewport — setProjection('globe') can - // disrupt center/zoom, so we apply terrain first, then set viewport - // on the next animation frame once the projection has settled. - // Change-only for the same reason as basemap: redundantly re-running - // setProjection + setTerrain on every reconnect races any in-flight - // style switch and can wedge the renderer (terrain + globe mid-flux - // hits MapLibre's terrainDepth painter crash — the map then never - // settles, and Geoman's source updates stall into 60s timeouts). + // Restore view mode BEFORE viewport — setProjection('globe') can + // disrupt center/zoom, so projection first, then viewport on the + // next animation frame. Change-only for the same reason as + // basemap: snapshots arrive on every reconnect and redundant + // projection churn must not happen. const snapTerrain = snapshot.terrain; if (snapTerrain && snapTerrain !== currentTerrain) {{ + // 3d = globe projection + sky only (no raster-DEM terrain — + // see restoreMapState for why it was retired). if (snapTerrain !== '2d') {{ currentTerrain = snapTerrain; try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} - ensureTerrainSource(); - map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); enableSky(); }} else {{ currentTerrain = '2d'; try {{ map.setProjection({{ type: 'mercator' }}); }} catch(e) {{}} - map.setTerrain(null); disableSky(); }} }} - // Terrain restore may have changed projection — re-arbitrate - // deck-ribbon vs flat-line for any restored arcs. + // Projection may have changed — re-arbitrate deck-ribbon vs + // flat-line for any restored arcs. syncArcMode(); - // Restore viewport AFTER terrain so globe projection doesn't - // clobber the center/zoom. Use requestAnimationFrame to let - // MapLibre finish the projection change before jumping. + // Restore viewport AFTER projection so a globe flip doesn't + // clobber the center/zoom — and ONLY when it actually differs + // from where the camera already is. Snapshots arrive on every + // (re)connect; unconditionally re-applying the stored view made + // fresh pages visibly "reset" moments after first paint. if (snapshot.viewport) {{ - requestAnimationFrame(function() {{ - if (snapshot.viewport.center) {{ + const v = snapshot.viewport; + let alreadyThere = false; + if (v.center) {{ + const c = map.getCenter(); + alreadyThere = + Math.abs(c.lng - v.center[0]) < 1e-3 && + Math.abs(c.lat - v.center[1]) < 1e-3 && + Math.abs(map.getZoom() - (v.zoom || DEFAULTS.zoom)) < 0.05; + }} + if (!alreadyThere) requestAnimationFrame(function() {{ + if (v.center) {{ map.jumpTo({{ - center: snapshot.viewport.center, - zoom: snapshot.viewport.zoom || DEFAULTS.zoom, - pitch: snapshot.viewport.pitch || 0, - bearing: snapshot.viewport.bearing || 0, + center: v.center, + zoom: v.zoom || DEFAULTS.zoom, + pitch: v.pitch || 0, + bearing: v.bearing || 0, }}); - }} else if (snapshot.viewport.bbox) {{ - const b = snapshot.viewport.bbox; - map.fitBounds([[b[0], b[1]], [b[2], b[3]]], {{ + }} else if (v.bbox) {{ + map.fitBounds([[v.bbox[0], v.bbox[1]], [v.bbox[2], v.bbox[3]]], {{ padding: 50, maxZoom: 18, }}); }} @@ -2558,17 +2622,8 @@ class BasemapPickerControl {{ console.log('Cleaned up Geoman controls, keeping only:', allowedTitles); }}, 1000); - // Apply default terrain mode from config (e.g., '3d' for globe) - // Nadir view: globe projection + terrain + sky, pitch forced to 0 (straight down) - if (currentTerrain && currentTerrain !== '2d') {{ - try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{}} - ensureTerrainSource(); - map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); - enableSky(); - // Force nadir (straight down) — pitch=0, bearing=0 - map.jumpTo({{ pitch: 0, bearing: 0 }}); - console.log('Applied default terrain mode: 3D Globe (nadir, pitch=0)'); - }} + // (Default view mode — globe projection + sky — is applied at + // 'style.load', before first paint; see the map init block.) // Escape key exits all modes document.addEventListener('keydown', function(e) {{ @@ -2794,56 +2849,35 @@ class BasemapPickerControl {{ exitDeleteMode(); }} - // ─── Terrain Mode (2D/3D) ─── - // AWS Terrain Tiles (Terrarium encoding) — free, no API key - const TERRAIN_DEM_URL = 'https://s3.amazonaws.com/elevation-tiles-prod/terrarium/{{z}}/{{x}}/{{y}}.png'; - let terrainSourceAdded = false; - - function ensureTerrainSource() {{ - if (terrainSourceAdded) return; - if (!map.getSource('terrain-dem')) {{ - map.addSource('terrain-dem', {{ - type: 'raster-dem', - tiles: [TERRAIN_DEM_URL], - tileSize: 256, - encoding: 'terrarium', - maxzoom: 15, - }}); - }} - terrainSourceAdded = true; - }} - + // ─── View Mode (2D flat / 3D globe) ─── + // '3d' is GLOBE PROJECTION + SKY only. MapLibre's raster-DEM terrain + // is retired: combined with globe (vertical perspective) it is + // explicitly half-supported upstream and was the trigger of a + // per-frame painter crash class ("shaderPreludeCode ... undefined") + // that stalled drawing into 60s Geoman timeouts. Real 3D terrain is + // the Babylon/3D-Tiles view's job. function applyTerrainMode(mode, animate) {{ currentTerrain = mode; if (mode === '3d') {{ - // Globe projection for 3D view try {{ map.setProjection({{ type: 'globe' }}); }} catch(e) {{ console.warn('Globe projection not available:', e.message); }} - ensureTerrainSource(); - map.setTerrain({{ source: 'terrain-dem', exaggeration: 1.5 }}); - // Atmospheric sky for the 3D view enableSky(); if (animate) {{ map.easeTo({{ pitch: 60, duration: 1500 }}); }} else {{ map.jumpTo({{ pitch: 60 }}); }} - console.log('Terrain mode: 3D Globe (pitch 60, terrain on, sky on, globe projection)'); + console.log('View mode: 3D Globe (pitch 60, sky on, no terrain)'); }} else {{ // 2D mode — flat Mercator try {{ map.setProjection({{ type: 'mercator' }}); }} catch(e) {{ console.warn('Projection switch failed:', e.message); }} if (animate) {{ map.easeTo({{ pitch: 0, bearing: 0, duration: 1500 }}); - // Remove terrain after animation completes - setTimeout(function() {{ - map.setTerrain(null); - disableSky(); - }}, 1600); + setTimeout(disableSky, 1600); }} else {{ map.jumpTo({{ pitch: 0, bearing: 0 }}); - map.setTerrain(null); disableSky(); }} - console.log('Terrain mode: 2D Flat (mercator)'); + console.log('View mode: 2D Flat (mercator)'); }} // Projection change flips which arc representation is visible // (deck ribbon in mercator, flat line in globe) — and forces a From 26d91affa0104b002a6de6af4e16ac2699ef5866 Mon Sep 17 00:00:00 2001 From: TommySquared Date: Mon, 14 Sep 2026 16:04:14 -0500 Subject: [PATCH 8/9] refactor: replace Geoman with Terra Draw; themed draw toolbar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Team-agreed direction: nobody was attached to maplibre-geoman-free, and its architecture was the source of the worst failure modes — internal source bookkeeping that could not survive a style rebuild (infinite retry loop, observed 20+GB Firefox tab OOM), async source-update machinery with literal 60s timeouts, and a free-tier toolbar that ignored its own config (we were hiding its buttons with DOM hacks) and never respected the light/dark theme. Terra Draw (v1.33, community-maintained, headless) inverts the design: the feature store lives in JS and the map is just a render target, so state survives anything. Integration: - Modes: polygon, rectangle, circle, linestring — mapped onto the existing draw_type protocol (polygon/box/circle/line); the SDK's enable_drawing accepts all four (was polygon/box only). - finish event → same flow as before: dashed pending copy immediately, user_drawn_feature over WS with client_id, feature removed from the draw store (the asset pipeline owns it once the echo returns). Draw mode stays active for consecutive shapes. - In-progress geometry styled from the live --eo-accent token; a MutationObserver re-styles on data-theme flips. - Toolbar is entirely ours: a MapLibre control themed with the page tokens (light/dark aware — closes the "map controls need to respect light/dark mode" item in eo-gpt#70), toggle buttons for the four draw modes plus Pan and click-to-delete. - Legacy vector-style rebuild path: Terra Draw is stop()ped before setStyle and start()ed after restore — clean round-trip, no source preservation hacks (the gm_* capture machinery is deleted). - Basemap-vs-draw layer classification keys on the adapter's td-* prefix instead of gm. Verified headless (Firefox, real mouse input): themed 6-button toolbar; rectangle draws confirm in ~0.27s on a fresh map, twice in a row without re-clicking the tool, right after a satellite toggle, and right after a legacy vector rebuild; polygon close-on-last-vertex verified; toolbar background follows theme flips; zero console errors. Merged- style suite still green (zero rebuilds on toggle switches, assets and draw state intact). Co-Authored-By: Claude Fable 5 --- server/mapcontrol_server/main.py | 499 +++++++++++++++---------------- 1 file changed, 240 insertions(+), 259 deletions(-) diff --git a/server/mapcontrol_server/main.py b/server/mapcontrol_server/main.py index b333ae3..8d96a77 100644 --- a/server/mapcontrol_server/main.py +++ b/server/mapcontrol_server/main.py @@ -395,7 +395,7 @@ def _thumbnail_url(entry) -> str: # none — naked canvas: no basemap picker, no draw tools, no embed # outfit. Only renders assets + emits interaction events # (the canonical ESIP design; what slide embeds want). - # controls — native map controls (basemap picker + Geoman draw tools + + # controls — native map controls (basemap picker + draw toolbar + # attribution) but NO embed outfit. What embedders with # their own management UI (e.g. EOGPT) want. # default — controls PLUS the built-in "default outfit" (layer panel @@ -480,7 +480,7 @@ def _thumbnail_url(entry) -> str: Map Control - + {ui_head} @@ -523,9 +523,9 @@ def _thumbnail_url(entry) -> str: The canonical ESIP design: a bare map that only renders assets and emits interaction events. No draw toolbar, no basemap picker, no custom buttons, no on-map attribution pill. JS below also skips - initializing Geoman and the picker control entirely; these rules - are the belt-and-suspenders layer for anything that still lands - in the DOM. + building the draw toolbar and the picker control entirely; these + rules are the belt-and-suspenders layer for anything that still + lands in the DOM. ATTRIBUTION CONTRACT: hiding the pill shifts the provider-credit obligation to the EMBEDDING PAGE — anyone iframing a ui=none map must render basemap attribution (e.g. "© Esri, © OpenStreetMap @@ -534,38 +534,43 @@ def _thumbnail_url(entry) -> str: html[data-esip-ui="none"] .maplibregl-ctrl-top-left, html[data-esip-ui="none"] .maplibregl-ctrl-bottom-right, html[data-esip-ui="none"] .basemap-picker-wrap, - html[data-esip-ui="none"] .custom-toolbar, - html[data-esip-ui="none"] [class*="gm-control"], - html[data-esip-ui="none"] [class*="geoman-control"] {{ + html[data-esip-ui="none"] .draw-toolbar {{ display: none !important; }} /* Custom toolbar buttons (undo, pan, delete) */ - .custom-toolbar {{ - position: absolute; - top: 10px; - right: 10px; - z-index: 1000; + /* ─── Draw toolbar (Terra Draw is headless — this is our own UI) ── + Themed via the tokens above so it follows light/dark live, exactly + like the basemap picker. One vertical group, top-left, mirroring + the MapLibre control aesthetic without depending on it. */ + .draw-toolbar {{ display: flex; flex-direction: column; - gap: 4px; + background: var(--eo-surface); + border: 1px solid var(--eo-border); + border-radius: 8px; + box-shadow: var(--eo-shadow); + overflow: hidden; }} - .custom-toolbar button {{ - width: 36px; - height: 36px; - border: 1px solid #ccc; - border-radius: 4px; - background: #fff; + .draw-toolbar button {{ + width: 32px; + height: 32px; + border: none; + border-bottom: 1px solid var(--eo-border); + background: transparent; + color: var(--eo-text-muted); cursor: pointer; - font-size: 18px; display: flex; align-items: center; justify-content: center; - box-shadow: 0 1px 4px rgba(0,0,0,0.2); - transition: background 0.15s; + transition: background 0.15s, color 0.15s; }} - .custom-toolbar button:hover {{ background: #f0f0f0; }} - .custom-toolbar button.active {{ background: #e94560; color: #fff; border-color: #e94560; }} - .custom-toolbar button:disabled {{ opacity: 0.4; cursor: default; }} + .draw-toolbar button:last-child {{ border-bottom: none; }} + .draw-toolbar button:hover {{ background: var(--eo-surface-hover); color: var(--eo-text); }} + .draw-toolbar button[aria-pressed="true"] {{ + background: var(--eo-accent-soft); + color: var(--eo-accent); + }} + .draw-toolbar button svg {{ width: 18px; height: 18px; display: block; }} .delete-mode-active {{ cursor: crosshair !important; }} /* ─── Basemap picker (collapsed dropdown) ───────────────────────── A trigger button that expands into a grouped thumbnail grid. @@ -692,7 +697,10 @@ def _thumbnail_url(entry) -> str:
- + + +