From 282e98352f1aabf0a657e1df6f796f4404e1abf8 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 09:57:45 -0400 Subject: [PATCH 01/10] feat(web): make INFORM "additional information" callouts an opt-in toggle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SY(INFORM01) "additional information available" box-on-a-leader markers are baked display-category Other, so enabling Other dumped an (i) marker on every feature carrying INFORM/TXTDSC/etc. — too much clutter on dense charts. Give them their own opt-in mariner toggle ("Information callouts", OFF by default), the same treatment CHDATD01 / data-quality already get: a client-side filter on the baked symbol_name, so it toggles live with no re-bake. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/chart-canvas/chart-canvas.mjs | 2 +- web/src/chart-canvas/s52-style.mjs | 5 +++++ web/src/chartplotter.mjs | 6 ++++++ web/src/core/core-settings.mjs | 1 + 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/web/src/chart-canvas/chart-canvas.mjs b/web/src/chart-canvas/chart-canvas.mjs index 00da080..b1c0df5 100644 --- a/web/src/chart-canvas/chart-canvas.mjs +++ b/web/src/chart-canvas/chart-canvas.mjs @@ -970,7 +970,7 @@ export class ChartCanvas extends HTMLElement { // Display category (multi-select) and boundary symbolization both filter // every chart layer by a baked per-feature tag (cat / bnd) — re-apply the // combined feature filter. Instant — no re-bake. - if (keys.some((k) => k === "displayBase" || k === "displayStandard" || k === "displayOther" || k === "boundaryStyle" || k === "simplifiedPoints" || k === "showFullSectorLines" || k === "showIsolatedDangersShallow" || k === "dataQuality" || k === "showMetaBounds" || k === "dateDependent" || k === "dateView" || k === "highlightDateDependent")) { + if (keys.some((k) => k === "displayBase" || k === "displayStandard" || k === "displayOther" || k === "boundaryStyle" || k === "simplifiedPoints" || k === "showFullSectorLines" || k === "showIsolatedDangersShallow" || k === "dataQuality" || k === "showMetaBounds" || k === "dateDependent" || k === "dateView" || k === "highlightDateDependent" || k === "showInformCallouts")) { this.applyFeatureFilters(); } } diff --git a/web/src/chart-canvas/s52-style.mjs b/web/src/chart-canvas/s52-style.mjs index 54161a0..1b0a592 100644 --- a/web/src/chart-canvas/s52-style.mjs +++ b/web/src/chart-canvas/s52-style.mjs @@ -322,6 +322,11 @@ export function combineFilters(base, mariner) { // shown only when the mariner enables it. (Out of period it is already gone via // the date filter above; this only governs in-period features.) if (!mariner.highlightDateDependent) parts.push(["!=", ["coalesce", ["get", "symbol_name"], ""], "CHDATD01"]); + // Information callouts (S-52 §10.6.1.1 INFORM01, viewing group 31030): the + // "additional information available" box-on-a-leader marker is baked category + // Other, but its own opt-in toggle (OFF by default) — so enabling Other doesn't + // bury a dense chart under (i) markers. Hidden unless showInformCallouts is on. + if (!mariner.showInformCallouts) parts.push(["!=", ["coalesce", ["get", "symbol_name"], ""], "INFORM01"]); // Meta-object coverage/region boundary lines are gated separately from the // "Other" display category (mariner.showMetaBounds, off by default), since // they read as cell boundaries and aren't useful alongside other "Other" data. diff --git a/web/src/chartplotter.mjs b/web/src/chartplotter.mjs index 1a99f18..f2eaaea 100644 --- a/web/src/chartplotter.mjs +++ b/web/src/chartplotter.mjs @@ -85,6 +85,12 @@ const DEFAULT_MARINER = { // marker on in-period date-dependent features — an optional highlight, off by // default (opt-in), like the info/document highlights. highlightDateDependent: false, + // "Information callouts" (S-52 §10.6.1.1 INFORM01 / viewing group 31030): the + // box-on-a-leader "additional information available" marker on features carrying + // INFORM/TXTDSC/etc. Baked display-category Other, but given its own opt-in toggle + // (OFF by default) so enabling Other isn't buried under (i) markers on dense + // charts — same treatment as highlightDateDependent. See combineFilters. + showInformCallouts: false, // S-52 PresLib §14.5 text groupings — the mariner toggles text by group, // independent of display category (each TX/TE carries a group number, §14.4). showLightDescriptions: true, // group 23: light characteristics (e.g. Fl(2)R 10s) diff --git a/web/src/core/core-settings.mjs b/web/src/core/core-settings.mjs index 8a75009..9c0c112 100644 --- a/web/src/core/core-settings.mjs +++ b/web/src/core/core-settings.mjs @@ -82,6 +82,7 @@ export function coreSettingsContributions(app) { { key: "showFullSectorLines", type: "toggle", label: "Full sector lines", desc: "Draw light sectors to full range, not short stubs" }, { key: "showIsolatedDangersShallow", type: "toggle", label: "Isolated dangers (shallow)", desc: "Only flag isolated dangers in shallow water" }, { key: "dataQuality", type: "toggle", label: "Data quality", desc: "Survey zones-of-confidence overlay" }, + { key: "showInformCallouts", type: "toggle", label: "Information callouts", desc: "“Additional information available” (i) markers on features that carry notes" }, { key: "showMetaBounds", type: "toggle", label: "Metadata boundaries", desc: "Chart coverage & region indicator lines" }, { key: "showScaleBoundaries", type: "toggle", label: "Scale boundaries", desc: "Outline where more detailed charts exist", default: true }, { From a9c8afde37d6c64d50c52a5cf25abf4d907cda17 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 10:28:28 -0400 Subject: [PATCH 02/10] fix(portrayal): stop ISODGR over-triggering on sub-contour dangers in safe water MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UDWHAZ05 flags an isolated danger when the feature depth is below the safety contour AND its surrounding water is itself safe (or unknown). DerivedAttrs only supplied defaultClearanceDepth, never surroundingDepth, so the rule always took its conservative "unknown surrounding ⇒ dangerous" branch and stamped a magenta ISODGR01 X on every sub-contour OBSTRN/WRECKS/UWTROC — including the "area of wrecks or obstructions in safe waters" (Chart 1 page 244), which is explicitly NOT dangerous. Supply surroundingDepth = the shoalest DRVAL1 of the containing depth area, ONLY when such an area is found; with no containing area it stays absent so the rule's conservative default still flags deep/unknown dangers. Verified on page 244: the safe-water area loses its X while "foul area dangerous for navigation" keeps it. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/portrayal/s101depth.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/engine/portrayal/s101depth.go b/internal/engine/portrayal/s101depth.go index c2f3ac7..b6d8880 100644 --- a/internal/engine/portrayal/s101depth.go +++ b/internal/engine/portrayal/s101depth.go @@ -87,12 +87,25 @@ func DerivedAttrs(f *s57.Feature, idx *DepthIndex) map[string]string { return nil } depth := 0.0 + surrounding := "" if pt, ok := representativePoint(f); ok { if d, ok := idx.shoalestDRVAL1(pt.Lat, pt.Lon); ok { depth = d + // surroundingDepth is the depth of the area the danger sits in (S-52 + // DEPVAL). UDWHAZ05 reads it: a sub-safety-contour danger whose + // surrounding water is itself shallow is NOT an isolated danger and + // must not get ISODGR01 unless "isolated dangers in shallow water" is + // on. Supply it ONLY when a containing depth area is found — leaving it + // absent (the no-area case) keeps the rule's conservative "unknown ⇒ + // dangerous" default, so deep/unknown dangers still flag. + surrounding = strconv.FormatFloat(d, 'f', -1, 64) } } - return map[string]string{"defaultClearanceDepth": strconv.FormatFloat(depth, 'f', -1, 64)} + out := map[string]string{"defaultClearanceDepth": strconv.FormatFloat(depth, 'f', -1, 64)} + if surrounding != "" { + out["surroundingDepth"] = surrounding + } + return out } // polygonRings returns a polygon's rings as [lon,lat] lists (Rings field first, From 0115275dd9f1c0d170c9fafa12eb09e8d1d5451a Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 10:28:28 -0400 Subject: [PATCH 03/10] fix(web): render producer-placed NEWOBJ/SYMINS text on an always-on layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S-52 §10.3.3.8 producer text instructions must always render. The PresLib "ECDIS Chart 1" two-line captions ("restricted area," + "anchoring prohibited") are encoded as TWO point features stacked one line apart, each a single-line SYMINS TX. The shared general `text` layer declutters (text-allow-overlap:false + text-optional), so the two adjacent boxes collided and the lower line was dropped — looking truncated. Give NEWOBJ text its own always-on layer (allow-overlap + ignore-placement), mirroring the existing light-text layer, and exclude NEWOBJ from the collidable general layer to avoid double-draw. Real geographic labels (not NEWOBJ) stay decluttered, and text-max-width:40 is untouched so single-line labels never wrap. Client-only, no re-bake. Verified on page 246: all five second lines now show. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/chart-canvas/chart-style.mjs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/web/src/chart-canvas/chart-style.mjs b/web/src/chart-canvas/chart-style.mjs index 5612450..7a4e2cd 100644 --- a/web/src/chart-canvas/chart-style.mjs +++ b/web/src/chart-canvas/chart-style.mjs @@ -84,7 +84,7 @@ function textLayers(mariner, palette) { const notLight = ["!=", ["get", "class"], "LIGHTS"]; return [{ id: "text", type: "symbol", source: "chart", "source-layer": "text", - filter: ["all", notLight, S52.textGroupFilter(mariner)], + filter: ["all", notLight, ["!=", ["get", "class"], "NEWOBJ"], S52.textGroupFilter(mariner)], layout: { "text-field": ["coalesce", ["get", "text"], ""], "text-font": FONT, "text-size": ["coalesce", ["get", "font_size_px"], 11], @@ -105,6 +105,30 @@ function textLayers(mariner, palette) { "text-halo-width": 1.4, "text-halo-blur": 0.5, }, + }, { + // Producer-placed text (NEWOBJ + SYMINS TX/TE, S-52 §10.3.3.8 — e.g. the PresLib + // "ECDIS Chart 1" legend captions): the producer's EXPLICIT instruction, which + // must always render. Two-line captions ("restricted area," + "anchoring + // prohibited") are TWO separate point features stacked one line apart; the + // general collidable layer above declutters them and drops the lower line. Honour + // them on their OWN always-on layer (text-allow-overlap), mirroring "light-text". + // text-max-width 40 is unchanged, so genuine single-line labels still never wrap. + id: "placed-text", type: "symbol", source: "chart", "source-layer": "text", + filter: ["all", ["==", ["get", "class"], "NEWOBJ"], S52.textGroupFilter(mariner)], + layout: { + "text-field": ["coalesce", ["get", "text"], ""], "text-font": FONT, + "text-size": ["coalesce", ["get", "font_size_px"], 11], + "text-anchor": TEXT_ANCHOR, + "text-max-width": 40, + "text-allow-overlap": true, "text-ignore-placement": true, + visibility: "visible", + }, + paint: { + "text-color": S52.textColor(active, palette), + "text-halo-color": S52.textHaloColor(active), + "text-halo-width": 1.4, + "text-halo-blur": 0.5, + }, }]; } // Multiply every PIXEL-VALUED size property by `k` so S-52 features render at From 72eff4e138559267fd65ec8e6a47f6d973cae89a Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 11:09:38 -0400 Subject: [PATCH 04/10] fix(bake): suppress coarse cross-band lines over derived-coverage cells MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per S-52 §10.1.4 the largest-scale data takes precedence; a coarser-band line drawn where a finer cell covers the same ground double-draws (no opaque fill hides a stroke). coverageScaleAt skipped cells with only a DERIVED extent rectangle (no M_COVR — e.g. the PresLib "ECDIS Chart 1" cells) for every non-point query, so the overview band's pipelines/contours/coastlines bled across the harbor panels live. Let derived coverage suppress coarse LINES (the centre test is hole-safe — a line in a genuine gap still shows), while still EXCLUDING it from FILL suppression so a coarser fill keeps filling real gaps (§10.1.4) instead of punching no-data holes. Renamed the flag pointQuery→includeDerived for what it now means. Verified live; the per-cell preslib harness can't show it (one cell, one band) so the static render diff is 0 — that's expected, not a no-op. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/bake/bake.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/internal/engine/bake/bake.go b/internal/engine/bake/bake.go index d439390..388e23e 100644 --- a/internal/engine/bake/bake.go +++ b/internal/engine/bake/bake.go @@ -1729,7 +1729,12 @@ func (b *Baker) emitTileInto(coord tile.TileCoord, extent uint32, buffer float64 // coarse line by the tile CENTRE (a line spans the tile, so the centre is // its representative point): it yields only where the centre has no finer // cell — best-available where the finer cell genuinely carries no data. - if s := b.coverageScaleAt(ctrLat, ctrLon, bandZ, false); s != 0 && s < r.cscl { + // includeDerived=true: a coarse line over a finer cell with only a derived + // extent (no M_COVR, e.g. the PresLib Chart-1 cells) still double-draws + // across bands and must be suppressed (S-52 §10.1.4 largest-scale wins). + // Lines never punch no-data fill holes, so this is hole-safe for a derived + // rect too — and it removes the live cross-band line bleed on Chart 1. + if s := b.coverageScaleAt(ctrLat, ctrLon, bandZ, true); s != 0 && s < r.cscl { suppressed = true } default: @@ -2104,7 +2109,7 @@ func (b *Baker) coverageBandAt(lat, lon float64) uint32 { // across bands AND between cells of different scale that fall in the SAME band (the // per-band coverageBandAt above can't distinguish those). bandZ-gated so a finer // cell that isn't shown yet at this zoom doesn't punch a hole in the coarser one. -func (b *Baker) coverageScaleAt(lat, lon float64, bandZ uint32, pointQuery bool) uint32 { +func (b *Baker) coverageScaleAt(lat, lon float64, bandZ uint32, includeDerived bool) uint32 { var best uint32 // 0 = none found yet; otherwise the finest (smallest) cscl p := geo.LatLon{Lat: lat, Lon: lon} for i := range b.covMeta { @@ -2112,8 +2117,18 @@ func (b *Baker) coverageScaleAt(lat, lon float64, bandZ uint32, pointQuery bool) if cm.cscl == 0 || cm.displayMin > bandZ { continue // unscaled, or this cell isn't drawn at this zoom } - if cm.derived && !pointQuery { - continue // a derived extent rectangle suppresses points, not fills (see covMeta.derived) + if cm.derived && !includeDerived { + // A derived rectangle marks where a cell IS, not where it has DATA, so it + // over-claims coverage. Per S-52 §10.1.4 a coarser FILL must remain to fill + // genuine gaps in the finer data (the finer fill occludes it on top where it + // has data) — so derived coverage must NOT suppress fills, or it would punch + // no-data holes inside the finer cell's sparse interior. POINTS and LINES are + // different: a coarse point/line drawn where a finer chart covers violates the + // "largest-scale data takes precedence" rule and double-draws across bands (no + // opaque fill hides it), so callers pass includeDerived=true for those. NB: the + // per-cell preslib harness can't show this (it frames one cell, one band), but + // it IS visible live when bands overlap — don't be fooled by a 0-pixel diff. + continue } if best != 0 && cm.cscl >= best { continue // not finer than the best so far — skip the costly point test From 745809ca57d4dfd4ae8b08993752b450f4bc3ad6 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 11:54:33 -0400 Subject: [PATCH 05/10] fix(web): restore crosshair + cursor-pick (overlay-load guard bailed permanently) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "don't add the catalog overlay before the style finishes loading" guard added an `if (!map.isStyleLoaded()) return` at the TOP of addCatalogOverlay — but that method's tail also wires the one-time map interaction listeners (the ECDIS crosshair cursor, the click → cursor-pick, coverage tap-to-fly). When the first onReady call hit a mid-rebuild style the guard bailed, and if no later style.load re-fired the whole block never ran: no crosshair, no pick, no coverage (focus source absent, _catalogMapWired stayed false). Fix: call addCatalogOverlay only on a LOADED style — directly when it already is, else on style.load — and drop the internal isStyleLoaded-and-return. This still avoids the "Style is not done loading" addSource throw, but never skips the wiring. Verified on the live app: crosshair restored, _catalogMapWired/coverage/focus all true, pick handler attached again. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/chartplotter.mjs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/web/src/chartplotter.mjs b/web/src/chartplotter.mjs index f2eaaea..a1a96a5 100644 --- a/web/src/chartplotter.mjs +++ b/web/src/chartplotter.mjs @@ -542,15 +542,18 @@ export class ChartPlotter extends HTMLElement { // Best-effort: if the style is mid-rebuild this no-ops (or throws on older // maps) — either way the style.load handler below re-adds the overlay once the // fresh style is ready, so never let it skip registering that listener. - try { this.addCatalogOverlay(map); } catch (e) { console.warn("[overlay] deferring to style.load:", e); } - // The plotter rebuilds the whole style (setStyle) when server sets load or the - // SCAMIN buckets refresh, wiping every app-added overlay (coverage boxes, pick & - // inspect highlights). Re-apply them after each rebuild, and repopulate the - // coverage boxes — otherwise they vanish the moment a set renders. - map.on("style.load", () => { - this.addCatalogOverlay(map); - this._refreshInstalledBounds(); - }); + // Add the app overlay (focus/inspect/pick sources + coverage) AND wire the map + // interaction listeners (crosshair cursor, click → ECDIS pick). addSource throws + // if the style isn't loaded, so call it only WHEN the style is ready: now if it + // already is, plus on every style.load (the plotter rebuilds the whole style on + // server-set load + SCAMIN refresh, wiping app-added overlays — re-apply each + // time). This avoids the "Style is not done loading" throw WITHOUT ever skipping + // the wiring — an earlier isStyleLoaded()-guard-and-RETURN inside addCatalogOverlay + // left the cursor/pick/coverage permanently unwired whenever the first call hit a + // mid-rebuild style and no later style.load re-fired. + const ensureOverlay = () => { this.addCatalogOverlay(map); this._refreshInstalledBounds(); }; + if (map.isStyleLoaded()) ensureOverlay(); + map.on("style.load", ensureOverlay); // Hold the 1:MIN_DETAIL_SCALE max-zoom floor on EVERY view change. It's // latitude-dependent (recompute as the centre moves), and a fly-to-chart raises // the cap to reach a pack's detail — without re-enforcing here that raised cap @@ -1102,13 +1105,10 @@ export class ChartPlotter extends HTMLElement { // change and SCAMIN-bucket refresh, which drops all these app-added sources/ // layers. A style.load handler (see onReady) re-invokes this against the fresh // style; the guard makes a redundant call (when the overlay is still present) a - // no-op so we never double-add. - // - // The style may still be REBUILDING when this first runs from onReady (a - // setStyle for the physical-scale restage / SCAMIN buckets can be in flight - // after the awaited catalog load) — addSource would throw "Style is not done - // loading". Bail; the onReady style.load handler re-invokes us once it's ready. - if (!map.isStyleLoaded()) return; + // no-op so we never double-add. The caller (onReady) only invokes this on a + // LOADED style — directly if ready, else on style.load — so addSource never + // throws "Style is not done loading"; do NOT add an isStyleLoaded()-guard return + // here (it silently skipped the cursor/pick/coverage wiring at the tail). if (map.getSource("focus")) return; const empty = { type: "FeatureCollection", features: [] }; map.addSource("focus", { type: "geojson", data: empty }); From 93b41657886b2bb925071d62f079cee4cea8c548 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 12:11:46 -0400 Subject: [PATCH 06/10] fix(server): track which cells go into each pack; /api/cells?active uses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search box (active charts) showed nearly the whole NOAA catalog: /api/cells?active filtered the cell index by bbox-overlap with each enabled pack's bounding box, but a pack whose cells are scattered worldwide has an effectively GLOBAL bbox, so every cached-but-not-baked cell in ENC_ROOT matched (7408/7413 "active"). The server had no record of which cells a pack was actually baked from. Record it: bakeAndRegister writes a per-pack cell manifest (/.cells.json) from the exact bake set. serveCells?active now returns cells that are IN an enabled pack's manifest; it falls back to bbox-overlap only for legacy packs baked before this (re-importing such a pack writes its manifest and moves it onto the exact path). Verified: import 14 cells + inject a cached-not-baked cell → active=14 (excludes it), all=15. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/server/import.go | 43 ++++++++++++++++++++++++++++++ internal/engine/server/tilesets.go | 37 ++++++++++++++++++------- 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/internal/engine/server/import.go b/internal/engine/server/import.go index 39334cf..34378d5 100644 --- a/internal/engine/server/import.go +++ b/internal/engine/server/import.go @@ -11,6 +11,7 @@ import ( "net/http" "os" "path/filepath" + "sort" "strings" "sync" "time" @@ -539,6 +540,12 @@ func (s *Server) bakeAndRegister(jobID, set string, cells map[string]baker.CellD if err := s.writeAndRegister(bandSet, pb, bandAux); err != nil { return err } + // Record which cells went into this pack (beside its pmtiles), so + // /api/cells?active returns exactly the installed cells — not every + // cached cell that overlaps the pack's (often global) bounding box. + if err := s.writeSetCells(bandSet, cells); err != nil { + log.Printf("import %s: cell manifest %q: %v", jobID, bandSet, err) + } first = false bands++ tiles += pb.Count() @@ -590,6 +597,42 @@ func (s *Server) setDir(set string) string { return filepath.Join(s.cacheDir, "import") } +// writeSetCells records the cell stems baked into `set` beside its pmtiles +// (/.cells.json). /api/cells?active reads these to return exactly the +// installed cells, instead of every cached cell whose bounds overlap the pack's +// (often global, for a worldwide-scattered import) bounding box. +func (s *Server) writeSetCells(set string, cells map[string]baker.CellData) error { + stems := make([]string, 0, len(cells)) + for n := range cells { + stems = append(stems, strings.TrimSuffix(n, ".000")) + } + sort.Strings(stems) + dir := s.setDir(set) + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + b, err := json.Marshal(stems) + if err != nil { + return err + } + return os.WriteFile(filepath.Join(dir, set+".cells.json"), b, 0o644) +} + +// setCells reads the cell-stem manifest written by writeSetCells for `set`, or nil +// (with ok=false) if the pack has none — a legacy pack baked before per-pack cell +// tracking, for which the caller falls back to bbox-overlap. +func (s *Server) setCells(set string) ([]string, bool) { + data, err := os.ReadFile(filepath.Join(s.setDir(set), set+".cells.json")) + if err != nil { + return nil, false + } + var stems []string + if json.Unmarshal(data, &stems) != nil { + return nil, false + } + return stems, true +} + // writeAndRegister writes the baked archive to /.pmtiles atomically // (temp + rename), writes the companion .aux.zip beside it (TXTDSC/PICREP, via // the auxfiles package), and registers the set (replacing any prior one). diff --git a/internal/engine/server/tilesets.go b/internal/engine/server/tilesets.go index 0256cd1..41e4043 100644 --- a/internal/engine/server/tilesets.go +++ b/internal/engine/server/tilesets.go @@ -299,9 +299,10 @@ func (s *Server) handleSetEnabled(w http.ResponseWriter, r *http.Request) { // un-indexed cell has no footprint to test or fly to). func (s *Server) serveCells(w http.ResponseWriter, r *http.Request) { active := r.URL.Query().Get("active") == "1" - var enabled [][4]float64 + var inPack map[string]bool // cells baked into enabled packs (exact, from manifests) + var legacy [][4]float64 // bounds of enabled packs WITHOUT a manifest (bbox fallback) if active { - enabled = s.enabledPackBounds() + inPack, legacy = s.enabledPackCells() } _, idx := s.cellIdx.snapshot() entries, _ := os.ReadDir(filepath.Join(s.dataDir, "ENC_ROOT")) @@ -313,8 +314,13 @@ func (s *Server) serveCells(w http.ResponseWriter, r *http.Request) { } n := e.Name() box, has := idx[n] - if active && (!has || !bboxOverlapsAny(box, enabled)) { - continue + if active { + // Active = actually baked into an enabled pack. Prefer the exact per-pack + // cell manifest; fall back to bbox-overlap only for legacy packs without + // one (so a globe-spanning import doesn't drag in every cached cell). + if !(inPack[n] || (has && bboxOverlapsAny(box, legacy))) { + continue + } } names = append(names, n) if has { @@ -329,21 +335,32 @@ func (s *Server) serveCells(w http.ResponseWriter, r *http.Request) { }{names, boxes}) } -// enabledPackBounds is each enabled pack's [W,S,E,N] (read from its archive), -// used by the ?active filter to test which cells are currently on the map. -func (s *Server) enabledPackBounds() [][4]float64 { - var out [][4]float64 +// enabledPackCells reports which cells are "active" (on the map). It returns (a) the +// union of cell stems recorded for each ENABLED pack that has a cell manifest (written +// at bake time by writeSetCells) — the exact installed set — and (b) the [W,S,E,N] of +// each enabled pack with NO manifest (a legacy pack baked before per-pack cell +// tracking), for the ?active filter to fall back to bbox-overlap on. Re-baking a +// legacy pack (re-import) writes its manifest and moves it onto the exact path. +func (s *Server) enabledPackCells() (map[string]bool, [][4]float64) { + cells := map[string]bool{} + var legacy [][4]float64 for _, name := range sortedKeys(s.packs) { if s.prefs.isDisabled(name) { continue } + if stems, ok := s.setCells(name); ok { + for _, st := range stems { + cells[st] = true + } + continue + } if src, err := tilesource.Open(s.packs[name]); err == nil { m := src.Meta() _ = tilesource.Close(src) - out = append(out, [4]float64{m.W, m.S, m.E, m.N}) + legacy = append(legacy, [4]float64{m.W, m.S, m.E, m.N}) } } - return out + return cells, legacy } // bboxOverlapsAny reports whether [W,S,E,N] box intersects any of the rects. From fe748be0011d9f0c7b7051db34d98327e75145ea Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 12:39:23 -0400 Subject: [PATCH 07/10] feat(web): precise DOM tap pads for INFORM01 info callouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cursor-pick can't "own" an info callout: the SY(INFORM01) box floats offset from the feature, but the symbol's hit quad is centred on the feature, so the fuzzy queryRenderedFeatures makes the whole symbol area "close enough" and symbol declutter / z-order drops some boxes entirely ("some work, some don't"). Overlay a transparent, exactly-positioned DOM pad on each visible box instead — a real clickable element (like the AIS-target Markers), placed via the feature's baked icon `scale` + the live size-scale so it tracks the rendered box. Tap → showInfoForFeature opens that object's additional information; tapping the feature itself still picks the feature. Sparse by design (only info-bearing features), so DOM markers are affordable here — unlike the general pick, which must stay on GPU queryRenderedFeatures. Follows the "Information callouts" toggle for free (symbol unrendered → no pads). Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/chartplotter.mjs | 24 ++++++ web/src/plugins/info-callouts.mjs | 120 ++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 web/src/plugins/info-callouts.mjs diff --git a/web/src/chartplotter.mjs b/web/src/chartplotter.mjs index a1a96a5..c486dd1 100644 --- a/web/src/chartplotter.mjs +++ b/web/src/chartplotter.mjs @@ -27,6 +27,7 @@ import { ConnectionsController } from "./plugins/connections.mjs"; // NMEA0183 d import { VesselStateStore } from "./data/vessel-state-store.mjs"; // live NMEA0183 vessel state (own-ship/AIS/HUD feed) import { OwnShip } from "./plugins/own-ship.mjs"; // own-ship marker + course predictor + follow camera import { AISOverlay } from "./plugins/ais-overlay.mjs"; // AIS targets (other vessels) from the live feed +import { InfoCallouts } from "./plugins/info-callouts.mjs"; // precise DOM tap pads on INFORM01 info-callout boxes import "./plugins/target-info.mjs"; // defines (own-ship / AIS tap-info picker) import { PALETTE_DAY_ICON, PALETTE_DUSK_ICON, PALETTE_NIGHT_ICON } from "./lib/openbridge-icons.mjs"; // OpenBridge scheme glyphs import { DISTRICTS, NOAA_ENC_URL } from "./plugins/chart-library.mjs"; // NOAA CG-district packs + ENC page (shared) @@ -661,6 +662,15 @@ export class ChartPlotter extends HTMLElement { this._ownShip = new OwnShip({ map, plotter: this._plotter, vessel: this._vessel, host: this.shadowRoot, onSelect: showInfo, units: () => this._mariner }); // AIS targets (other vessels) from the live feed. this._ais = new AISOverlay({ map, assets: this._assets, widget: this._widget, onSelect: showInfo, units: () => this._mariner }); + // Precise DOM tap pads on the INFORM01 "additional information" callout boxes + // (the box floats offset from the feature, so the fuzzy symbol pick can't own + // it). Sparse by nature — only info-bearing features — so DOM markers are fine. + this._infoCallouts = new InfoCallouts({ + map, + getSizeScale: () => (this._plotter && this._plotter._featureSizeScale ? this._plotter._featureSizeScale() : 1), + atlasPpu: (this._plotter && this._plotter._atlasPpu) || 0.08, + onSelect: (f) => this.showInfoForFeature(f), + }); } // Persist the view so a refresh resumes where you were; refresh the coverage @@ -1298,6 +1308,20 @@ export class ChartPlotter extends HTMLElement { el.show(uniq, ev ? { x: ev.clientX, y: ev.clientY } : null); } + // Open the pick report for ONE feature — the info-callout pads (InfoCallouts) call + // this when their box is tapped, so a callout surfaces exactly its own object's + // additional information rather than a cursor-pick of whatever's under the box. + showInfoForFeature(f) { + if (!f) return; + const el = this._ensurePickEl(); + if (!el) return; + f._hiGeom = f.geometry; + el.setCatalogue(this._s57cat); + el.setAux(this._aux); + el.setUnits(this._mariner); + el.show([f], null); + } + // Create the cursor-pick panel on first use and bridge it to the map highlight. // Returns null if hasn't been defined (module failed to load). _ensurePickEl() { diff --git a/web/src/plugins/info-callouts.mjs b/web/src/plugins/info-callouts.mjs new file mode 100644 index 0000000..36218cf --- /dev/null +++ b/web/src/plugins/info-callouts.mjs @@ -0,0 +1,120 @@ +// InfoCallouts gives the S-52 §10.6.1.1 "additional information available" markers +// (SY(INFORM01), the box-on-a-leader) a PRECISE, layering-proof tap target. +// +// The marker is a baked map symbol whose icon hit-quad is centred on the FEATURE, +// so MapLibre's fuzzy queryRenderedFeatures makes the whole symbol area tappable +// ("close enough") and symbol declutter/z-order makes some boxes un-pickable. This +// overlay instead drops a transparent, exactly-sized DOM pad on each visible box +// (a real clickable element, like the AIS-target Markers) — tapping it opens that +// feature's info, and tapping the feature itself is left to pick the feature. +// +// It is purely an INTERACTION layer: the baked INFORM01 sprite stays the visual +// box-on-leader; the pad is invisible and sits on top. It follows the mariner +// toggle for free — when "Information callouts" is off the symbol isn't rendered, +// so queryRenderedFeatures returns none and no pads are placed. + +// The INFORM01.svg "i" box, relative to the sprite pivot (the feature), in mm: the +// box centre and (square) size. Used to place + size the pad over the rendered box. +const BOX_CENTRE_MM = [12.4, -12.6]; // +x right, -y up (SVG y is down) +const BOX_SIZE_MM = 5.0; +const MIN_PAD_PX = 22; // touch-friendly floor, regardless of zoom-independent symbol size + +export class InfoCallouts { + constructor({ map, getSizeScale, atlasPpu = 0.08, onSelect } = {}) { + this._map = map; + this._getSizeScale = getSizeScale || (() => 1); + this._atlasPpu = atlasPpu || 0.08; + this._onSelect = onSelect; // (feature) => open its info + this._markers = new Map(); // key -> { marker, el } + this._timer = 0; + this._refresh = this._refresh.bind(this); + this._schedule = this._schedule.bind(this); + map.on("moveend", this._schedule); + map.on("idle", this._schedule); + this._schedule(); + } + + _schedule() { + clearTimeout(this._timer); + this._timer = setTimeout(this._refresh, 120); // debounce the pan/zoom churn + } + + // px the rendered icon occupies per sprite-mm: the atlas is 8 px/mm, scaled by the + // feature's baked icon scale (scale/atlasPpu) and the physical-size multiplier. So + // the pad tracks the ACTUAL rendered box even while the symbol size is calibrated. + _pxPerMM(scale) { + return 8 * ((scale || 0.0378) / this._atlasPpu) * this._getSizeScale(); + } + + _pointSymbolLayers() { + try { + return this._map.getStyle().layers + .filter((l) => l.type === "symbol" && /point_symbols/.test(l.id)) + .map((l) => l.id); + } catch { + return []; + } + } + + _refresh() { + const m = this._map; + const layers = this._pointSymbolLayers(); + let feats = []; + try { + feats = (layers.length ? m.queryRenderedFeatures({ layers }) : m.queryRenderedFeatures()) + .filter((f) => f.properties && f.properties.symbol_name === "INFORM01" && f.geometry && f.geometry.type === "Point"); + } catch { + return; + } + const seen = new Set(); + for (const f of feats) { + const p = f.properties; + const key = (p.cell || "") + "|" + (p.class || "") + "|" + f.geometry.coordinates.join(","); + if (seen.has(key)) continue; + seen.add(key); + const pxmm = this._pxPerMM(+p.scale); + const off = [BOX_CENTRE_MM[0] * pxmm, BOX_CENTRE_MM[1] * pxmm]; + const size = Math.max(MIN_PAD_PX, BOX_SIZE_MM * pxmm); + let rec = this._markers.get(key); + if (!rec) { + const el = document.createElement("div"); + el.className = "info-callout-pad"; + el.title = "Additional information — tap to view"; + // Invisible by default (the baked S-52 box stays the visual); a faint ring on + // hover/touch shows it's the live tap target. pointer-events auto so it owns + // the click; box-sizing so the ring doesn't grow it. + el.style.cssText = + "box-sizing:border-box;border-radius:4px;cursor:pointer;pointer-events:auto;" + + "background:transparent;border:2px solid transparent;transition:border-color .1s;"; + el.addEventListener("pointerenter", () => { el.style.borderColor = "var(--info-callout-hi,#cc3aa8)"; }); + el.addEventListener("pointerleave", () => { el.style.borderColor = "transparent"; }); + // Stop the click reaching the map so it doesn't also fire the cursor-pick. + const open = (e) => { e.stopPropagation(); e.preventDefault(); if (this._onSelect) this._onSelect(rec ? rec.feat : f); }; + el.addEventListener("click", open); + el.addEventListener("pointerdown", (e) => e.stopPropagation()); + const marker = new window.maplibregl.Marker({ element: el, anchor: "center", offset: off }).setLngLat(f.geometry.coordinates).addTo(m); + rec = { marker, el, feat: f }; + this._markers.set(key, rec); + } else { + rec.feat = f; + rec.marker.setOffset(off); + rec.marker.setLngLat(f.geometry.coordinates); + } + rec.el.style.width = rec.el.style.height = `${Math.round(size)}px`; + } + for (const [k, rec] of this._markers) { + if (!seen.has(k)) { + rec.marker.remove(); + this._markers.delete(k); + } + } + } + + destroy() { + clearTimeout(this._timer); + this._map.off("moveend", this._schedule); + this._map.off("idle", this._schedule); + for (const rec of this._markers.values()) rec.marker.remove(); + this._markers.clear(); + } +} From ac114934907443aa82ee62b5d7be1b4ea70a40e7 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 13:03:33 -0400 Subject: [PATCH 08/10] feat(web): display Calibration tab + fix the 1.333x feature-size baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whole-chart features rendered ~1.333x too big: the baker emits sizes at the 1/96" CSS reference pixel (0.26458 mm, = portrayal.DefaultPxPerSymbolUnit), but the client's physical-size multiplier assumed the 1/72" typographic point (0.35278 mm) and scaled everything by 0.35278/pxPitch. Align the client reference to 0.26458 mm so the math is correct (and it explains why changing the BAKER constant was a render no-op — the mismatch was client-side). True physical size still needs the screen's real CSS-pixel pitch, which the browser won't reveal — so add a "Calibration" settings tab (its own left-rail section, a contribution like the Advanced tab): a reference box that should be exactly 5 mm (the S-52 CHKSYM check box). Measure it with a ruler, enter the value, and it sets pxPitch = pitch × measured/5 (physical size ∝ 1/pitch), rescaling symbols/lines/text to true size and persisting via setPxPitch (localStorage + server settings). Reset returns to the CSS-reference default. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/chart-canvas/chart-canvas.mjs | 16 ++++-- web/src/chartplotter.mjs | 3 + web/src/plugins/calibration.mjs | 82 +++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 web/src/plugins/calibration.mjs diff --git a/web/src/chart-canvas/chart-canvas.mjs b/web/src/chart-canvas/chart-canvas.mjs index b1c0df5..77e0013 100644 --- a/web/src/chart-canvas/chart-canvas.mjs +++ b/web/src/chart-canvas/chart-canvas.mjs @@ -67,13 +67,17 @@ import { // the layer builder AND this element's registerPattern — and imported back here. import { buildChartLayers, PAT_PREFIX } from "./chart-style.mjs"; -const FEATURE_SCALE = 0.01 / 0.35278; +const FEATURE_SCALE = 0.01 / 0.26458; // The baker emits feature pixel sizes (icon `scale`, `width_px`, `font_size_px`, -// pattern raster) as if 1 px = 1 typographic point = 0.35278 mm (72 DPI). To render -// at TRUE physical size we multiply every size by 0.35278/pxPitch (see _scaleSizes -// in chart-style.mjs / _featureSizeScale below). On the default CSS pixel (0.2645 mm) -// that is ≈1.333×; a calibrated screen pitch makes it exact. -const BAKED_FEATURE_PITCH_MM = 0.35278; +// pattern raster) at the 1/96-inch CSS reference pixel = 0.26458 mm — the SAME +// reference as portrayal.DefaultPxPerSymbolUnit (0.01/0.26458). To render at TRUE +// physical size we multiply every size by 0.26458/pxPitch (see _scaleSizes in +// chart-style.mjs / _featureSizeScale below): on a screen whose real CSS-pixel pitch +// is 0.26458 mm that is 1×, and the Calibration panel sets pxPitch from a ruler +// measurement of the 5 mm check box for any other screen. (Was 0.35278, the 1/72" +// typographic point — a reference the baker does NOT use, which rendered the whole +// chart 0.35278/0.26458 = 1.333× too big.) +const BAKED_FEATURE_PITCH_MM = 0.26458; // Linear (constant-velocity) easing for the follow camera — see updateFollow. The // default ease-in/out would stall at each fix boundary, reading as a step. const LINEAR = (t) => t; diff --git a/web/src/chartplotter.mjs b/web/src/chartplotter.mjs index c486dd1..eb0db9f 100644 --- a/web/src/chartplotter.mjs +++ b/web/src/chartplotter.mjs @@ -22,6 +22,7 @@ import "./plugins/chart-library.mjs"; // defines (the "Charts li import "./plugins/settings-dialog.mjs"; // defines (the settings panel host) import { SettingsRegistry } from "./core/settings-registry.mjs"; // contribution registry for the settings panel import { coreSettingsContributions } from "./core/core-settings.mjs"; // the app's own display settings as contributions +import { calibrationContribution } from "./plugins/calibration.mjs"; // "Calibration" tab — ruler-measure the 5 mm box → true physical scale import { DevTools } from "./plugins/dev-tools.mjs"; // the slim contributed Advanced-tab dev tools (rebake + feature inspector) import { ConnectionsController } from "./plugins/connections.mjs"; // NMEA0183 data-source manager (Connections tab) import { VesselStateStore } from "./data/vessel-state-store.mjs"; // live NMEA0183 vessel state (own-ship/AIS/HUD feed) @@ -371,6 +372,8 @@ export class ChartPlotter extends HTMLElement { if (this._widget && c.id === "core-advanced") continue; this._settingsRegistry.register(c); } + // Display calibration (ruler-measure the 5 mm check box → true physical scale). + this._settingsRegistry.register(calibrationContribution(this)); this._settingsDlg = this.shadowRoot.getElementById("settings-dlg"); if (this._settingsDlg) this._settingsDlg.configure({ registry: this._settingsRegistry }); diff --git a/web/src/plugins/calibration.mjs b/web/src/plugins/calibration.mjs new file mode 100644 index 0000000..dc51c42 --- /dev/null +++ b/web/src/plugins/calibration.mjs @@ -0,0 +1,82 @@ +// calibration.mjs — a "Calibration" settings tab that makes the chart render at TRUE +// physical size on THIS screen. S-52 features are drawn at their real millimetre size +// (icons, line weights, text), which only works if the app knows the screen's actual +// CSS-pixel pitch. We can't read that from the browser, so the user calibrates it the +// ECDIS way: a reference box that should be exactly 5 mm wide (the S-52 CHKSYM check +// box), measured with a ruler. They enter what they actually measure and the whole +// chart rescales. +// +// Registers itself as a settings contribution with a render(host) custom slot, like +// the Advanced/dev-tools tab. Pure UI: the only app coupling is reading the current +// pitch (app._pxPitch) and calling app.setPxPitch(mm), which persists + re-renders. + +import { DEFAULT_PX_PITCH_MM, clampPxPitch } from "../lib/util.mjs"; + +const REF_MM = 5; // the S-52 size-check box (CHKSYM01) is 5 mm × 5 mm + +// A feature P mm wide renders at P/pxPitch CSS px (chart-canvas _featureSizeScale), so +// the reference box uses the same mapping — it's a faithful proxy for a 5 mm feature. +const boxPx = (pitch) => Math.max(1, Math.round(REF_MM / pitch)); + +export function calibrationContribution(app) { + return { + id: "calibration", + tab: { id: "calibration", label: "Calibration" }, + order: 4, + render: (host) => renderCalibration(host, app), + }; +} + +function renderCalibration(host, app) { + const calibrated = typeof app._pxPitch === "number" && app._pxPitch > 0; + const pitch = clampPxPitch(calibrated ? app._pxPitch : undefined); + const px = boxPx(pitch); + host.innerHTML = ` + +
+

Make the chart match real-world size. Hold a ruler to your screen and measure the box — it should be exactly ${REF_MM} mm across. Enter what you actually measure and the whole chart (symbols, line weights, text) rescales to true physical size.

+
+
+
+ +
+ + +
+

Pixel pitch: ${pitch.toFixed(4)} mm${calibrated ? "" : " (default — uncalibrated)"}

+
+
+

Tip: a wider measurement zooms the chart down, a narrower one up. Apply, then re-measure to confirm the box reads 5 mm.

+
`; + + const mmInput = host.querySelector("#cal-mm"); + host.querySelector("#cal-apply").addEventListener("click", () => { + const measured = parseFloat(mmInput.value); + if (!(measured > 0)) return; + const cur = clampPxPitch(typeof app._pxPitch === "number" && app._pxPitch > 0 ? app._pxPitch : undefined); + // Physical size ∝ 1/pitch, so to turn the measured size into REF_MM scale the + // pitch by measured/REF (clampPxPitch in setPxPitch guards absurd values). + app.setPxPitch(cur * (measured / REF_MM)); + renderCalibration(host, app); // redraw at the new calibration to verify + }); + host.querySelector("#cal-reset").addEventListener("click", () => { + app.setPxPitch(undefined); // back to the CSS-reference default + renderCalibration(host, app); + }); +} + +export { DEFAULT_PX_PITCH_MM }; From 3ca4d5f873d46d0756c608e95cfd076ebb2755f6 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 13:27:10 -0400 Subject: [PATCH 09/10] fix(web): order area fills by S-52 draw priority so overlapping land/water render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a cell with OVERLAPPING area polygons — the PresLib "ECDIS Chart 1" inset has one deep-water DEPARE under the shallow water + land — the `areas` fill layer had no fill-sort-key, so MapLibre painted in tile order and the deep-water polygon landed on top, hiding the land and shallow water under uniform deep-water blue (worst above z13, where the harbor cell takes over from the coarser overview band). Real ENCs tile their areas (no overlap) so this never bit them. Add fill-sort-key = draw_prio*1000 - drval1: paints in S-52 DrawingPriority order (DEPARE 3 < LNDARE 12, so land draws over water) with a depth tiebreaker (shallower water over deeper). No-op for non-overlapping real charts. Page 241 now shows tan land over the blue river instead of solid deep-water fill. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/chart-canvas/chart-style.mjs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/web/src/chart-canvas/chart-style.mjs b/web/src/chart-canvas/chart-style.mjs index 7a4e2cd..82a5392 100644 --- a/web/src/chart-canvas/chart-style.mjs +++ b/web/src/chart-canvas/chart-style.mjs @@ -162,7 +162,16 @@ function buildLayers(mariner, palette, atlasPpu, osm, sizeScale) { // see buildStyle.) const notLand = ["match", ["get", "color_token"], ["LANDA", "CHBRN"], false, true]; const base = [ - { id: "areas", type: "fill", source: "chart", "source-layer": "areas", ...(osm ? { filter: notLand } : {}), paint: { "fill-color": S52.areasFillColor(palette, mariner) } }, + // Paint area fills in S-52 display-priority order (DrawingPriority, baked as + // draw_prio: DEPARE=3, LNDARE=12…) so a higher-priority fill draws ON TOP — e.g. + // land over water. Real ENCs tile their areas (no overlap, so order is moot), but + // a cell with OVERLAPPING areas (the PresLib "ECDIS Chart 1" inset: one deep-water + // polygon under the shallow water + land) would otherwise paint last-in-tile on + // top, hiding land/shallow under deep water. Tiebreak by -drval1 so shallower + // water draws over deeper within the same priority. + { id: "areas", type: "fill", source: "chart", "source-layer": "areas", ...(osm ? { filter: notLand } : {}), + layout: { "fill-sort-key": ["-", ["*", ["coalesce", ["get", "draw_prio"], 0], 1000], ["coalesce", ["get", "drval1"], 0]] }, + paint: { "fill-color": S52.areasFillColor(palette, mariner) } }, { id: "area_patterns", type: "fill", source: "chart", "source-layer": "area_patterns", paint: { "fill-pattern": ["concat", PAT_PREFIX, ["coalesce", ["get", "pattern_name"], ""]] } }, // SHALLOW_PATTERN (SEABED01, client-side): DIAMOND1 over depth areas on // the shallow side of the live safety contour, shown only when the From ab5da0bf56d94e8728a175d8772ca594844aef36 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 26 Jun 2026 13:52:10 -0400 Subject: [PATCH 10/10] fix(server): never drop a cell reindex; clean up manifest on pack delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cell index used a single `built` flag: rebuild() reset it to false and build() re-claimed it. Two rapid reindexes (forget + rebuild on import) could race so that the second build() found built==true and returned WITHOUT re-scanning — the just-forgotten cells were never re-indexed and the search went stale until the next index event ("search index getting stale when I remove and reindex"). Replace the flag with a single-flight scan plus a dirty re-run: kick() starts a scan if none is running, else marks the index dirty so the running scan loops once more when it finishes. A (re)build requested mid-scan is therefore never lost. build()/rebuild() are now async wrappers over kick(); wait() (backed by a sync.Cond) lets tests and synchronous callers block until the index settles. Also harden the delete path: handleDeleteSet now removes the lingering .cells.json manifest (so the pack dir actually empties) and reconciles the index. The active search (?active=1) already drops a deleted pack via packDel + the removed manifest; the source cells stay in ENC_ROOT for a future re-bake. Adds TestActiveCellsDropOnDelete and waits in the cell-index tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/engine/server/cellindex.go | 73 +++++++++++----- internal/engine/server/cellindex_test.go | 4 + internal/engine/server/cells_active_test.go | 92 +++++++++++++++++++++ internal/engine/server/http.go | 2 +- internal/engine/server/import.go | 2 +- internal/engine/server/tilesets.go | 8 +- 6 files changed, 157 insertions(+), 24 deletions(-) create mode 100644 internal/engine/server/cells_active_test.go diff --git a/internal/engine/server/cellindex.go b/internal/engine/server/cellindex.go index 2237c75..9c3ce39 100644 --- a/internal/engine/server/cellindex.go +++ b/internal/engine/server/cellindex.go @@ -19,11 +19,13 @@ import ( // deliberately simple — a flat JSON map, not a database; the data is tiny (a few // floats per cell) and read-mostly. type cellIndex struct { - mu sync.RWMutex - bbox map[string][4]float64 // cell stem → [W,S,E,N] - path string // cells-index.json - encRoot string // /ENC_ROOT - built bool // backfill scan finished + mu sync.RWMutex + cond *sync.Cond // broadcast when a scan finishes (for wait()) + bbox map[string][4]float64 // cell stem → [W,S,E,N] + path string // cells-index.json + encRoot string // /ENC_ROOT + scanning bool // a scan goroutine is running + dirty bool // a (re)build was requested during a scan → scan again } func newCellIndex(dataDir string) *cellIndex { @@ -32,6 +34,7 @@ func newCellIndex(dataDir string) *cellIndex { path: filepath.Join(dataDir, "cells-index.json"), encRoot: filepath.Join(dataDir, "ENC_ROOT"), } + ci.cond = sync.NewCond(&ci.mu) if data, err := os.ReadFile(ci.path); err == nil { _ = json.Unmarshal(data, &ci.bbox) } @@ -74,32 +77,60 @@ func (ci *cellIndex) save() { _ = os.Rename(tmp, ci.path) } -// rebuild re-opens the backfill (e.g. after an import added new cached cells) and -// indexes any not already present. Run in a goroutine. -func (ci *cellIndex) rebuild() { +// build kicks the initial backfill; rebuild requests a fresh pass after the cache +// changed (import added cells, a set was deleted). Both funnel through kick(). +func (ci *cellIndex) build() { ci.kick() } +func (ci *cellIndex) rebuild() { ci.kick() } + +// kick ensures the index is (re)scanned. Single-flight with a dirty re-run: if a +// scan is already running it just marks the index dirty so that scan loops once +// more when it finishes — so a (re)build requested mid-scan is never lost (the old +// built-flag reset/claim could drop a concurrent reindex, leaving the index stale). +func (ci *cellIndex) kick() { ci.mu.Lock() - ci.built = false + ci.dirty = true + if ci.scanning { + ci.mu.Unlock() + return + } + ci.scanning = true ci.mu.Unlock() - ci.build() + go ci.run() } -// build backfills the index by reading every cached cell's header once. Runs in a -// background goroutine (started once) so it never blocks a request; queries see -// the index grow as it fills, and it's a no-op after the first complete pass. -func (ci *cellIndex) build() { - ci.mu.Lock() - if ci.built { +func (ci *cellIndex) run() { + for { + ci.mu.Lock() + ci.dirty = false ci.mu.Unlock() - return + ci.scan() + ci.mu.Lock() + if !ci.dirty { // nothing changed during the scan — done + ci.scanning = false + ci.cond.Broadcast() // wake any wait()ers + ci.mu.Unlock() + return + } + ci.mu.Unlock() // a (re)build arrived mid-scan — scan again + } +} + +// wait blocks until no scan is in flight — for tests and any caller that needs the +// index settled. kick() sets scanning before it returns, so a build()/rebuild() +// immediately followed by wait() always observes the in-flight scan and its re-runs. +func (ci *cellIndex) wait() { + ci.mu.Lock() + for ci.scanning { + ci.cond.Wait() } - ci.built = true // claim the build; reset only if the scan can't start ci.mu.Unlock() +} +// scan reads every cached cell's header once (bbox cached so repeat scans skip the +// already-indexed) and reconciles: drops index entries for cells no longer on disk. +func (ci *cellIndex) scan() { entries, err := os.ReadDir(ci.encRoot) if err != nil { - ci.mu.Lock() - ci.built = false - ci.mu.Unlock() return } present := make(map[string]bool, len(entries)) diff --git a/internal/engine/server/cellindex_test.go b/internal/engine/server/cellindex_test.go index 38e98c4..e082a43 100644 --- a/internal/engine/server/cellindex_test.go +++ b/internal/engine/server/cellindex_test.go @@ -41,6 +41,7 @@ func TestCellIndexBuild(t *testing.T) { ci := newCellIndex(dir) ci.build() + ci.wait() bb, ok := ci.get(cell) if !ok { t.Fatal("cell not indexed after build") @@ -72,6 +73,7 @@ func TestCellIndexFreshness(t *testing.T) { } ci := newCellIndex(dir) ci.build() + ci.wait() if _, ok := ci.get(cell); !ok { t.Fatal("not indexed") } @@ -81,6 +83,7 @@ func TestCellIndexFreshness(t *testing.T) { t.Fatal("forget did not drop the entry") } ci.rebuild() + ci.wait() if _, ok := ci.get(cell); !ok { t.Fatal("rebuild did not re-index after forget") } @@ -89,6 +92,7 @@ func TestCellIndexFreshness(t *testing.T) { t.Fatal(err) } ci.rebuild() + ci.wait() if _, ok := ci.get(cell); ok { t.Error("rebuild did not prune a removed cell") } diff --git a/internal/engine/server/cells_active_test.go b/internal/engine/server/cells_active_test.go new file mode 100644 index 0000000..2455515 --- /dev/null +++ b/internal/engine/server/cells_active_test.go @@ -0,0 +1,92 @@ +package server + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/beetlebugorg/chartplotter/internal/engine/baker" +) + +// activeCells GETs /api/cells?active=1 and returns the cell-name set. +func activeCells(t *testing.T, base string) map[string]bool { + t.Helper() + r, err := http.Get(base + "/api/cells?active=1") + if err != nil { + t.Fatal(err) + } + defer r.Body.Close() + var got struct { + Cells []string `json:"cells"` + } + if err := json.NewDecoder(r.Body).Decode(&got); err != nil { + t.Fatal(err) + } + set := map[string]bool{} + for _, c := range got.Cells { + set[c] = true + } + return set +} + +// TestActiveCellsDropOnDelete: a manifest-tracked pack's cells appear under +// ?active=1, and DELETEing the pack drops them AND removes the lingering +// .cells.json manifest. This is the "search shows uninstalled cells" / "stale +// after remove" regression: the active set is driven by the live pack list + its +// manifest, so removing the pack (packDel) and its manifest clears the cells, while +// the source stays in ENC_ROOT for a future re-bake. +func TestActiveCellsDropOnDelete(t *testing.T) { + dir := t.TempDir() + s := New(dir, dir, dir, false) + + const cell = "US5MD11M" + // The cell's source dir must exist in ENC_ROOT (serveCells lists it from there). + if err := os.MkdirAll(filepath.Join(dir, "ENC_ROOT", cell), 0o755); err != nil { + t.Fatal(err) + } + // Register a band-set with an exact cell manifest, and add it as an enabled pack. + const set = "noaa-d5-harbor" + if err := s.writeSetCells(set, map[string]baker.CellData{cell + ".000": {}}); err != nil { + t.Fatal(err) + } + manifest := filepath.Join(s.setDir(set), set+".cells.json") + if _, err := os.Stat(manifest); err != nil { + t.Fatalf("manifest not written: %v", err) + } + s.packAdd(set, filepath.Join(s.setDir(set), set+".pmtiles")) + + ts := httptest.NewServer(s) + defer ts.Close() + + if !activeCells(t, ts.URL)[cell] { + t.Fatalf("cell %s should be active while its pack is installed", cell) + } + + // DELETE the district → its band-sets are unregistered and their baked files + + // manifests removed. + req, _ := http.NewRequest(http.MethodDelete, ts.URL+"/api/set?set=noaa-d5", nil) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("delete status %d", resp.StatusCode) + } + + if activeCells(t, ts.URL)[cell] { + t.Errorf("cell %s still active after its pack was deleted (stale search)", cell) + } + if _, err := os.Stat(manifest); !os.IsNotExist(err) { + t.Errorf("manifest %s not removed on delete (err=%v)", manifest, err) + } + // The source cell is intentionally kept for a future re-bake. + if _, err := os.Stat(filepath.Join(dir, "ENC_ROOT", cell)); err != nil { + t.Errorf("source cell should be kept in ENC_ROOT: %v", err) + } +} diff --git a/internal/engine/server/http.go b/internal/engine/server/http.go index 04336d1..cb88f89 100644 --- a/internal/engine/server/http.go +++ b/internal/engine/server/http.go @@ -65,7 +65,7 @@ func New(assetsDir, cacheDir, dataDir string, allowRemote bool) *Server { dataDir = cacheDir } s := &Server{assetsDir: assetsDir, cacheDir: cacheDir, dataDir: dataDir, allowRemote: allowRemote, sets: newTileSets(), imports: newImportJobs(), auxIdx: newAuxIndex(), cellIdx: newCellIndex(dataDir)} - go s.cellIdx.build() // backfill cell bounds once, in the background (never blocks a request) + s.cellIdx.build() // backfill cell bounds in the background (kick spawns its own goroutine) // Discover every baked pack on disk (provider trees + flat tiles/), then // register the ENABLED ones (disabled packs stay on disk but off the map). State // lives in /prefs.json so it survives restarts and is shared across clients. diff --git a/internal/engine/server/import.go b/internal/engine/server/import.go index 34378d5..71e2217 100644 --- a/internal/engine/server/import.go +++ b/internal/engine/server/import.go @@ -336,7 +336,7 @@ func (s *Server) cacheCells(cells map[string]baker.CellData) { stems = append(stems, strings.TrimSuffix(name, ".000")) } s.cellIdx.forget(stems) // re-imported cells: drop stale bounds so the rebuild re-parses - go s.cellIdx.rebuild() // index the (re-)cached cells' bounds in the background + s.cellIdx.rebuild() // re-index in the background (kick spawns its own goroutine; dirty re-run picks up a reindex that lands mid-scan) } } diff --git a/internal/engine/server/tilesets.go b/internal/engine/server/tilesets.go index 41e4043..c8d49c4 100644 --- a/internal/engine/server/tilesets.go +++ b/internal/engine/server/tilesets.go @@ -171,9 +171,15 @@ func (s *Server) handleDeleteSet(w http.ResponseWriter, r *http.Request) { dir := s.setDir(name) _ = os.Remove(filepath.Join(dir, name+".pmtiles")) _ = os.Remove(filepath.Join(dir, name+".aux.zip")) - _ = os.Remove(dir) // best-effort: drop the pack dir if now empty + _ = os.Remove(filepath.Join(dir, name+".cells.json")) // the per-set cell manifest + _ = os.Remove(dir) // best-effort: drop the pack dir if now empty } s.auxIdx.invalidate() // a district's companion aux.zip is gone — re-index /api/aux + // The active search (?active=1) now drops these cells: packDel removed the pack and + // we deleted its manifest, so enabledPackCells() no longer counts them. The source + // cells stay in ENC_ROOT, so the raw index keeps their bounds for a future re-bake; + // reconcile here only prunes cells whose source is actually gone. + s.cellIdx.rebuild() w.Header().Set("Content-Type", jsonCT) io.WriteString(w, `{"ok":true}`) }