S-101 Fixes#10
Merged
Merged
Conversation
… "lower" → "emit" The S-101 close-the-gap set (two gaps + a rename). Single commit because the changes are entangled across engine.go / s101build.go and each must compile. Gap #3 — honor LinePlacement on point symbols: emitPrimitives now places a point symbol at the LinePlacement:Relative,<frac> position along a line feature (arc-length, cos-lat corrected) instead of collapsing every such symbol to the feature midpoint — route / track arrows, cable & pipeline markers. Gap #2 — date-dependent portrayal: Our engine.run() loop mirrored main.lua's success path but omitted its date handling, so seasonal features (e.g. Chesapeake Bay buoys & lights with PERSTA/PEREND, lifted for winter ice) emitted no Date:/TimeValid: and got no marker. - bridge: synthesize the S-101 fixedDateRange (DATSTA/DATEND) and periodicDateRange (PERSTA/PEREND) complex attributes from the flat S-57 simple attributes (dateStart/dateEnd alias both pairs, so the wrapping complex attribute carries the fixed-vs-periodic distinction). - engine: wire ProcessFixedAndPeriodicDates + AddDateDependentSymbol in, so a dated feature emits Date:/TimeValid: and the CHDATD01 marker symbol. - instructions.Reduce: capture the Date:/TimeValid: modifiers (previously dropped) into DrawCommand; FeatureBuild surfaces the feature-level range as the seam a future date-aware baker tag + client live-date filter uses. Rename — drop the compiler jargon "lower"/"lowering" for the S-52-era "emit": LowerS101 → emitPrimitives, S101Builder.lower → buildFeature, s101lower.go → s101emit.go (+ test); comments reworded throughout. Audit stays clean (0 errors / 0 unmapped); new tests cover both gaps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…etry Gap #1 of the S-101 close-the-gap set. The S-101 rule emits a sector/directional light's legs and arc as AugmentedRay / ArcByRadius geometry-construction instructions (screen-mm sizes); we were DROPPING them (OpOther) and instead re-deriving the figure in Go from the raw S-57 attributes (sectorLightPrims + expandSector). That Go re-derivation is a spec-divergence source — it has to reproduce the from-seaward +180 bearing reversal and the colour mapping itself, and got them subtly wrong (white lights drawn with a Go colour + an OUTLW arc backing instead of the catalogue's LITYW arc over a CHBLK backing). Now we consume the catalogue's output, which is authoritative under S-101: - instructions.Reduce parses AugmentedRay/ArcByRadius into a current figure and emits each stroking LineInstruction as OpAugmentedLine carrying the ray/arc params + the rule's simple-line style; ClearGeometry resets. - a new AugmentedFigure primitive carries one stroked element (leg or arc/ring) with the rule's colour/width/dash; emitPrimitives builds it; buildFeature tags LIGHTS legs with VALNMR for the full-light-lines variant. - the baker's tessellateFigure replaces expandSector: it tessellates each figure per-zoom from the rule's bearings/radii (no Go +180 reversal, no hardcoded OUTLW/widths), preserving the sleg short/full toggle, dedup and scamin. - retire sectorLightPrims, SectorLight/SectorParams, expandSector and s101sector.go. Verified end-to-end on US5MD1MC: the three sector lights now portray a red (LITRD) and two white→yellow (LITYW) arcs over CHBLK backings — zero OUTLW. Geometry (leg lengths, arc radii, final bearings) is unchanged; only the catalogue-correct colours change. Audit stays clean (LIGHTS 183/183/0/0/0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lter Foundation for the S-52 §10.4.1.1 / Fig 1 date-dependent display rule: a date-dependent object outside its period is not displayed for the current date (the first gate of the ECDIS display pipeline). The portrayal already surfaces the period on FeatureBuild (DATSTA/DATEND fixed, PERSTA/PEREND recurring); the baker now stamps date_start / date_end / time_valid onto every one of the feature's primitives — point_symbols, text, lines, areas and sector_lines — so the client can apply the filter live without a re-bake. Threaded via b.curDate* (set per pass, like curScamin) into the common() attr builder, plus carried on sectorPrim because sectors tessellate at tile-emit time after curDate* has moved on. Verified on US4MD81M: the seasonal Slaughter Creek / Marshy Creek buoys carry the tags on point_symbols + text. Client-side filter (current-date evaluation incl. recurring --MMDD ranges, the date-range mariner control, and re-gating CHDATD01 to the 90022 highlight toggle) is the next step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the S-52 PresLib §10.4.1.1 / Fig 1 rule — the first gate of the ECDIS
display pipeline: a date-dependent object outside its validity period is not
displayed for the viewing date. Default-on, as the spec mandates.
Baker: refine the baked period into a filter-ready form — date_recurring (present
iff dated; 1 = recurring month-day range, 0 = one-off full date) plus the
comparable bound strings date_start/date_end ("MMDD" or "YYYYMMDD"), each present
only when that bound exists (one-sided = semi-open). Replaces the raw "--MMDD" +
time_valid tags so the client filter needs no slice/format-detection.
Client: S52.dateFilter builds a MapLibre expression comparing the viewing date
(real today, or mariner.dateView for planning) to the bounds — with year-wrap for
a recurring range whose start month-day is after its end (e.g. 1101 → 0315) and
semi-open handling. Folded into combineFilters so every chart layer (point
symbols, text, sector lines, lines, areas) is gated uniformly; mariner.dateDependent
(default true) is the escape valve. Reactivity wired into setMariner.
The period logic is unit-tested (node --test, _inDatePeriod reference mirrors the
expression: summer/winter-wrap/fixed/semi-open/undated). The MapLibre expression
itself still needs in-browser confirmation.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t" toggle Fixes the over-display from the gap-#2 marker work: CHDATD01 was emitted always- on, but per S-52 §10.6.1.1 it is the optional viewing-group-90022 "Highlight date dependent" overlay (the Feature-highlights layer, alongside Highlight info / document) — toggleable and off by default. The marker bakes its symbol_name (as categoryFilter already relies on for ISODGR01), so combineFilters now drops symbol_name=="CHDATD01" unless mariner.highlightDateDependent is on (default false). Composes correctly with the date filter: out of period the whole feature (marker included) is already gone; in period the marker shows only when the highlight is enabled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…olour
Lots of lights rendered as a bare colour letter ("G", "R", "Y") instead of their
full characteristic. Root cause: LITDSN02 reads lightCharacteristic / signalGroup
/ signalPeriod from the rhythmOfLight complex attribute, but the bridge never
synthesized it — it builds sectorCharacteristics (so SECTORED lights, which read
their character from there, were fine) but all-around lights read rhythmOfLight
and got nil, dropping everything but the colour (which LITDSN02 takes as a
separate argument). The S-57 LITCHR/SIGPER were present all along.
Synthesize rhythmOfLight from S-57 LITCHR/SIGGRP/SIGPER in the bridge, mirroring
buildLightSectors. Verified on US5MD1MC: the 8 colour-only lights now render
"Fl G 4s", "Fl R 2.5s", "Fl(4+3) Y 30s", … (short-text count 8 → 0); sectored
lights unchanged; audit clean (LIGHTS 183/183/0/0/0).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The chart flickered "like a cache reloads" while zooming/panning. Root cause: on
map "idle", _refreshScaminBuckets() scheduled rebuild() — a full
map.setStyle(buildStyle(), {diff:false}) — which tears down and re-registers every
source, layer, sprite and pattern. It fired whenever the per-SCAMIN bucket
minzooms needed updating: the centre latitude drifting >2° (any mode; the only
trigger in server mode) or a new SCAMIN value loading in a fresh tile (PMTiles).
But latitude drift only SHIFTS the existing buckets' minzooms — no layers added.
So re-gate them in place with map.setLayerZoomRange per "#sm<scamin>" bucket layer
(minzoom = scaminDisplayZoom(scamin, lat), maxzoom preserved) instead of a full
restyle: same native gating, zero teardown, no flicker. The full rebuild now
runs ONLY when a genuinely-new SCAMIN value needs new bucket layers (PMTiles,
rare, converges). A value crossing the band floor self-corrects on the next such
rebuild; for the sub-2° drift this runs on the error is <0.05 zoom (already the
system's tolerance). Logic unit-tested (mock map: only #sm layers re-gated, to the
lat-adjusted minzoom, maxzoom kept).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… zoom flicker
The actual fix for the PMTiles zoom flicker (the prior commit only addressed the
latitude-drift case + a server path this deployment doesn't use). In PMTiles mode
the client DISCOVERED SCAMIN values at runtime by scanning loaded tiles
(querySourceFeatures) on every map idle; zooming into fresh tiles surfaced new
values → grew the set → full map.setStyle() rebuild → the "cache reloads" flicker.
The baker already publishes each archive's distinct SCAMIN denominators in the
PMTiles JSON metadata (SetScamin → pmtiles.go), and the Go reader already parses
it — but the web PMTiles source never read it. So:
- PMTilesArchive.init() reads the manifest from the JSON metadata (header
offsets 24/32, raw — writer sets internal-compression = none), exposing
archive.scamin.
- MultiArchive unions its packs' manifests (archive.scamin → band.scamin).
- chart-sources sources _scaminValues from the loaded bands' manifests instead
of scanning tiles. The set is known at LOAD, so zooming surfaces no new values
and triggers NO rebuild. The full rebuild now fires only when the value set
changes (a pack loads/unloads) — never during zoom. Latitude drift still uses
the in-place setLayerZoomRange re-gate.
Client-only — no re-bake needed (the manifest is already in baked archives). An
archive without it (older) simply gets no SCAMIN gating until re-baked, but never
the per-zoom rebuild. Unit-tested: MultiArchive unions/sorts/dedupes the sets and
tolerates manifest-less packs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eak) addCatalogOverlay re-runs on every style.load to restore its style-scoped sources/layers (setStyle drops them). But it also registered map-level listeners — dragstart / dragend / click — which SURVIVE setStyle, so each rebuild leaked a duplicate set. Under the SCAMIN rebuild churn (fixed in 810ee8f) this compounded: many setStyle passes → many duplicate click/drag handlers + a sprite/style-JSON re-fetch (the loadJSON on the render loop) each pass → a CPU sink. Guard the map-level listeners + cursor behind a one-shot flag so they wire once, mirroring the existing getSource("focus") guard on the sources. With 810ee8f stopping the per-idle rebuild loop and this stopping the per-rebuild listener leak, the catalog overlay is fully idempotent across restyles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…,000 Two zoom-limit fixes: 1. The 1:MIN_DETAIL_SCALE max-zoom floor was only applied at boot/restore/import, not on view change. A fly-to-chart raises the cap to reach a pack's detail and nothing put it back, so you could magnify to z18 ≈ 1:900. Re-apply the floor on every moveend (it's latitude-dependent), and cap the fly target at the destination's floor so it never overshoots then snaps back. 2. MIN_DETAIL_SCALE 2000 → 4000, and compute maxZoomForScaleFloor from the PHYSICAL scale (the px pitch the scalebar uses) instead of the OGC 0.28 mm reference — so the cap lands exactly on the 1:4,000 the HUD shows, not ~5% off. Band display-min realignment to the NOAA standard scales (harbour appearing at its coarse ~1:22k standard rather than 1:11k) is a separate, larger change (band zoom ranges + re-bake) — to follow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ake)
Every band became best-available ~0.7 zoom too late — harbour didn't draw until
~1:11k (z14) even though its coarse standard is 1:22k. Realign each band's start
zoom to its NOAA coarse standard scale (ENC Design Handbook Table 1) at ~40°N:
band coarse scale {min,max}: was → now
overview 1:10,000,000 {0,8} → {0,7}
general 1:1,500,000 {8,10} → {7,9}
coastal 1:350,000 {10,12}→ {9,11}
approach 1:90,000 {12,14}→ {11,13}
harbor 1:22,000 {14,16}→ {13,16}
berthing 1:4,000 {16,18}→ {16,18} (1:4k ≈ z16, unchanged)
Baker: Band.ZoomRange(), BakeBands(), bandBakeCeil(+2 sharpen now coastal 11→13,
approach 13→15) — the doc comment already described this scheme; the code now
matches it. BandForScale (CSCL→band) is scale-based and unchanged.
Client: CHART_BANDS, BAND_DISPLAY_MIN (chart-sources), BAND_MINZOOM/BAND_MAXZOOM
(bands.mjs) to the same ranges.
REQUIRES A RE-BAKE — the baked tile zoom ranges changed (harbour now bakes from
z13, etc.); old archives serve the old ranges until re-baked.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A refactor dropped the share-view button; the read side (parseViewHash / #v= and the /api/share snapshot loader) survived, so only the create-link UI was missing. Add it back: a share button in the bottom-right control cluster copies a #v=lon,lat,zoom[,bearing,pitch] link to the clipboard (cells/tiles already live on the server, so the opener — including a headless browser for debugging — just reopens the same camera). Restores _shareView verbatim from its original form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nd.bake
The per-band PMTiles sources hard-coded maxzoom = band.bake (a constant). If that
ever exceeds what an archive actually holds — e.g. a band-range change before a
re-bake — MapLibre requests empty tiles past the bake instead of overzooming the
deepest tile, so the whole band reads as blank. Server mode already uses the set's
real range for exactly this reason.
- sourcesDict: maxzoom = the loaded archive's real deepest baked zoom (PMTiles
header, via MultiArchive.maxZoom), falling back to band.bake until loaded.
- _updateSourceZoom: extend the existing in-place chart-all maxzoom update to
every loaded band source (no restyle), so it tracks after an archive loads.
Per-band minzoom stays 0 (sub-band SCAMIN features); only the merged "all"
source tracks the archive minzoom.
Makes the band display robust to bake/archive drift in either direction.
Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Closes three S-101 portrayal gaps, plus follow-on fixes surfaced during testing.