Skip to content

Fix S-101 multipoint sounding glyphs collapsing onto one anchor#292

Merged
philliphoff merged 5 commits into
mainfrom
philliphoff/fix-s101-sounding-artifacts
Jun 16, 2026
Merged

Fix S-101 multipoint sounding glyphs collapsing onto one anchor#292
philliphoff merged 5 commits into
mainfrom
philliphoff/fix-s101-sounding-artifacts

Conversation

@philliphoff

Copy link
Copy Markdown
Owner

Summary

Multi-digit depth soundings in S-101 ENCs were rendering as overlapping black blobs instead of clean numeric labels. The root cause is in the shared drawing-instruction parser: S-101 multipoint sounding features emit one AugmentedPoint:GeographicCRS,lon,lat per sounding, followed by several PointInstructions, one per digit glyph (e.g. SOUNDG21;SOUNDG13;SOUNDG08 for a three-digit depth). The parser reset the augmented anchor to null after the first PointInstruction, so every glyph but the first fell back to the feature's primary geometry point and piled up on the same spot.

The fix treats AugmentedPoint as a spatial-geometry context (S-100 Part 9 §11.5) that persists across all following point/text instructions until the next AugmentedPoint or ClearGeometry, rather than as per-instruction state. Per-instruction style state (rotation, scale, offset, text alignment) is still reset as before.

This PR also includes the earlier companion fix for the translucent stacked squares around soundings (dedupe of hit-test rectangles to one per feature/anchor) and removes temporary pivot debug logging used during investigation. Confirmed visually in the viewer: the affected soundings now render as a single value.

Spec alignment

  • S-101 ENC (s101-enc)
  • S-100 framework (s100-framework)
  • S-102 bathymetry (s102-bathymetry)
  • S-104 water level (s104-water-level)
  • S-111 surface currents (s111-surface-currents)
  • S-124 navigational warnings (s124-nav-warnings)
  • S-129 under keel clearance (s129-ukc)
  • N/A - change is purely infrastructural (build, CI, docs, tooling)

Spec section references cited in code/docs: S-100 Part 9 §11.5 (AugmentedPoint spatial-geometry context persistence), cited in the parser comments.

Note: the parser lives in EncDotNet.S100.Core and is shared by all vector products, but the bug only manifests for products that emit multiple point instructions under a single AugmentedPoint (S-101 multipoint soundings). Behaviour for single-point features is unchanged, since the anchor stays null when no AugmentedPoint is emitted.

Tests

  • Added/updated xunit tests under tests/
  • Tests requiring real data files use SkippableFact
  • dotnet test --configuration Release passes locally

The parser regression test was updated to assert that the anchor persists across all glyphs of a multipoint sounding and switches on the next AugmentedPoint.

Documentation

  • Updated the affected project's src/<project>/README.md
  • Updated conceptual docs under docs/ if user-facing behaviour changed
  • New public APIs have XML doc comments

N/A - no public API surface changed; behaviour-only fix with in-code comments citing the spec.

Dependencies

  • No new NuGet dependencies, OR versions added to Directory.Packages.props (not in the .csproj)
  • gh-advisory-database security check run for any new dependency

Breaking changes

None.

Multipoint sounding features emit one AugmentedPoint per sounding
followed by several PointInstructions (one per digit glyph). The
drawing-instruction parser reset the augmented anchor to null after
the first PointInstruction, so every glyph but the first fell back to
the feature's primary geometry point and piled up into an overlapping
blob.

Per S-100 Part 9 §11.5 an AugmentedPoint establishes a spatial-geometry
context that persists across all following point/text instructions
until the next AugmentedPoint or ClearGeometry. Stop resetting the
anchor per instruction.

Also adds the multipoint hit-rect dedupe (Fix #1, translucent stacked
squares), updates the parser regression test to assert anchor
persistence across glyphs, and removes temporary pivot debug logging.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Performance Gate

PASSED — no regressions.

Threshold: 10.0%, MAD multiplier (k): 3.0, retry-zone mult: 2.0×

Scenario summary

Scenario Status Δ median (%) z (Δ/MAD) Base median (ms) Samples (b/c)
exchange-set-open ✅ pass -1.1 -1.27 1.30 5/5
s101-portray-cold ✅ pass +2.5 +0.90 0.26 5/5
s101-portray-warm ✅ pass +17.3 +1.64 0.21 5/5
s101-real-cold ✅ pass +12.6 +5.45 0.27 5/5
s101-real-warm ✅ pass +5.3 +1.97 0.29 5/5
s101-render-warm ✅ pass +18.7 +5.33 0.21 5/5
s102-coverage ✅ pass +10.2 +1.26 0.17 5/5
s102-coverage-open ✅ pass +2.0 +1.27 2.81 20/20
s102-coverage-render-large ✅ pass +7.6 +1.61 0.19 5/5
s102-real-warm ✅ pass +1.3 +0.37 0.31 5/5
s111-real-warm ✅ pass +2.6 +0.73 0.29 5/5
s124-vector ✅ pass -15.1 -3.15 0.25 5/5
s201-vector ✅ pass +33.0 +22.12 0.17 5/5

