Fix map draw latency + Terra Draw swap + basemap/geotiff robustness (#70) - #10
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
restore for raster styles
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 <noreply@anthropic.com>
3d mode is globe-only (no MapLibre terrain)
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
A raster with no CRS whose bounds are projected meters (e.g. a change
raster differenced from EPSG:3832 inputs whose write dropped the CRS
profile) was assumed to be EPSG:4326, placing the overlay at
coordinates like (-819450, -487230) — off the map. The result was an
invisible "phantom": a layer present in the manager with a thumbnail
but nothing on the map (users saw the basemap through where it should
be and mistook it for the overlay). _compute_bounds_4326 now fails loud
when CRS is absent AND coordinates aren't valid lon/lat, so the event
returns a clear error and no asset is created. CRS-less rasters that
are genuinely geographic still pass through unchanged.
Verified with the real Sepik outputs: sepik_change_1990_2024.tif
(crs=None) → clear error, zero assets created; sepik_water_{1990,2024}
(EPSG:3832) → render correctly at (142.6, -4.4).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the family of map bugs behind eo-gpt#70 — most visibly "the polygon tools become unusable for up to a minute after a run displays results." What began as a latency investigation uncovered several distinct root causes plus two structural design problems, all fixed here. Every fix was reproduced and verified with a headless browser (Firefox specifically — several of these do not reproduce in Chromium).
The two structural changes (worth reviewing first)
1. One permanent style; basemap switching by visibility toggles.
map.setStyle()is a destructive full-world rebuild — it wipes every source and layer, and everything stateful (the draw tool's internals, terrain, user assets, the deck overlay) had to be captured and resurrected around it. Three of the root causes below were shards of that one collision. Now the style loaded at init is permanent: every raster basemap is injected into it (hidden, namespaced source ids), and switching among the loaded vector basemap + any raster basemap is pure visibility toggling.setStylesurvives only for switching to a different vector style (rare).2. Drawing tool: Geoman → Terra Draw.
maplibre-geoman-free's architecture was the source of the worst failures — internal source bookkeeping that could not survive a style rebuild (infinite retry loop, observed OOM-ing a Firefox tab at 20+GB), async source-update machinery with literal 60s timeouts, and a free-tier toolbar that ignored its own config and never respected light/dark. Terra Draw (v1.33, community-maintained, headless) keeps its feature store in JS with the map as a render target, so state survives anything. The toolbar is now entirely ours, themed with the page tokens — which also closes the "map controls need to respect light/dark mode" item in #70.
3d mode is now globe-only. MapLibre raster-DEM terrain is retired: globe + terrain is explicitly half-supported upstream and was the trigger of a per-frame painter crash. Real 3D terrain is planned as a separate 2.5D-mercator + Babylon/3D-Tiles effort.
Individual bug fixes (in commit order)
client_idround-trip; a newinclude_geojson=falsesummary mode on the assets endpoint (10 MB → 1 KB per poll); GeoTIFF processing moved off the event loop withasyncio.to_thread.queryRenderedFeaturesper second of mouse movement) → both hover systems are now dwell-gated and suspended while drawing.style.load) → all resolved by the structural change above plus register-handler-before-setStyle ordering.style.load.Verification
Headless Firefox with real mouse input: draws confirm in ~0.27s on a fresh map, consecutively without re-clicking, right after a satellite toggle (zero style rebuilds), and right after a legacy vector rebuild; themed toolbar follows theme flips; GeoTIFF overlays render at every zoom in globe and mercator across 4326/UTM and RGB/single-band; the real Sepik outputs confirm the CRS guard (change file → clear error, water files → correct placement). Zero console errors across all scenarios.
Notes
StreamableHTTPSessionManagertolerates one lifespan per process); present onmain, unrelated to this change. New tests were verified individually.🤖 Generated with Claude Code