Vector layer injection, styling, and picking (phase 2 of issue #1)#3
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
| if (!Array.isArray(candidate.features)) { | ||
| throw new Error("the FeatureCollection has no features array"); | ||
| } | ||
| return parsed as FeatureCollection; |
There was a problem hiding this comment.
🤖 from Claude (review)
Null / absent feature geometry crashes both rendering and picking (unhandled). parseFeatureCollection validates only the root (type === "FeatureCollection" + features is an array); it does not touch individual features. But GeoJSON permits geometry: null (an "unlocated" feature), and the key is only spec-required, not spec-non-null. Both consumers then dereference it unconditionally:
vectorPicking.tsbuildPickIndex:const geometry = feature.geometry; geometry.type === "Polygon"→TypeError: Cannot read properties of null (reading 'type')(verified with a probe).geojsonPolygons.tsgeojson2gpuPolygonFillGeometry:feature.geometry.type === "Polygon"— same throw.
The render crash is the worse one: addVectorLayerFromFile wraps only readVectorLayerFile + store.addVectorLayer in its try/catch; the actual geometry build runs later inside the deep-watch on layerStack (updateVectorLayers), outside that catch, so an injected FeatureCollection with one null-geometry feature is stored successfully, then throws unhandled in the reactive callback and leaves the scene half-synced. TypeScript hides it because parsed as FeatureCollection defaults G to non-null Geometry.
Since this is user-injected arbitrary GeoJSON, this is reachable. Fix at the root: in parseFeatureCollection, reject (or drop) features whose geometry is null/undefined or whose geometry.type is missing, and add a test. Defensively, guarding feature.geometry?.type in buildPickIndex and the fill builder would also stop the crash.
There was a problem hiding this comment.
🤖 from Claude
Fixed in 846e7f8: parseFeatureCollection now drops features whose geometry is null/absent with a single console.warn naming the count, so a mostly-good file still loads and neither the pick index nor the fill builder ever dereferences a null geometry; added a test covering a collection with null + missing-geometry features loading with only the good feature surviving.
| if (!outer || outer.length < 4) { | ||
| return null; | ||
| } | ||
| const referenceLon = outer[0][0]; |
There was a problem hiding this comment.
🤖 from Claude (review)
Picking does not skip pole-enclosing rings, but the fill does — you can hover-hit a polygon that renders no fill, and the even-odd test over it is undefined. In addPolygonFill (geojsonPolygons.ts) a ring whose longitudeWinding is >= 359.9 is explicitly skipped ("triangulates to 0 faces; skip it") so no fill is drawn. buildPickPolygon here has no such guard — it only checks outer.length < 4, then builds a bbox and adds the ring to the pick index. Consequences:
- The visual/pick contract diverges: hovering inside a pole-enclosing outline reports a feature whose fill is intentionally absent.
- Worse, the even-odd
pointInRingtest is run in unwrapped-longitude space on a ring that winds a full 360° of longitude — the unwrapped contour doubles back on itself, so the inside/outside result there is not well-defined (the same degeneracy that makes the fill un-triangulatable). So the hit is not just visually inconsistent, it is unreliable.
Fix: mirror the phase-1 guard — compute longitudeWinding (or reuse it) on the outer ring in buildPickPolygon and return null when |winding| >= 359.9, so picking and rendering agree on which rings are renderable. Add a pole-ring case to vectorPicking.test.ts.
There was a problem hiding this comment.
🤖 from Claude
Fixed in b8e29ac: exported the phase-1 longitudeWinding helper from geojsonPolygons.ts and reused it in buildPickPolygon, which now returns null when |winding| >= 359.9, so picking and rendering agree on which rings are renderable; added a pole-ring case to vectorPicking.test.ts asserting no hit anywhere near the ring.
| await addVectorLayerFromFile(file); | ||
| } else { | ||
| logError( | ||
| new Error("Only .geojson/.json FeatureCollections can be dropped"), |
There was a problem hiding this comment.
🤖 from Claude (review)
Drag-drop rejects the very texture files the Upload button accepts, with a misleading error. This new window drop handler routes only isSupportedVectorLayerFile(file) to injection and toasts "Only .geojson/.json FeatureCollections can be dropped" for everything else. But the Upload button (onFileSelected) accepts PNG/JPEG/GeoTIFF and GeoJSON, and there was no window-drop handler before this PR — so dropping a PNG/GeoTIFF, which a user reasonably expects to work since the button takes it, now surfaces an error implying drops are GeoJSON-only.
If routing dropped images through the texture path (loadImageTexture + saveTexture) is out of scope for phase 2, that is defensible, but then at least do not toast an error for files the app otherwise accepts — silently ignore non-vector drops, or word the message so it does not read as "drops are GeoJSON-only." As written it is an asymmetry a user will hit on the first GeoTIFF drop. (Flagging against PR judgment-call 5, which chose error-over-silence but did not note this upload/drop mismatch.)
There was a problem hiding this comment.
🤖 from Claude
Fixed in db15310 (full-routing option, ~8 lines): the window drop handler now routes GeoJSON to the vector path and PNG/JPEG/GeoTIFF to the texture path (saveTexture + store.addTextureLayer, mirroring LayerPanel onFileSelected), and only genuinely unsupported extensions toast, with a message listing both families ("PNG/JPEG/GeoTIFF images or GeoJSON files").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax
Refs #1 (phase 2)
Phase 2 of the viewer plan: user-injected GeoJSON vector layers with per-layer styling and hover picking. Acceptance target: drag in / point at a zagg
shard_outlines/granule_footprintsGeoJSON with zero code.What this does
Injection (three paths, all id-keyed so multiple concurrent layers work):
.geojson/.json(application/geo+json,application/json); dispatch between texture and vector goes by file type, mirroring the texture-layer flow.GlobeView.vue; only activates for drags carryingFiles, so it never interferes with the panel's layer-reorder drags).PopupDialogpattern) fetches and validates a FeatureCollection.Validation lives in
src/lib/layers/vectorLayerFormats.ts(mirrorstextureLayerFormats.ts): malformed JSON, non-FeatureCollection root, missingfeaturesarray, and a 50 MB size cap (checked againstFile.size/Content-Length/ body length before parse). Errors surface through the house toast pattern (useLog.logError). Layer names come from the filename / URL basename.Styling controls (
LayerPanel.vueper vector entry): fill color, fill opacity, stroke color in a style popover; a delete button removes the layer. Style state is avectorStylefield on the layer entry (TVectorLayerStyle, defaults = the phase-1 constants, nowVECTOR_LAYER_STYLE_DEFAULTSin the store). The scene sync applies style changes as pure uniform updates (applyVectorLayerStyleinuseGridOverlays.ts:fillColor/fillOpacityon the fill material,lineColoron the outline material) — geometry is never rebuilt on style change.Hover/picking with feature-properties readout: hovering a vector polygon (with hover enabled, same toggle as the grid readout) shows the feature's
propertiesas key/value lines — shard id, granule id for the zagg case — viaVectorHoverReadout.vue, which mirrors the gridHoverReadoutpresentation and sits below the pointer (the grid readout sits above, so both can show). Topmost visible vector layer wins; within a collection the last-drawn matching feature wins.Picking approach: raycaster to the surface, point-in-polygon in data space
Open question 1 on #1 (picking tech) pre-decided the three.js raycaster over GPU id-picking — right-sized for shard counts of 10²–10⁴ features. Within that, the pick is not done by raycasting the vector fill meshes: their CPU-side
positionattribute is baked at build time for the then-current projection, and projection switches update only shader uniforms (that's the phase-1 design — no geometry rebuilds), so the meshes' CPU geometry goes stale the first time the projection changes and intersections would be wrong.Instead the existing scene raycast is reused:
useGridScenealready raycasts the pointer against the pick surface and inverts to lat/lon (hoveredGeoPoint) for both globe and flat projections.useVectorFeatureHovermaps that lat/lon through an even-odd point-in-polygon test against the source FeatureCollection (src/lib/layers/vectorPicking.ts). This is projection-independent by construction, needs no per-triangle feature-index attribute (feature identity falls out of the data), and is cheap at shard scale (bbox prefilter + a WeakMap-cached pick index per collection). Longitudes are unwrapped per polygon against its first vertex — the same convention as the fill triangulation — so antimeridian-crossing polygons pick correctly.Phases
How tested
npm run test— 51 passing (35 baseline + 16 new): FeatureCollection validation (malformed JSON, wrong root, missing features array, oversize), URL basename naming, point-in-polygon picking (inside/outside/hole/MultiPolygon/antimeridian/topmost-overlap/degenerate rings), store style defaults + clamped updates + hovered-feature state. All CPU-only, no WebGL.npm run typecheck,npm run lint-ci(max-warnings 0),npm run build— green.Questions for review
?vector_sampledev hook removed (devVectorSample.ts+ theLayerPanelmount hook): the URL/file/drop injection paths now cover its purpose with zero code. Restore it if you want a one-keystroke dev fixture.🤖 Generated with Claude Code
https://claude.ai/code/session_01N32Mx4rWhbTK8xXm7Mt2ax