exchange-set-open

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 1.30 1.28
Baseline MAD (ms) 0.01
Δ median -1.1%
z (Δ/MAD) -1.27

Spans (sum of all iterations)

Span Baseline (ms) Candidate (ms) Delta Status
s100.asset.read 2.50 2.47 -1.1% ▫️
s100.exchangeset.parse 44.94 44.30 -1.4% ▫️

Metrics

Metric Baseline Candidate Delta Status
s100.asset.read.duration 0.19 0.23 +26.3%

s101-portray-cold

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.26 0.27
Baseline MAD (ms) 0.01
Δ median +2.5%
z (Δ/MAD) +0.90

s101-portray-warm

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.21 0.24
Baseline MAD (ms) 0.02
Δ median +17.3%
z (Δ/MAD) +1.64

s101-real-cold

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.27 0.30
Baseline MAD (ms) 0.01
Δ median +12.6%
z (Δ/MAD) +5.45

s101-real-warm

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.29 0.30
Baseline MAD (ms) 0.01
Δ median +5.3%
z (Δ/MAD) +1.97

s101-render-warm

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.21 0.25
Baseline MAD (ms) 0.01
Δ median +18.7%
z (Δ/MAD) +5.33

s102-coverage

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.17 0.18
Baseline MAD (ms) 0.01
Δ median +10.2%
z (Δ/MAD) +1.26

s102-coverage-open

Iteration statistics

Stat Baseline Candidate
Samples 20 20
Median (ms) 2.81 2.87
Baseline MAD (ms) 0.05
Δ median +2.0%
z (Δ/MAD) +1.27

Spans (sum of all iterations)

Span Baseline (ms) Candidate (ms) Delta Status
s100.dataset.open 452.51 453.48 +0.2% ▫️
s100.hdf5.dataset.read 164.21 165.24 +0.6% ▫️
s100.hdf5.file.open 18.38 19.10 +3.9% ▫️
s100.hdf5.open 18.58 18.68 +0.5% ▫️

Metrics

Metric Baseline Candidate Delta Status
s100.hdf5.read.bytes 36456.00 36456.00 +0.0% ▫️
s100.hdf5.read.duration 27.58 26.77 -2.9% ▫️
s100.hdf5.read.duration 32.36 31.75 -1.9% ▫️
s100.hdf5.read.duration 11.11 11.05 -0.6% ▫️

s102-coverage-render-large

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.19 0.21
Baseline MAD (ms) 0.01
Δ median +7.6%
z (Δ/MAD) +1.61

s102-real-warm

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.31 0.32
Baseline MAD (ms) 0.01
Δ median +1.3%
z (Δ/MAD) +0.37

s111-real-warm

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.29 0.30
Baseline MAD (ms) 0.01
Δ median +2.6%
z (Δ/MAD) +0.73

s124-vector

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.25 0.21
Baseline MAD (ms) 0.01
Δ median -15.1%
z (Δ/MAD) -3.15

s201-vector

Iteration statistics

Stat Baseline Candidate
Samples 5 5
Median (ms) 0.17 0.23
Baseline MAD (ms) 0.00
Δ median +33.0%
z (Δ/MAD) +22.12

Generated by EncDotNet.S100.PerfReport gate command

philliphoff and others added 3 commits June 15, 2026 23:51
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route only linux-arm64 to the dedicated SIMD S57 baseline while win-arm64 continues to use the faithful default baseline.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The non-macOS S57RenderingTests.EncCell_DayPalette render is bimodal: the
same OS/arch emits either the faithful or the SIMD raster-path variant
depending on the runner CPU. This PR's denser sounding labels widened the
variant gap to 8.49%, so the per-platform linux-arm64 baseline could no
longer reliably pass (a single linux-arm64 runner produces either variant).

Revert to a single faithful baseline and raise the non-macOS tolerance to
10% so it absorbs both variants; osx-arm64 stays strict at 5%. Remove the
now-redundant linux-arm64 baseline and platform selector. Documented as a
stopgap; #294 tracks the deterministic fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@philliphoff philliphoff merged commit 6e228a5 into main Jun 16, 2026
11 checks passed
@philliphoff philliphoff deleted the philliphoff/fix-s101-sounding-artifacts branch June 16, 2026 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant