PR-L1 — S-98 layer-stack plumbing (no inter-product rules)#118
Merged
Conversation
Adds docs/design/s98-interoperability.md documenting the proposed plumbing for S-98 ECDIS interoperability across the loaded dataset stack: display-plane model, an initial five inter-product rules covering S-101/S-102, S-101/S-124, S-101/S-104, and S-101/S-111, a codebase impact map (S98DisplayPlane enum, IInteroperabilityAuthority, LayerStackBuilder, processor plane defaults, S-101 fill/linework split, stack-ordered pick), the catalogue-driven rule-table shape, a snapshot rebaseline plan, and twelve open TBDs to be resolved before PR-L1. Spec cited: S-98 Edition 2.0.0 (October 2025), in particular Annex A clauses 4.4 (levels), 8 (interop rules), A-3.2 (Level 1 schema), B-3.1.2 (predefined combinations), A-6.9.1 (gridded bathymetry rule), and Main 9.2.1 (priority layers). No production code touched; only a new design doc and a one-line docs/toc.yml entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduces typed S-98 display planes and a central
InteroperabilityAuthority that sorts every loaded dataset's layers
into deterministic S-98 paint order. No IC parsing, no inter-product
suppression, no UI — those land in PR-L2/L3. The five v1 rules from
docs/design/s98-interoperability.md §3 are expressed purely as
default plane assignments.
Core
----
* S98DisplayPlane enum (9 canonical planes from S-98 Annex A
§A-6.9.1 / MSC.530(106)/Rev.1 §Appendix 2) with spec citations.
Pipelines
---------
* LayerStackEntry / IInteroperabilityAuthority / InteroperabilityAuthority
default impl with hardcoded plane table covering S-101 (split into
area-fill vs line-work), S-102, S-104, S-111 (color-band / arrows /
stations), S-122, S-124, S-125, S-127, S-128, S-129, S-131, S-201,
S-411, S-421. Stable sort by (Plane, WithinPlanePriority, input order).
* LayerStackBuilder collects per-dataset entries top-of-UI first and
delegates to the authority.
* DatasetResult.StackEntries: parallel to Layers; populated by every
processor.
* S101DatasetProcessor: TBD-3 split — runs the pipeline once then
partitions the drawing instructions into AreaInstruction (→
BaseChartUnder) vs everything else (→ BaseChartOver). Emits two
Mapsui layers + two StackEntries. LayerNames "s101.areas" /
"s101.linework" for sub-layer toggles.
* S102/S104/S111/S122/S124/S125/S127/S128/S129/S131/S201/S411/S421/S57
each emit StackEntries on their per-spec default plane.
Viewer
------
* DatasetLoaderService tracks _entryStackEntries parallel to
_entryLayers; FlattenLayerOrder now feeds LayerStackBuilder. First-
load no longer relies on "new layers go on top" — the S-98 sort
always wins so a late-loaded S-102 still slots between S-101 area
fills and line work.
* IDatasetLoaderService exposes CurrentStackedLayers + LayerStackChanged
(default-implemented so existing test doubles needn't change).
* PickService.ResolveHits re-orders mapInfo.MapInfoRecords by
descending stack index before the dedup walk so multi-hit picks
return top-of-stack first. Cites S-98 Main §10.12 / Annex A §A-6.12
("interrogation under development").
Tests
-----
* InteroperabilityAuthorityTests (28 tests): plane assignment per
product, sort stability, within-plane priority, tiebreaker, and
cross-dataset interleave independent of load order.
* PickService.HandlePick_OrdersHitsByLayerStackTopFirst.
* All 322 Pipelines + 279 Viewer + 26 VisualRegression tests pass.
No snapshots needed rebaselining — each per-spec single-layer
dataset still paints at the same plane, and the S-101 split paints
the same instructions in the same order across two layers Mapsui
walks adjacently.
Conventions
-----------
* .NET 10, nullable enabled, ArgumentNullException.ThrowIfNull.
* XML doc citations to S-98 / MSC.530(106)/Rev.1 / S-100 Part 9 on
every public API and every plane-assignment in the default table.
* No new bundled S98 spec content.
Out of scope (deferred to PR-L2 / L3 per design note §10)
---------------------------------------------------------
* IC parsing / rule tables / suppression / replacement / hybridization.
* Per-feature plane filters (Annex A §4.3).
* Layer Controls UI (PR-L3).
* Safety-contour MSC.232 exception.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Performance Gate✅ PASSED — no regressions. Threshold: 10.0%, MAD multiplier (k): 3.0, retry-zone mult: 2.0× Scenario summary
exchange-set-openIteration statistics
Spans (sum of all iterations)
Metrics
s101-portray-coldIteration statistics
Spans (sum of all iterations)
Metrics
s101-portray-warmIteration statistics
Spans (sum of all iterations)
Metrics
s101-render-warmIteration statistics
Spans (sum of all iterations)
Metrics
s102-coverageIteration statistics
Spans (sum of all iterations)
Metrics
s102-coverage-openIteration statistics
Spans (sum of all iterations)
Metrics
s102-coverage-render-largeIteration statistics
Spans (sum of all iterations)
Metrics
s124-vectorIteration statistics
Spans (sum of all iterations)
Metrics
s201-vectorIteration statistics
Spans (sum of all iterations)
Metrics
Generated by EncDotNet.S100.PerfReport gate command |
The DatasetLoaderService was hardcoded to InteroperabilityAuthority.Default. Promotes the authority to an optional constructor parameter so hosts (or tests) can plug in alternative stacking policies via the existing IInteroperabilityAuthority seam. Adds LoadOrderInteroperabilityAuthority as a working alternative — a strict dataset-by-dataset policy that paints the bottom-of-UI dataset's layers first, then the next, etc., ignoring the S-98 plane entirely (classic GIS 'load order is paint order' semantics). It still delegates GetDefaultPlane to the S-98 oracle so layer-controls UIs can still label layers with their conceptual plane. Tests ----- * LoadOrderAuthority_ignores_plane_and_uses_strict_dataset_ordering: same fixture as the S-98 interleave test produces a different ordering through the alternative authority. * LoadOrderAuthority_still_exposes_S98_default_planes_for_labels. Conventions ----------- * Default of the optional ctor parameter is the S-98 authority — no behaviour change for existing callers. * XML doc on the interface now advertises pluggability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Consumers that participate in cross-dataset paint ordering now depend on an authority *provider* rather than holding an IInteroperabilityAuthority reference directly. The provider exposes the currently active authority on each consult and raises CurrentChanged when the host swaps it, so DatasetLoaderService re-flattens the layer stack in response (e.g. when a viewer setting toggles between S-98 and strict load-order policy). - New IInteroperabilityAuthorityProvider interface with Current + CurrentChanged event; default mutable impl InteroperabilityAuthorityProvider whose Set(authority) raises the event (even when the new reference equals the previous one, so a host can force a re-sort after a catalogue tweak). - DatasetLoaderService ctor now takes an IInteroperabilityAuthorityProvider? (defaulting to a fresh provider holding the S-98 oracle) and subscribes to CurrentChanged to call ReorderDatasetLayers(FlattenLayerOrder()). - FlattenLayerOrder + the fallback plane-synthesis path consult _authorityProvider.Current on each call. - GmlDatasetProcessorBase still stamps the canonical S-98 plane via InteroperabilityAuthority.Default at render time: that field records 'which conceptual plane does this layer's content belong to?' — a per-product fact — while the *sort policy* lives in the authority that DatasetLoaderService consults. Documented inline. - 5 new InteroperabilityAuthorityProviderTests cover Current default, ctor initial-value, Set updates + raises event, force-resort on reference-equal Set, null-guard. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes every static 'Default' singleton on the interoperability types and forces consumers to receive the authority provider through their constructor (resolved from the DI container). Production: - Delete InteroperabilityAuthority.Default and LoadOrderInteroperabilityAuthority.Default + its parameterless ctor. Authorities are stateless types whose instances are produced by DI. - InteroperabilityAuthorityProvider ctor now requires the initial authority; no implicit fallback. - GmlDatasetProcessorBase requires IInteroperabilityAuthorityProvider in its ctor and consults provider.Current at render time when stamping each LayerStackEntry. All ten GML processor subclasses (S-122/S-124/S-125/S-127/S-128/S-129/S-201/S-411/S-421) take the provider as a required ctor parameter and forward it to the base. - DatasetPipelineFactory takes the provider and forwards it to every GML processor it constructs. - DatasetLoaderService ctor: provider is now required (was optional). - App.axaml.cs registers IInteroperabilityAuthorityProvider as a singleton wrapping a fresh InteroperabilityAuthority, and injects it into DatasetPipelineFactory; DatasetLoaderService gets it via auto-resolved DI. Tests: - InteroperabilityAuthorityTests / InteroperabilityAuthorityProviderTests construct authorities/providers explicitly with . - New TestAuthority helper in the viewer tests yields a fresh S-98 provider for processor-instantiation tests. - DatasetPipelineFactoryFeatureCatalogueReuseTests and the visual- regression RenderHarness pass a provider explicitly. - PerfRunner reflection-based factory lookup now tries the PR-L1 ctor (with provider) first and falls back to the base-SHA shapes for cross-SHA perf overlays. Build + 329 pipeline + 279 viewer tests green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The perf-gate workflow overlays the PR's PerfRunner sources onto the base SHA checkout and builds against the base-SHA library binaries (see .github/workflows/perf.yml lines 31-45). Hard-typed references to EncDotNet.S100.Datasets.Pipelines.Interoperability.* fail to compile against the base SHA, which does not yet expose that namespace. Switch SharedInfrastructure.CreatePipelineFactory to probe the provider interface and concrete types via Assembly.GetType and construct instances via Activator.CreateInstance, so the tooling compiles against both base and PR library surfaces. The legacy 4-param and resolver-based ctors remain as fallbacks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
1 participant
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
Implements PR-L1 of the S-98 interoperability rollout (per
docs/design/s98-interoperability.md, PR-L0 #117): deterministic S-98-shaped layer stacking across all loaded datasets. No inter-product suppression rules. No IC-driven plane reassignment. No UI for layer controls.What's in this PR
S98DisplayPlaneenum with the 9 canonical planes from S-98 Annex A §A-6.9.1 / MSC.530(106)/Rev.1 §Appendix 2.DatasetResult.StackEntriesproperty withLayerStackEntryrecords describing which S-98 plane each emitted layer belongs in.InteroperabilityAuthority.Defaultcarries a hardcoded plane table (TBD-8 resolved) and a stable sort:(Plane, WithinPlanePriority, input order).LayerStackBuilder— collects entries from every loaded dataset top-of-UI-first and delegates to the authority.DatasetLoaderService.FlattenLayerOrdernow defers entirely to it.PickService.ResolveHitssorts hits by descending stack index before dedup, so multi-hit picks return top-of-stack first. S-98 Main §10.12 / Annex A §A-6.12 designate interrogation as "under development" — this is a viewer-side decision.Locked decisions (per spec brief)
InteroperabilityAuthoritytable (OnDemandSurface(20) <BaseChartOver(30))S101DatasetProcessor.RenderpartitionsAreaInstructionfrom the restOnDemandSurfaceInteroperabilityAuthority.TODOcomment notes PR-L2 may move itInteroperabilityAuthority. No external rule packTest counts
EncDotNet.S100.Pipelines.TestsEncDotNet.S100.Viewer.TestsEncDotNet.S100.VisualRegression.TestsFull
dotnet build EncDotNet.S100.slnx+ targeteddotnet test --configuration Releasegreen.Visual regression rebaselining
None required. All 26 snapshots still match. Two reasons:
Performance — S-101 split cost
The split runs the pipeline once and partitions
prepared(IReadOnlyList<DrawingInstruction>) into areas vs non-areas; only the Mapsui rendering step runs twice. For a typical single ENC cell this is a small constant overhead (no second pipeline pass). I haven't measured on a large multi-cell load — if profiling shows >100 ms regression, a v2 mitigation is a single-pass dual-sink renderer. Filed as a follow-up.Explicitly NOT in this PR
Acceptance gates
Build_interleaves_across_planes_independent_of_load_order.)HandlePick_OrdersHitsByLayerStackTopFirst.)Notes for reviewers
IDatasetLoaderService.CurrentStackedLayers+LayerStackChangedare default-implemented on the interface (return empty / no-op event). This lets existing test doubles compile without changes. PR-L3 may promote them to required members once the Layer Controls UI consumes them.DatasetResult.StackEntriesis nullable but always populated by the in-tree processors. The fallback path inDatasetLoaderService.FlattenLayerOrdersynthesises defaults viaInteroperabilityAuthority.Default.GetDefaultPlane(spec)if a processor ever forgets — defensive.No inter-product rule logic snuck in:
InteroperabilityAuthorityonly consults the product name (and an optional layer-kind hint) when assigning a default plane. It never inspects another loaded dataset.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com