From 9128fde5cf6c6e01e10ef629a61b306ff58da282 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 23:45:28 +0000 Subject: [PATCH 1/2] Give feature pointers a long-name type and add lookups by long name Breaking: S57FeaturePointer.Name is now S57LongName (was S57RecordName), and S57Chart.ReferencingFeatures is keyed by S57LongName. An FFPT pointer names its target by long name (AGEN, FIDN, FIDS) only, but it was an S57RecordName with RCNM and RCID left zero, so a pointer's name never matched a feature's record name and the library offered no way to follow it. FIDN, an unsigned 32-bit value, was also cast to int. - Add S57LongName, a readonly record struct of AGEN, FIDN and FIDS. - Add LongName to S57RecordName, S57FeatureRecord and S57TypedFeature; it is null for a record without a FOID field. - Add S57Chart.FeaturesByLongName, GetFeature(S57LongName) and GetReferencingFeatures(S57LongName), and S57Document.GetFeatureRecord(S57LongName). - The long-name index is a read-only Dictionary rather than an ImmutableDictionary, which kept the chart's retained memory within its budget. Building a chart allocates 3.2% more for it, so the S57ChartFromDocument and S57ChartRetained budgets rise to the measured values. - The break is recorded in CompatibilitySuppressions.xml for 0.8.0. The reader's output is otherwise unchanged: the fingerprint helper formats a pointer's long name as before, and the synthetic and local real-cell fingerprints match. In nine NOAA cells, all 662 feature pointers resolve with GetFeature. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YagvELyJKBANk61DyFGUNJ --- docs/reading-s57-charts.md | 22 +++++++ src/EncDotNet.S57/Charts/S57Chart.cs | 65 +++++++++++++++++-- src/EncDotNet.S57/Charts/S57TypedFeature.cs | 6 ++ .../CompatibilitySuppressions.xml | 32 +++++++++ src/EncDotNet.S57/S57Document.cs | 23 +++++++ src/EncDotNet.S57/S57DocumentReader.cs | 6 +- src/EncDotNet.S57/S57FeaturePointer.cs | 8 ++- src/EncDotNet.S57/S57FeatureRecord.cs | 6 ++ src/EncDotNet.S57/S57LongName.cs | 42 ++++++++++++ src/EncDotNet.S57/S57RecordName.cs | 9 +++ .../PerformanceBudgets.cs | 4 +- tests/EncDotNet.UnitTests/S57ChartTests.cs | 30 ++++++++- .../S57DocumentApplyChangesTests.cs | 18 ++--- .../S57DocumentFingerprint.cs | 6 ++ .../S57DocumentReaderTests.cs | 62 +++++++++++++++++- 15 files changed, 315 insertions(+), 24 deletions(-) create mode 100644 src/EncDotNet.S57/CompatibilitySuppressions.xml create mode 100644 src/EncDotNet.S57/S57LongName.cs diff --git a/docs/reading-s57-charts.md b/docs/reading-s57-charts.md index 6907d19..c76b785 100644 --- a/docs/reading-s57-charts.md +++ b/docs/reading-s57-charts.md @@ -126,6 +126,28 @@ for cells without full topology, their exterior and interior edges [Architecture](architecture.md) page explains how features, edges, nodes and faces relate. +### Follow relationships between features + +Features refer to other features, such as a beacon to the lights on it, with +feature pointers (the FFPT field). A pointer names its target by long name +(`S57LongName`: agency, feature ID and subdivision), the identifier a feature +keeps across editions and updates. `GetFeature` finds the target, and +`GetReferencingFeatures` finds the features that point to a given one: + +```csharp +foreach (S57TypedFeature feature in chart.AllFeatures.Values) +{ + foreach (S57FeaturePointer pointer in feature.RelatedFeatures) + { + S57TypedFeature? related = chart.GetFeature(pointer.Name); + Console.WriteLine($"{feature.ObjectCode} -> {related?.ObjectCode} ({pointer.Relationship})"); + } +} +``` + +`S57Document.GetFeatureRecord(S57LongName)` does the same on a document +without building a chart, but searches every record on each call. + ## Apply update cells A base cell (`.000`) is followed by update cells (`.001`, `.002`, and so on) diff --git a/src/EncDotNet.S57/Charts/S57Chart.cs b/src/EncDotNet.S57/Charts/S57Chart.cs index 4fe57be..88e2a41 100644 --- a/src/EncDotNet.S57/Charts/S57Chart.cs +++ b/src/EncDotNet.S57/Charts/S57Chart.cs @@ -1,4 +1,5 @@ using System.Collections.Immutable; +using System.Collections.ObjectModel; namespace EncDotNet.S57.Charts; @@ -56,10 +57,16 @@ public sealed record S57Chart public IReadOnlyDictionary AllFeatures { get; } /// - /// Gets the reverse feature-pointer index: maps a feature's record name to all features + /// Gets the features that have a long name (LNAM), indexed by it. Feature pointers + /// () refer to features by long name. + /// + public IReadOnlyDictionary FeaturesByLongName { get; } + + /// + /// Gets the reverse feature-pointer index: maps a feature's long name to all features /// that reference it via FFPT (feature-to-feature pointers). /// - public IReadOnlyDictionary> ReferencingFeatures { get; } + public IReadOnlyDictionary> ReferencingFeatures { get; } /// /// Gets the spatial co-location index: maps a spatial record name to all point features @@ -125,6 +132,7 @@ public S57Chart( AreaFeatures = areaFeatures ?? []; MetaFeatures = metaFeatures ?? []; AllFeatures = BuildAllFeaturesIndex(PointFeatures, LineFeatures, AreaFeatures, MetaFeatures); + FeaturesByLongName = BuildFeaturesByLongNameIndex(AllFeatures); ReferencingFeatures = BuildReferencingFeaturesIndex(AllFeatures); ColocatedPointFeatures = BuildColocatedPointFeaturesIndex(PointFeatures); FeaturesByObjectCode = BuildFeaturesByObjectCodeIndex(PointFeatures, LineFeatures, AreaFeatures, MetaFeatures); @@ -162,10 +170,28 @@ private static ImmutableDictionary BuildAllFeatu return builder.ToImmutable(); } - private static ImmutableDictionary> BuildReferencingFeaturesIndex( + private static ReadOnlyDictionary BuildFeaturesByLongNameIndex( IReadOnlyDictionary allFeatures) { - var builder = new Dictionary.Builder>(); + // A hash table, not an ImmutableDictionary: it is read far more than built, and a node per + // entry would add about a twentieth to the chart's memory. Nothing mutates it once built. + var index = new Dictionary(allFeatures.Count); + foreach (var feature in allFeatures.Values) + { + if (feature.LongName is { } longName) + { + index[longName] = feature; + } + } + + index.TrimExcess(); + return index.AsReadOnly(); + } + + private static ImmutableDictionary> BuildReferencingFeaturesIndex( + IReadOnlyDictionary allFeatures) + { + var builder = new Dictionary.Builder>(); foreach (var feature in allFeatures.Values) { @@ -182,7 +208,7 @@ private static ImmutableDictionary } } - var result = ImmutableDictionary.CreateBuilder>(); + var result = ImmutableDictionary.CreateBuilder>(); foreach (var (name, list) in builder) { result[name] = list.ToImmutable(); @@ -358,6 +384,7 @@ private S57Chart(S57Document document) AreaFeatures = areaFeatures.ToImmutable(); MetaFeatures = metaFeatures.ToImmutable(); AllFeatures = allFeatures.ToImmutable(); + FeaturesByLongName = BuildFeaturesByLongNameIndex(AllFeatures); ReferencingFeatures = BuildReferencingFeaturesIndex(AllFeatures); ColocatedPointFeatures = BuildColocatedPointFeaturesIndex(PointFeatures); FeaturesByObjectCode = BuildFeaturesByObjectCodeIndex(PointFeatures, LineFeatures, AreaFeatures, MetaFeatures); @@ -461,6 +488,34 @@ public static async Task FromStreamAsync(Stream stream, CancellationTo return Faces.TryGetValue(name, out var face) ? face : null; } + /// + /// Gets the feature with the specified long name, such as the target of a feature pointer. + /// + /// The long name (LNAM) to find. + /// The feature, or null if the chart has none with that long name. + /// + /// + /// foreach (var pointer in feature.RelatedFeatures) + /// { + /// S57TypedFeature? related = chart.GetFeature(pointer.Name); + /// } + /// + /// + public S57TypedFeature? GetFeature(S57LongName longName) + { + return FeaturesByLongName.TryGetValue(longName, out var feature) ? feature : null; + } + + /// + /// Gets the features whose feature pointers refer to the specified feature. + /// + /// The long name (LNAM) of the referenced feature. + /// The referencing features, or an empty list if there are none. + public IReadOnlyList GetReferencingFeatures(S57LongName longName) + { + return ReferencingFeatures.TryGetValue(longName, out var features) ? features : []; + } + /// /// Gets all point features that reference the specified spatial record (co-located features). /// diff --git a/src/EncDotNet.S57/Charts/S57TypedFeature.cs b/src/EncDotNet.S57/Charts/S57TypedFeature.cs index 14c6040..a7841ac 100644 --- a/src/EncDotNet.S57/Charts/S57TypedFeature.cs +++ b/src/EncDotNet.S57/Charts/S57TypedFeature.cs @@ -8,6 +8,12 @@ public abstract record S57TypedFeature /// Gets the record name that uniquely identifies this feature. public S57RecordName RecordName { get; } + /// + /// Gets the feature's long name (LNAM), which other features' + /// pointers use to refer to it, or null when the feature has none. + /// + public S57LongName? LongName => RecordName.LongName; + /// Gets the object code (OBJL) identifying the feature type. public S57ObjectCode ObjectCode { get; } diff --git a/src/EncDotNet.S57/CompatibilitySuppressions.xml b/src/EncDotNet.S57/CompatibilitySuppressions.xml new file mode 100644 index 0000000..22da9dd --- /dev/null +++ b/src/EncDotNet.S57/CompatibilitySuppressions.xml @@ -0,0 +1,32 @@ + + + + + CP0002 + M:EncDotNet.S57.Charts.S57Chart.get_ReferencingFeatures + lib/net10.0/EncDotNet.S57.dll + lib/net10.0/EncDotNet.S57.dll + true + + + CP0002 + M:EncDotNet.S57.S57FeaturePointer.get_Name + lib/net10.0/EncDotNet.S57.dll + lib/net10.0/EncDotNet.S57.dll + true + + + CP0002 + M:EncDotNet.S57.Charts.S57Chart.get_ReferencingFeatures + lib/net8.0/EncDotNet.S57.dll + lib/net8.0/EncDotNet.S57.dll + true + + + CP0002 + M:EncDotNet.S57.S57FeaturePointer.get_Name + lib/net8.0/EncDotNet.S57.dll + lib/net8.0/EncDotNet.S57.dll + true + + \ No newline at end of file diff --git a/src/EncDotNet.S57/S57Document.cs b/src/EncDotNet.S57/S57Document.cs index 731a6b8..e58eaaf 100644 --- a/src/EncDotNet.S57/S57Document.cs +++ b/src/EncDotNet.S57/S57Document.cs @@ -19,6 +19,29 @@ public sealed record S57Document /// Gets all vector (spatial) records. public IReadOnlyList VectorRecords { get; init; } = []; + /// + /// Gets a feature record by its long name (LNAM), such as the target of a feature pointer. + /// + /// The long name to find. + /// The first feature record with that long name, or null if there is none. + /// + /// This searches every feature record. To resolve many pointers, build an + /// and use , + /// which uses an index. + /// + public S57FeatureRecord? GetFeatureRecord(S57LongName longName) + { + foreach (var record in FeatureRecords) + { + if (record.LongName == longName) + { + return record; + } + } + + return null; + } + /// /// Gets a feature record by its record name. /// diff --git a/src/EncDotNet.S57/S57DocumentReader.cs b/src/EncDotNet.S57/S57DocumentReader.cs index 3cf41fe..8988b5f 100644 --- a/src/EncDotNet.S57/S57DocumentReader.cs +++ b/src/EncDotNet.S57/S57DocumentReader.cs @@ -875,9 +875,9 @@ private static S57RecordName DecomposeNameField(ReadOnlySpan data) } /// - /// Decomposes a LNAM composite subfield (8 bytes: AGEN(2) + FIDN(4) + FIDS(2)) into an S57RecordName. + /// Decomposes a LNAM composite subfield (8 bytes: AGEN(2) + FIDN(4) + FIDS(2)) into an S57LongName. /// - private static S57RecordName DecomposeLongNameField(ReadOnlySpan data) + private static S57LongName DecomposeLongNameField(ReadOnlySpan data) { if (data.Length < 8) { @@ -887,7 +887,7 @@ private static S57RecordName DecomposeLongNameField(ReadOnlySpan data) var agen = BinaryPrimitives.ReadUInt16LittleEndian(data.Slice(0, 2)); var fidn = BinaryPrimitives.ReadUInt32LittleEndian(data.Slice(2, 4)); var fids = BinaryPrimitives.ReadUInt16LittleEndian(data.Slice(6, 2)); - return S57RecordName.FromLongName(agen, (int)fidn, fids); + return new S57LongName(agen, fidn, fids); } /// diff --git a/src/EncDotNet.S57/S57FeaturePointer.cs b/src/EncDotNet.S57/S57FeaturePointer.cs index 01fe90f..fbf306e 100644 --- a/src/EncDotNet.S57/S57FeaturePointer.cs +++ b/src/EncDotNet.S57/S57FeaturePointer.cs @@ -5,8 +5,12 @@ namespace EncDotNet.S57; /// public readonly record struct S57FeaturePointer { - /// Gets the name of the related feature record. - public S57RecordName Name { get; init; } + /// + /// Gets the long name (LNAM) of the related feature object (S-57 Part 3, 4.6). Look the + /// feature up with or + /// . + /// + public S57LongName Name { get; init; } /// Gets the relationship indicator. public S57RelationshipIndicator Relationship { get; init; } diff --git a/src/EncDotNet.S57/S57FeatureRecord.cs b/src/EncDotNet.S57/S57FeatureRecord.cs index 99d7628..eeacf23 100644 --- a/src/EncDotNet.S57/S57FeatureRecord.cs +++ b/src/EncDotNet.S57/S57FeatureRecord.cs @@ -8,6 +8,12 @@ public sealed record S57FeatureRecord /// Gets the record name. public S57RecordName RecordName { get; init; } + /// + /// Gets the feature's long name (LNAM) from its FOID field, which feature pointers use to + /// refer to it, or null when the record has no FOID field. + /// + public S57LongName? LongName => RecordName.LongName; + /// Gets the object geometric primitive (PRIM). public S57GeometricPrimitive Primitive { get; init; } diff --git a/src/EncDotNet.S57/S57LongName.cs b/src/EncDotNet.S57/S57LongName.cs new file mode 100644 index 0000000..c8f4bdb --- /dev/null +++ b/src/EncDotNet.S57/S57LongName.cs @@ -0,0 +1,42 @@ +using System.Globalization; + +namespace EncDotNet.S57; + +/// +/// Represents a feature object identifier, the "Long Name" (LNAM) that identifies a feature +/// object and that feature-to-feature pointers use to refer to it (S-57 Part 3, 4.3 and 4.6). +/// +/// +/// A long name is the feature's producing agency (AGEN), feature identification number (FIDN) +/// and feature identification subdivision (FIDS), from its FOID field. Unlike a record's +/// (RCNM and RCID), it stays the same across editions and updates, +/// which is why FFPT pointers use it as their foreign key. +/// +public readonly record struct S57LongName +{ + /// + /// Initializes a new instance of the struct. + /// + /// The producing agency (AGEN). + /// The feature identification number (FIDN). + /// The feature identification subdivision (FIDS). + public S57LongName(ushort agencyCode, uint featureId, ushort featureSubdivision) + { + AgencyCode = agencyCode; + FeatureId = featureId; + FeatureSubdivision = featureSubdivision; + } + + /// Gets the producing agency code (AGEN). + public ushort AgencyCode { get; init; } + + /// Gets the feature identification number (FIDN). + public uint FeatureId { get; init; } + + /// Gets the feature identification subdivision (FIDS). + public ushort FeatureSubdivision { get; init; } + + /// + public override string ToString() => + string.Create(CultureInfo.InvariantCulture, $"AGEN={AgencyCode}, FIDN={FeatureId}, FIDS={FeatureSubdivision}"); +} diff --git a/src/EncDotNet.S57/S57RecordName.cs b/src/EncDotNet.S57/S57RecordName.cs index 1efe128..3da6bc5 100644 --- a/src/EncDotNet.S57/S57RecordName.cs +++ b/src/EncDotNet.S57/S57RecordName.cs @@ -59,6 +59,15 @@ public static S57RecordName FromLongName(int agencyCode, int featureId, int feat }; } + /// + /// Gets the feature's long name (AGEN, FIDN, FIDS), or null when the record has none, + /// such as a vector record or a feature record without a FOID field. + /// + public S57LongName? LongName => + AgencyCode == 0 && FeatureId == 0 && FeatureSubdivision == 0 + ? null + : new S57LongName((ushort)AgencyCode, unchecked((uint)FeatureId), (ushort)FeatureSubdivision); + /// public override string ToString() { diff --git a/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs b/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs index a11d084..943054f 100644 --- a/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs +++ b/tests/EncDotNet.PerformanceTests/PerformanceBudgets.cs @@ -26,10 +26,10 @@ internal static class PerformanceBudgets public const long Iso8211DocumentWriterWrite = 2_318_456; public const long S57DocumentReaderRead = 3_713_184; public const long S57DocumentApplyChanges = 685_440; - public const long S57ChartFromDocument = 1_030_312; + public const long S57ChartFromDocument = 1_063_752; // Retained bytes held by the resulting model. public const long Iso8211DocumentRetained = 2_048_712; public const long S57DocumentRetained = 1_194_928; - public const long S57ChartRetained = 1_145_696; + public const long S57ChartRetained = 1_178_960; } diff --git a/tests/EncDotNet.UnitTests/S57ChartTests.cs b/tests/EncDotNet.UnitTests/S57ChartTests.cs index 0d2b2ba..621b037 100644 --- a/tests/EncDotNet.UnitTests/S57ChartTests.cs +++ b/tests/EncDotNet.UnitTests/S57ChartTests.cs @@ -880,6 +880,32 @@ public void MetaFeature_HasNoGeometry() Assert.Equal("MetaValue", metaFeature.GetAttributeValue(1)); } + [Fact] + public void GetFeature_ByPointerName_ResolvesTargetAndReferencingFeatures() + { + var master = new S57LongName(550, 1, 1); + var slave = new S57LongName(550, 2, 1); + var masterRecord = CreateFeatureRecord(1, S57GeometricPrimitive.Point, S57ObjectCode.BCNLAT, + featurePointers: [new S57FeaturePointer { Name = slave, Relationship = S57RelationshipIndicator.Master, Comment = "" }]); + masterRecord = masterRecord with { RecordName = masterRecord.RecordName with { AgencyCode = 550, FeatureId = 1, FeatureSubdivision = 1 } }; + var slaveRecord = CreateFeatureRecord(2, S57GeometricPrimitive.Point, S57ObjectCode.LIGHTS); + slaveRecord = slaveRecord with { RecordName = slaveRecord.RecordName with { AgencyCode = 550, FeatureId = 2, FeatureSubdivision = 1 } }; + var withoutFoid = CreateFeatureRecord(3, S57GeometricPrimitive.Point, S57ObjectCode.LIGHTS); + var chart = S57Chart.FromDocument(CreateDocument(features: [masterRecord, slaveRecord, withoutFoid])); + + var pointer = Assert.Single(chart.GetFeature(master)!.RelatedFeatures); + var target = chart.GetFeature(pointer.Name); + + Assert.NotNull(target); + Assert.Equal(2, target.RecordName.RecordId); + Assert.Equal(slave, target.LongName); + Assert.Equal(1, Assert.Single(chart.GetReferencingFeatures(slave)).RecordName.RecordId); + Assert.Empty(chart.GetReferencingFeatures(master)); + Assert.Null(chart.GetFeature(new S57LongName(550, 3, 1))); + Assert.Equal(2, chart.FeaturesByLongName.Count); + Assert.Equal(3, chart.AllFeatures.Count); + } + [Fact] public void MetaFeature_WithRelatedFeatures_PreservesRelationships() { @@ -888,7 +914,7 @@ public void MetaFeature_WithRelatedFeatures_PreservesRelationships() { new S57FeaturePointer { - Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 10), + Name = new S57LongName(550, 10, 1), Relationship = S57RelationshipIndicator.Master, Comment = "Test" } @@ -903,7 +929,7 @@ public void MetaFeature_WithRelatedFeatures_PreservesRelationships() var metaFeature = chart.MetaFeatures[0]; Assert.True(metaFeature.HasRelatedFeatures); Assert.Single(metaFeature.RelatedFeatures); - Assert.Equal(10, metaFeature.RelatedFeatures[0].Name.RecordId); + Assert.Equal(10u, metaFeature.RelatedFeatures[0].Name.FeatureId); Assert.Equal(S57RelationshipIndicator.Master, metaFeature.RelatedFeatures[0].Relationship); } diff --git a/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs b/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs index 6c4a027..e19f30a 100644 --- a/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs +++ b/tests/EncDotNet.UnitTests/S57DocumentApplyChangesTests.cs @@ -817,13 +817,13 @@ public void ApplyChanges_FfpcInsert_InsertsFeaturePointersAtIndex() { var basePtr = new S57FeaturePointer { - Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 100), + Name = new S57LongName(550, 100, 1), Relationship = S57RelationshipIndicator.Master, Comment = "" }; var newPtr = new S57FeaturePointer { - Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 200), + Name = new S57LongName(550, 200, 1), Relationship = S57RelationshipIndicator.Slave, Comment = "" }; @@ -852,8 +852,8 @@ public void ApplyChanges_FfpcInsert_InsertsFeaturePointersAtIndex() var ptrs = result.FeatureRecords[0].FeaturePointers; Assert.Equal(2, ptrs.Count); - Assert.Equal(100, ptrs[0].Name.RecordId); - Assert.Equal(200, ptrs[1].Name.RecordId); + Assert.Equal(100u, ptrs[0].Name.FeatureId); + Assert.Equal(200u, ptrs[1].Name.FeatureId); } #endregion @@ -985,11 +985,11 @@ public void ApplyChanges_FfpcInsertBeyondAppendPosition_ThrowsNamingRecordAndFie { var basePtr = new S57FeaturePointer { - Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 100), + Name = new S57LongName(550, 100, 1), Relationship = S57RelationshipIndicator.Master, Comment = "" }; - var newPtr = basePtr with { Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 200) }; + var newPtr = basePtr with { Name = new S57LongName(550, 200, 1) }; var baseDoc = new S57Document { FeatureRecords = [CreateFeature(1, featurePointers: [basePtr])], @@ -1091,11 +1091,11 @@ public void ApplyChanges_FfpcInsertAtAppendPosition_Succeeds() { var basePtr = new S57FeaturePointer { - Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 100), + Name = new S57LongName(550, 100, 1), Relationship = S57RelationshipIndicator.Master, Comment = "" }; - var newPtr = basePtr with { Name = S57RecordName.FromRcnmRcid(S57RecordNameCodes.Feature, 200) }; + var newPtr = basePtr with { Name = new S57LongName(550, 200, 1) }; var baseDoc = new S57Document { FeatureRecords = [CreateFeature(1, featurePointers: [basePtr])], @@ -1119,7 +1119,7 @@ public void ApplyChanges_FfpcInsertAtAppendPosition_Succeeds() var result = baseDoc.ApplyChanges(update); - Assert.Equal([100, 200], result.FeatureRecords[0].FeaturePointers.Select(p => p.Name.RecordId)); + Assert.Equal([100, 200], result.FeatureRecords[0].FeaturePointers.Select(p => (int)p.Name.FeatureId)); } #endregion diff --git a/tests/EncDotNet.UnitTests/S57DocumentFingerprint.cs b/tests/EncDotNet.UnitTests/S57DocumentFingerprint.cs index 991d6f7..93246e2 100644 --- a/tests/EncDotNet.UnitTests/S57DocumentFingerprint.cs +++ b/tests/EncDotNet.UnitTests/S57DocumentFingerprint.cs @@ -97,6 +97,12 @@ private static string Name(S57RecordName name) => string.Create( CultureInfo.InvariantCulture, $"{name.RecordNameCode}/{name.RecordId}/{name.AgencyCode}/{name.FeatureId}/{name.FeatureSubdivision}"); + // Feature pointers hold only the long name; format it as the record name it used to be, so + // the recorded hashes still hold. + private static string Name(S57LongName name) => string.Create( + CultureInfo.InvariantCulture, + $"0/0/{name.AgencyCode}/{(int)name.FeatureId}/{name.FeatureSubdivision}"); + private static void Attributes(StringBuilder text, string kind, IReadOnlyList attributes) { foreach (var a in attributes) diff --git a/tests/EncDotNet.UnitTests/S57DocumentReaderTests.cs b/tests/EncDotNet.UnitTests/S57DocumentReaderTests.cs index a0c0b32..d8ba5e6 100644 --- a/tests/EncDotNet.UnitTests/S57DocumentReaderTests.cs +++ b/tests/EncDotNet.UnitTests/S57DocumentReaderTests.cs @@ -378,7 +378,8 @@ private static byte[] CreateFeatureRecord( S57SpatialPointer[]? spatialPointers = null, S57FieldUpdateControl? spatialPointerControl = null, S57FeaturePointer[]? featurePointers = null, - S57FieldUpdateControl? featurePointerControl = null) + S57FieldUpdateControl? featurePointerControl = null, + S57LongName? longName = null) { using var ms = new MemoryStream(); using var writer = new BinaryWriter(ms); @@ -401,6 +402,18 @@ private static byte[] CreateFeatureRecord( ("FRID", fridData) }; + // Add FOID field if a long name is provided + if (longName is { } lnam) + { + using var foidMs = new MemoryStream(); + using var foidWriter = new BinaryWriter(foidMs); + foidWriter.Write(lnam.AgencyCode); + foidWriter.Write(lnam.FeatureId); + foidWriter.Write(lnam.FeatureSubdivision); + foidWriter.Write((byte)0x1E); + fields.Add(("FOID", foidMs.ToArray())); + } + // Add ATTF field if attributes provided if (attributes != null && attributes.Length > 0) { @@ -1147,6 +1160,53 @@ public void GetFeatureRecord_ByRecordName_ReturnsMatchingRecord() Assert.Equal(S57ObjectCode.WRECKS, result.ObjectCode); } + [Fact] + public void Read_FeatureWithFeaturePointers_ParsesLongNames() + { + var featureRecord = CreateFeatureRecord( + rcid: 1, + longName: new S57LongName(550, 1001, 1), + featurePointers: + [ + new S57FeaturePointer + { + Name = new S57LongName(550, 4_000_000_000, 2), + Relationship = S57RelationshipIndicator.Slave, + Comment = "", + }, + ]); + var document = S57DocumentReader.Read(CreateS57Document(featureRecord)); + + var feature = Assert.Single(document.FeatureRecords); + Assert.Equal(new S57LongName(550, 1001, 1), feature.LongName); + var pointer = Assert.Single(feature.FeaturePointers); + Assert.Equal(new S57LongName(550, 4_000_000_000, 2), pointer.Name); + Assert.Equal(S57RelationshipIndicator.Slave, pointer.Relationship); + } + + [Fact] + public void GetFeatureRecord_ByLongName_ReturnsMatchingRecord() + { + var data = CreateS57Document( + CreateFeatureRecord(rcid: 1, objl: 75, longName: new S57LongName(550, 1, 1)), + CreateFeatureRecord(rcid: 2, objl: 159, longName: new S57LongName(550, 2, 1))); + var document = S57DocumentReader.Read(data); + + var result = document.GetFeatureRecord(new S57LongName(550, 2, 1)); + + Assert.NotNull(result); + Assert.Equal(2, result.RecordName.RecordId); + Assert.Null(document.GetFeatureRecord(new S57LongName(550, 3, 1))); + } + + [Fact] + public void Read_FeatureWithoutFoid_HasNoLongName() + { + var document = S57DocumentReader.Read(CreateS57Document(CreateFeatureRecord(rcid: 1))); + + Assert.Null(Assert.Single(document.FeatureRecords).LongName); + } + [Fact] public void GetFeatureRecord_NonExistentName_ReturnsNull() { From 999c2585771d4256d72a4e281af6a48cbbcc4c25 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 23:46:49 +0000 Subject: [PATCH 2/2] Note in the s57-enc skill that feature pointers use long names Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01YagvELyJKBANk61DyFGUNJ --- .github/skills/s57-enc/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/skills/s57-enc/SKILL.md b/.github/skills/s57-enc/SKILL.md index bbfd572..2854c77 100644 --- a/.github/skills/s57-enc/SKILL.md +++ b/.github/skills/s57-enc/SKILL.md @@ -59,7 +59,9 @@ out breaking changes and their EncDotNet.S100 impact in the PR description. - **Attributes** (`ImmutableArray`): ATTL/ATVL pairs from ATTF and NATF. - **Spatial pointers** (FSPT, `S57SpatialPointer`) and **feature pointers** - (FFPT, `S57FeaturePointer`). + (FFPT, `S57FeaturePointer`). A feature pointer names its target by long name + (`S57LongName`: AGEN, FIDN, FIDS from the target's FOID), not by RCNM/RCID; + resolve it with `S57Chart.GetFeature(S57LongName)`. **Vector records** (`S57VectorRecord`), by RCNM: