diff --git a/CHANGELOG.md b/CHANGELOG.md
index 49d7203..ae1a810 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1370,6 +1370,25 @@ Current version on `main`: **0.0.1**.
about silently never matched.
### Fixed
+- **Virtual-junction linkage and connection internals round-trip**
+ ([#537](https://github.com/Robomous/RoadMaker/issues/537)). Three holes, all
+ in modeled scopes that #453's preservation sweep deliberately does not cover:
+
+ - **`@elementS`/`@elementDir` were read nowhere.**
+ `asam.net:xodr:1.7.0:road.linkage.virtjunc_link_attribute_usage` *requires*
+ both on a `` pointing at a virtual junction, and `read_link` took only
+ `elementType`/`elementId`/`contactPoint` — with no diagnostic. A conformant
+ file lost its virtual-junction linkage on the first save. They are carried
+ on `RoadLink` now and written back only when the source had them, so an
+ ordinary road link stays byte-identical.
+ - **A virtual junction's own ``s were deleted.** §12.7 permits
+ them, but the arms-xor-spans policy cleared the list — warned, then
+ destroyed. They are preserved instead: held out of `connections` so nothing
+ tries to build geometry from them (arms-xor-spans still holds for
+ *generation*), and re-emitted unchanged.
+ - **`` `@type` and non-`` children were dropped.**
+ `@id` regeneration is benign because the writer renumbers deterministically;
+ `@type` carries meaning the file owns.
- **Five spec areas that were warned about and then thrown away now round-trip**
([#539](https://github.com/Robomous/RoadMaker/issues/539), fmt-f2):
`` and the legacy `` (§10.5.1), road
diff --git a/core/include/roadmaker/road/junction.hpp b/core/include/roadmaker/road/junction.hpp
index d91f9dd..c4bdd05 100644
--- a/core/include/roadmaker/road/junction.hpp
+++ b/core/include/roadmaker/road/junction.hpp
@@ -114,6 +114,18 @@ struct JunctionConnection {
/// Pairs of {incoming lane odr_id, connecting lane odr_id}.
std::vector> lane_links;
+
+ /// `@type` — `"default"` or `"virtual"` on a connection (§12.3, Table 61),
+ /// kept verbatim. Empty ⇒ absent. Dropped before #537: `@id` regeneration is
+ /// benign because the writer renumbers deterministically, but `@type` carries
+ /// meaning the file owns.
+ std::string type_str;
+
+ /// Unknown attributes and unmodeled children of `` — anything
+ /// besides `` — preserved verbatim (#537). The reader iterated
+ /// `children("laneLink")` only, so a ``/`` on a
+ /// virtual-junction connection, or any later addition, vanished silently.
+ RawXml preserved;
};
/// An authored override for the pavement fillet at one junction corner — the
@@ -492,6 +504,16 @@ struct Junction {
/// merely defaulted.
std::string type_str;
+ /// Connections a VIRTUAL junction declared (§12.7), held aside rather than
+ /// deleted (#537).
+ ///
+ /// arms-xor-spans still holds for GENERATION: a span junction never cuts its
+ /// main road, so there is nothing to derive and these must stay out of
+ /// `connections` or the mesher would try. But a virtual junction may legally
+ /// declare them, and clearing the list destroyed a conformant file's data.
+ /// The writer re-emits these verbatim.
+ std::vector preserved_connections;
+
/// `` elements of a **direct** junction (§12.4). Non-empty only
/// when `type == JunctionType::Direct`; a direct junction's connections have
/// no connecting road, so they cannot live in `connections` and the derived
diff --git a/core/include/roadmaker/road/road.hpp b/core/include/roadmaker/road/road.hpp
index 4f9b8e0..d9626cc 100644
--- a/core/include/roadmaker/road/road.hpp
+++ b/core/include/roadmaker/road/road.hpp
@@ -42,6 +42,19 @@ enum class ContactPoint {
struct RoadLink {
std::variant target;
ContactPoint contact = ContactPoint::Start;
+
+ /// `@elementS` — where on the linked element the connection sits [m].
+ /// Required alongside `@elementDir` when the link points at a **virtual**
+ /// junction (`asam.net:xodr:1.7.0:road.linkage.virtjunc_link_attribute_usage`,
+ /// §10 road linkage) and absent otherwise. Unset ⇒ the attribute was absent,
+ /// so it round-trips absent (#537).
+ std::optional element_s;
+
+ /// `@elementDir` — `"+"` or `"-"`, the direction along the linked element.
+ /// Kept verbatim: it is a two-value enumeration whose spelling is the whole
+ /// content, and an unknown value must survive rather than be normalised.
+ /// Empty ⇒ absent.
+ std::string element_dir;
};
/// Plan-view waypoint [m] for clothoid path fitting (authoring API and the
diff --git a/core/src/xodr/reader.cpp b/core/src/xodr/reader.cpp
index 44f958a..58bfc76 100644
--- a/core/src/xodr/reader.cpp
+++ b/core/src/xodr/reader.cpp
@@ -2375,8 +2375,19 @@ class Parser {
if (!junction.arms.empty() || !junction.connections.empty()) {
diag(Severity::Warning,
location,
- "virtual junction also declares connections or rm:arms; they were dropped",
+ "virtual junction also declares connections or rm:arms; RoadMaker "
+ "generates no geometry for them, but they are preserved and "
+ "re-emitted unchanged",
rules::kJunctionVirtualAttributes);
+ // arms-xor-spans still holds for GENERATION — a span junction never
+ // cuts its main road, so there is nothing to derive. But a virtual
+ // junction may legally declare connections (§12.7), and deleting them
+ // destroyed a conformant file's data (#537). They are moved to the
+ // preserved tier instead: absent from `connections` so nothing tries to
+ // build geometry from them, and re-emitted verbatim on write.
+ for (const JunctionConnection& connection : junction.connections) {
+ junction.preserved_connections.push_back(connection);
+ }
junction.arms.clear();
junction.connections.clear();
}
@@ -2459,6 +2470,14 @@ class Parser {
result.lane_links.emplace_back(lane_link.attribute("from").as_int(),
lane_link.attribute("to").as_int());
}
+ // @type carries meaning the file owns (§12.3 Table 61); @id does not,
+ // because the writer renumbers connections deterministically. Unknown
+ // attributes and non- children ride the preserved tier (#537).
+ result.type_str = connection.attribute("type").value();
+ static constexpr std::string_view kConnectionAttrs[] = {
+ "id", "incomingRoad", "connectingRoad", "contactPoint", "type"};
+ static constexpr std::string_view kConnectionChildren[] = {"laneLink"};
+ capture_unmodeled(connection, kConnectionAttrs, kConnectionChildren, result.preserved);
junction.connections.push_back(std::move(result));
}
}
@@ -3480,7 +3499,9 @@ class Parser {
struct PendingLink {
std::string element_type; // "road" | "junction"
std::string element_id;
- std::string contact_point; // "start" | "end" (roads only)
+ std::string contact_point; // "start" | "end" (roads only)
+ std::optional element_s; // virtual junctions only (§10)
+ std::string element_dir; // "+" | "-", virtual junctions only
bool present = false;
};
@@ -3495,12 +3516,21 @@ class Parser {
if (!node) {
return {};
}
- return PendingLink{
+ PendingLink link{
.element_type = node.attribute("elementType").value(),
.element_id = node.attribute("elementId").value(),
.contact_point = node.attribute("contactPoint").value(),
+ .element_dir = node.attribute("elementDir").value(),
.present = true,
};
+ // @elementS/@elementDir are REQUIRED when the link points at a virtual
+ // junction (asam.net:xodr:1.7.0:road.linkage.virtjunc_link_attribute_usage)
+ // and were read nowhere before #537, so a conformant file lost its
+ // virtual-junction linkage on save with no diagnostic.
+ if (const pugi::xml_attribute element_s = node.attribute("elementS")) {
+ link.element_s = element_s.as_double();
+ }
+ return link;
}
void resolve_references() {
@@ -3564,6 +3594,11 @@ class Parser {
rules::kRoadLinkAttributeUsage);
return std::nullopt;
}
+ // Carried for every link kind, though only a virtual junction may legally
+ // use them: preserving what the file wrote beats silently normalising it,
+ // and the writer emits them iff they were present (#537).
+ link.element_s = pending.element_s;
+ link.element_dir = pending.element_dir;
return link;
}
diff --git a/core/src/xodr/writer.cpp b/core/src/xodr/writer.cpp
index 4a63426..8445c1f 100644
--- a/core/src/xodr/writer.cpp
+++ b/core/src/xodr/writer.cpp
@@ -390,6 +390,15 @@ void write_link_element(pugi::xml_node link,
node.append_attribute("elementType").set_value("junction");
node.append_attribute("elementId").set_value(element_id.c_str());
}
+ // @elementS/@elementDir: required for a link into a VIRTUAL junction
+ // (asam.net:xodr:1.7.0:road.linkage.virtjunc_link_attribute_usage) and absent
+ // otherwise, so they are written iff the source carried them (#537).
+ if (road_link.element_s.has_value()) {
+ set_num(node, "elementS", *road_link.element_s);
+ }
+ if (!road_link.element_dir.empty()) {
+ node.append_attribute("elementDir").set_value(road_link.element_dir.c_str());
+ }
}
void write_lane(pugi::xml_node side, const Lane& lane) {
@@ -1658,11 +1667,50 @@ void write_junction(pugi::xml_node root,
node.append_attribute("connectingRoad").set_value(connecting->odr_id.c_str());
node.append_attribute("contactPoint")
.set_value(connection.contact_point == ContactPoint::End ? "end" : "start");
+ if (!connection.type_str.empty()) {
+ node.append_attribute("type").set_value(connection.type_str.c_str());
+ }
+ for (const auto& [name, value] : connection.preserved.attributes) {
+ node.append_attribute(name.c_str()).set_value(value.c_str());
+ }
for (const auto& [from, to] : connection.lane_links) {
pugi::xml_node link = node.append_child("laneLink");
link.append_attribute("from").set_value(from);
link.append_attribute("to").set_value(to);
}
+ for (const std::string& fragment : connection.preserved.children) {
+ append_fragment(node, fragment);
+ }
+ }
+
+ // A virtual junction's own connections (§12.7): held aside by the reader so
+ // nothing tries to build geometry from them, re-emitted here unchanged (#537).
+ for (const JunctionConnection& connection : junction.preserved_connections) {
+ const Road* incoming = network.road(connection.incoming_road);
+ const Road* connecting = network.road(connection.connecting_road);
+ if (incoming == nullptr || connecting == nullptr) {
+ continue;
+ }
+ pugi::xml_node node = junction_node.append_child("connection");
+ node.append_attribute("id").set_value(connection_id++);
+ node.append_attribute("incomingRoad").set_value(incoming->odr_id.c_str());
+ node.append_attribute("connectingRoad").set_value(connecting->odr_id.c_str());
+ node.append_attribute("contactPoint")
+ .set_value(connection.contact_point == ContactPoint::End ? "end" : "start");
+ if (!connection.type_str.empty()) {
+ node.append_attribute("type").set_value(connection.type_str.c_str());
+ }
+ for (const auto& [name, value] : connection.preserved.attributes) {
+ node.append_attribute(name.c_str()).set_value(value.c_str());
+ }
+ for (const auto& [from, to] : connection.lane_links) {
+ pugi::xml_node link = node.append_child("laneLink");
+ link.append_attribute("from").set_value(from);
+ link.append_attribute("to").set_value(to);
+ }
+ for (const std::string& fragment : connection.preserved.children) {
+ append_fragment(node, fragment);
+ }
}
// The signal synchronization group (§12.14, Table 84): ``
diff --git a/core/tests/fuzz/corpus/virtual_junction_links.xodr b/core/tests/fuzz/corpus/virtual_junction_links.xodr
new file mode 100644
index 0000000..e8012ff
--- /dev/null
+++ b/core/tests/fuzz/corpus/virtual_junction_links.xodr
@@ -0,0 +1,75 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/core/tests/test_round_trip.cpp b/core/tests/test_round_trip.cpp
index e073ae9..9cff93f 100644
--- a/core/tests/test_round_trip.cpp
+++ b/core/tests/test_round_trip.cpp
@@ -416,6 +416,81 @@ TEST(RoundTrip, PreservationDoesNotDuplicateModeledOrDerivedContent) {
EXPECT_EQ(count("network;
+
+ // 1. @elementS/@elementDir reached the model. Required for a virtual-junction
+ // link (road.linkage.virtjunc_link_attribute_usage) and read nowhere before.
+ const roadmaker::Road& main = *network.road(network.find_road("1"));
+ ASSERT_TRUE(main.predecessor.has_value());
+ ASSERT_TRUE(main.predecessor->element_s.has_value());
+ EXPECT_DOUBLE_EQ(*main.predecessor->element_s, 0.0);
+ EXPECT_EQ(main.predecessor->element_dir, "+");
+ ASSERT_TRUE(main.successor.has_value());
+ ASSERT_TRUE(main.successor->element_s.has_value());
+ EXPECT_DOUBLE_EQ(*main.successor->element_s, 200.0);
+ EXPECT_EQ(main.successor->element_dir, "-");
+
+ // 2. The virtual junction's connection was PRESERVED, not deleted — and is
+ // held out of `connections` so nothing tries to build geometry from it.
+ const roadmaker::Junction& junction = *network.junction(network.find_junction("100"));
+ EXPECT_TRUE(junction.connections.empty()) << "a span junction generates nothing";
+ ASSERT_EQ(junction.preserved_connections.size(), 1U) << "the connection was destroyed again";
+
+ // 3. @type and the unmodeled child rode along.
+ EXPECT_EQ(junction.preserved_connections[0].type_str, "virtual");
+ EXPECT_FALSE(junction.preserved_connections[0].preserved.children.empty());
+
+ // The bytes. A model-only assertion would pass on a writer that emits none
+ // of it.
+ const auto written = roadmaker::write_xodr(loaded->network, "virtual_junction_links");
+ ASSERT_TRUE(written.has_value());
+ EXPECT_NE(written->find(R"(elementS="0")"), std::string::npos);
+ EXPECT_NE(written->find(R"(elementS="200")"), std::string::npos);
+ EXPECT_NE(written->find(R"(elementDir="+")"), std::string::npos);
+ EXPECT_NE(written->find(R"(elementDir="-")"), std::string::npos);
+ EXPECT_NE(written->find(R"(type="virtual")"), std::string::npos);
+ EXPECT_NE(written->find("acme:connectionNote"), std::string::npos);
+
+ // Fixed point, and the linkage is still there after the second trip.
+ const auto reparsed = roadmaker::parse_xodr(*written, "virtual_junction_links");
+ ASSERT_TRUE(reparsed.has_value());
+ const roadmaker::Road& round = *reparsed->network.road(reparsed->network.find_road("1"));
+ ASSERT_TRUE(round.predecessor.has_value());
+ EXPECT_EQ(round.predecessor->element_dir, "+");
+ const auto again = roadmaker::write_xodr(reparsed->network, "virtual_junction_links");
+ ASSERT_TRUE(again.has_value());
+ EXPECT_EQ(*written, *again);
+ EXPECT_EQ(roadmaker::count_errors(loaded->diagnostics), 0U);
+}
+
+TEST(RoundTrip, AnOrdinaryRoadLinkWritesNoElementSOrDir) {
+ // @elementS/@elementDir are virtual-junction-only, so an ordinary link must
+ // not start emitting them — that would churn every fixture in the suite and
+ // assert something the standard forbids there.
+ RoadNetwork network;
+ const auto road = roadmaker::author_clothoid_road(
+ network,
+ std::array{Waypoint{.x = 0.0, .y = 0.0}, Waypoint{.x = 100.0, .y = 0.0}},
+ LaneProfile::two_lane_default(),
+ "Plain",
+ "1");
+ ASSERT_TRUE(road.has_value());
+ const auto written = roadmaker::write_xodr(network, "plain");
+ ASSERT_TRUE(written.has_value());
+ EXPECT_EQ(written->find("elementS="), std::string::npos);
+ EXPECT_EQ(written->find("elementDir="), std::string::npos);
+}
+
// --- direct and crossing junctions (#534) ------------------------------------
//
// A direct junction (§12.4) has NO connecting road: each carries