diff --git a/CHANGELOG.md b/CHANGELOG.md index 17e6ce90..a7fa4b06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1465,6 +1465,24 @@ Current version on `main`: **0.0.1**. same undo macro, instead of creating a second object. ### Changed +- **The persistence layer keeps one table per OpenDRIVE enum instead of two** + ([#563](https://github.com/Robomous/RoadMaker/issues/563)). Six enums — + `e_laneType`, `e_roadMarkType`, `e_roadMarkColor`, `e_lane_direction`, + `e_objectType` and `@orientation` — were spelled out twice: an `if`-chain in + `xodr/reader.cpp` and a `switch` in `xodr/writer.cpp`, in different files, + with nothing tying the two directions together. That is the shape + [#476](https://github.com/Robomous/RoadMaker/issues/476) came in: the writer + re-spelled parsed enums into different semantics on save. They now share one + `constexpr` table each (`core/src/xodr/enum_names.hpp`), so a read and a write + cannot disagree. Five pugixml scalar helpers the two formats each kept a + private copy of (`to_double`, `num`, `set_num`, `set_optional_num`, + `node_to_string`) collapse into `core/src/xml/xml_common.hpp` the same way. + No behaviour change: re-emitting all 143 tracked `.xodr`/`.xosc` fixtures is + byte-identical, diagnostics included. + + `append_fragment` was NOT shared despite the identical name and shape — the + OpenSCENARIO writer passes `pugi::parse_fragment` and the OpenDRIVE one does + not, so that pair is a real divergence and stays per-format. - **New roads default to the urban-with-sidewalks template** ([#355](https://github.com/Robomous/RoadMaker/issues/355)): the Create Road tool, its toolbar dropdown, and the Library fallback now start from @@ -1876,6 +1894,12 @@ Current version on `main`: **0.0.1**. `asam.net:xodr:1.4.0:ids.only_ref_defined_ids` so the drop is never silent. ### Removed +- `Environment::procedural_sky` (editor renderer) + ([#563](https://github.com/Robomous/RoadMaker/issues/563)). Set by + `sober_lighting()` and asserted by two scene-builder tests, but never read by + `GLRenderer` — a dead flag with a passing test and a "later render polish" + promise attached. The sampled-HDRI path it stood for will need its own field + if it is ever built. - `edit::junction_stop_lines` and `StopLineParams`, superseded by the derived stop-line entity above; the "Add stop lines to all arms" junction context action goes with them, since every arm already has one. diff --git a/core/src/edit/operations.cpp b/core/src/edit/operations.cpp index 3651732b..2560e59b 100644 --- a/core/src/edit/operations.cpp +++ b/core/src/edit/operations.cpp @@ -60,6 +60,7 @@ #include "../mesh/junction_stoplines_detail.hpp" #include "../mesh/object_placement.hpp" +#include "../road/junction_adjacency.hpp" namespace roadmaker::edit { @@ -991,18 +992,7 @@ std::unique_ptr junction_stage(const RoadNetwork& network, std::span carried, TurnSetPolicy policy); -/// The road end a link names, or nullopt when the link is absent or points at a -/// junction rather than a road. -std::optional linked_end(const std::optional& link) { - if (!link.has_value()) { - return std::nullopt; - } - const RoadId* road = std::get_if(&link->target); - if (road == nullptr) { - return std::nullopt; - } - return RoadEnd{.road = *road, .contact = link->contact}; -} +using road_detail::linked_end; /// The link slot a contact owns: predecessor at a Start, successor at an End. std::optional& link_slot(Road& road, ContactPoint contact) { diff --git a/core/src/mesh/junction_maneuvers.cpp b/core/src/mesh/junction_maneuvers.cpp index 1337c148..bd69e5c3 100644 --- a/core/src/mesh/junction_maneuvers.cpp +++ b/core/src/mesh/junction_maneuvers.cpp @@ -29,22 +29,13 @@ #include #include +#include "../road/junction_adjacency.hpp" + namespace roadmaker { namespace { -/// The road end a connecting road's link names, or nullopt when the link is -/// absent or points at a junction rather than a road. -std::optional linked_end(const std::optional& link) { - if (!link.has_value()) { - return std::nullopt; - } - const RoadId* road = std::get_if(&link->target); - if (road == nullptr) { - return std::nullopt; - } - return RoadEnd{.road = *road, .contact = link->contact}; -} +using road_detail::linked_end; /// The outgoing lane id a connecting road links to — the successor of its /// single right-hand driving lane. Mirrors retarget_junction's TurnKey read so diff --git a/core/src/osc/reader.cpp b/core/src/osc/reader.cpp index 46bf4987..8e222bc9 100644 --- a/core/src/osc/reader.cpp +++ b/core/src/osc/reader.cpp @@ -53,8 +53,7 @@ #include #include -#include - +#include #include #include #include @@ -69,39 +68,18 @@ #include #include +#include "../xml/xml_common.hpp" + namespace roadmaker::osc { namespace { // --- scalars ---------------------------------------------------------------- -/// Locale-independent `double` parsing; rejects trailing garbage and -/// non-finite results. -/// -/// Copied from core/src/xodr/reader.cpp:53-71 rather than shared, for the same -/// reason `num()` is copied in this format's writer (osc/writer.cpp:59-63): the -/// two standards' scalar policies are independent and either may need to -/// diverge. OpenSCENARIO additionally admits `$parameter` expressions in -/// numeric attributes (§9), which this reader does NOT evaluate — such a value -/// fails here and takes the preserve-the-spelling path below, which is the -/// correct outcome for a value whose meaning is only known at runtime. -std::optional to_double(std::string_view text) { - const char* first = text.data(); - const char* last = text.data() + text.size(); - double value{}; - const auto result = fast_float::from_chars(first, last, value); - if (result.ec != std::errc{}) { - return std::nullopt; - } - for (const char* p = result.ptr; p != last; ++p) { - if (*p != ' ' && *p != '\t' && *p != '\r' && *p != '\n') { - return std::nullopt; - } - } - if (!std::isfinite(value)) { - return std::nullopt; - } - return value; -} +/// A numeric attribute this rejects is never dropped: it takes the +/// preserve-the-spelling path below. That is the correct outcome for +/// OpenSCENARIO's `$parameter` expressions (§9), which this reader does not +/// evaluate because their meaning is only known at runtime. +using xml_common::to_double; /// Strict non-negative integer parsing for `@revMajor` / `@revMinor`. std::optional to_revision(std::string_view text) { @@ -118,25 +96,10 @@ std::optional to_revision(std::string_view text) { return value; } -/// Serializes a node as a self-contained XML fragment, for the preserved tier. -/// -/// `pugi::format_raw` drops the indentation the source document happened to -/// carry, which is why a preserved fragment comes back re-canonicalized rather -/// than byte-identical — fmt-s2's caveat (#326), stated here because this is -/// where it originates. -std::string node_to_string(const pugi::xml_node& node) { - std::ostringstream out; - node.print(out, "", pugi::format_raw); - return out.str(); -} +using xml_common::node_to_string; bool is_one_of(std::string_view name, std::initializer_list known) { - for (const std::string_view candidate : known) { - if (name == candidate) { - return true; - } - } - return false; + return std::ranges::find(known, name) != known.end(); } std::optional to_semantics(std::string_view text) { diff --git a/core/src/osc/writer.cpp b/core/src/osc/writer.cpp index 7af17aa6..91317bd9 100644 --- a/core/src/osc/writer.cpp +++ b/core/src/osc/writer.cpp @@ -51,32 +51,16 @@ #include #include +#include "../xml/xml_common.hpp" + namespace roadmaker::osc { namespace { // --- formatting ------------------------------------------------------------- -/// Shortest-precision round-trippable formatting; locale-independent. -/// -/// Copied deliberately from core/src/xodr/writer.cpp:52-60 rather than shared: -/// the two formats' number policies are independent and either may need to -/// diverge. The "-0" normalization is load-bearing — without it a negative -/// zero reaches the file and no round trip normalizes it away — and it has its -/// own test here, because the OpenDRIVE suite does not cover this copy. -std::string num(double value) { - std::string text = fmt::format("{}", value); - return text == "-0" ? "0" : text; -} - -void set_num(pugi::xml_node node, const char* name, double value) { - node.append_attribute(name).set_value(num(value).c_str()); -} - -void set_optional_num(pugi::xml_node node, const char* name, const std::optional& value) { - if (value.has_value()) { - set_num(node, name, *value); - } -} +using xml_common::num; +using xml_common::set_num; +using xml_common::set_optional_num; /// Sets an attribute only when the string is non-empty. Optional OpenSCENARIO /// attributes are omitted, never written empty: `reference=""` names a @@ -148,6 +132,10 @@ std::size_t preserved_element_count(const RawXml& preserved, std::string_view el return count; } +/// Appends a preserved fragment verbatim. NOT shareable with the OpenDRIVE +/// writer's same-named helper despite the identical body shape: this one adds +/// `pugi::parse_fragment` and that one takes pugixml's defaults. A REAL +/// divergence, unlike the scalar helpers those two writers used to copy (#563). void append_fragment(pugi::xml_node parent, const std::string& fragment) { parent.append_buffer( fragment.data(), fragment.size(), pugi::parse_default | pugi::parse_fragment); diff --git a/core/src/road/junction_adjacency.hpp b/core/src/road/junction_adjacency.hpp index 6aff68d3..2e610b11 100644 --- a/core/src/road/junction_adjacency.hpp +++ b/core/src/road/junction_adjacency.hpp @@ -80,4 +80,21 @@ touched_junctions(const RoadNetwork& network, RoadId id, const Road& road) { return false; } +/// The road end a link names, or nullopt when the link is absent or points at a +/// junction rather than a road. +/// +/// One definition, shared by the command layer and the junction mesher (#563): +/// both need to ask "which road end is on the other side of this link?", and +/// both used to answer it with their own byte-identical copy. +[[nodiscard]] inline std::optional linked_end(const std::optional& link) { + if (!link.has_value()) { + return std::nullopt; + } + const RoadId* road = std::get_if(&link->target); + if (road == nullptr) { + return std::nullopt; + } + return RoadEnd{.road = *road, .contact = link->contact}; +} + } // namespace roadmaker::road_detail diff --git a/core/src/xml/xml_common.hpp b/core/src/xml/xml_common.hpp new file mode 100644 index 00000000..dff96772 --- /dev/null +++ b/core/src/xml/xml_common.hpp @@ -0,0 +1,98 @@ +/* + * Copyright 2026 Robomous + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +// Internal (non-installed) pugixml scalar helpers shared by both persistence +// layers, xodr/ and osc/. Each existed twice, per format, each copy carrying a +// comment defending the duplication against a divergence that never happened +// (#563). Split them again the day a policy actually differs. +// +// NOT shared, deliberately: `append_fragment`. The two formats pass different +// pugixml parse flags (OpenSCENARIO adds `parse_fragment`) — a real divergence. + +#include +#include + +#include + +#include +#include +#include +#include +#include +#include + +namespace roadmaker::xml_common { + +/// Locale-independent double parsing; rejects trailing garbage (whitespace is +/// tolerated). `std::stod` is locale-dependent — never use it for ASAM IO. +/// +/// A value this rejects is never dropped: both readers fall through to the +/// preserve-the-spelling path. That is the correct outcome for OpenSCENARIO's +/// `$parameter` expressions (§9), whose meaning is only known at runtime. +[[nodiscard]] inline std::optional to_double(std::string_view text) { + const char* first = text.data(); + const char* last = text.data() + text.size(); + double value{}; + const auto result = fast_float::from_chars(first, last, value); + if (result.ec != std::errc{}) { + return std::nullopt; + } + for (const char* p = result.ptr; p != last; ++p) { + if (*p != ' ' && *p != '\t' && *p != '\r' && *p != '\n') { + return std::nullopt; + } + } + if (!std::isfinite(value)) { + return std::nullopt; + } + return value; +} + +/// Shortest-precision round-trippable formatting; locale-independent. +/// +/// The "-0" normalization is load-bearing: without it a negative zero reaches +/// the file and no round trip normalizes it away. +[[nodiscard]] inline std::string num(double value) { + std::string text = fmt::format("{}", value); + return text == "-0" ? "0" : text; +} + +inline void set_num(pugi::xml_node node, const char* name, double value) { + node.append_attribute(name).set_value(num(value).c_str()); +} + +inline void +set_optional_num(pugi::xml_node node, const char* name, const std::optional& value) { + if (value.has_value()) { + set_num(node, name, *value); + } +} + +/// Serializes a node as a self-contained XML fragment, for the verbatim +/// preservation tier (roadmaker/xodr/raw_xml.hpp). +/// +/// `pugi::format_raw` drops whatever indentation the source document happened +/// to carry, which is why a preserved fragment comes back re-canonicalized +/// rather than byte-identical — fmt-s2's caveat (#326) originates here. +[[nodiscard]] inline std::string node_to_string(const pugi::xml_node& node) { + std::ostringstream out; + node.print(out, "", pugi::format_raw); + return out.str(); +} + +} // namespace roadmaker::xml_common diff --git a/core/src/xodr/enum_names.hpp b/core/src/xodr/enum_names.hpp new file mode 100644 index 00000000..e31413cb --- /dev/null +++ b/core/src/xodr/enum_names.hpp @@ -0,0 +1,165 @@ +/* + * Copyright 2026 Robomous + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +// Internal (non-installed): every OpenDRIVE enum spelling, one table per enum, +// shared by xodr/reader.cpp and xodr/writer.cpp. Before #563 each of these was +// an if-chain in the reader AND a switch in the writer, in different files — +// the shape #476 came in, where the two drifted and the writer re-spelled +// parsed enums into different semantics on save. +// +// One row per accepted spelling; the FIRST row naming a value is what the +// writer emits, later rows with that value are read-only aliases. +// +// THREE THINGS THAT MUST NEVER BECOME A ROW: +// 1. `*::Other` — the unmodeled bucket. Its lossy write stand-in is +// `name_of`'s `fallback` argument instead. As a row, "solid" would parse +// back as `RoadMarkType::Other`: #476, reintroduced. +// 2. The empty string — "attribute absent" is per-attribute policy, and it +// belongs at the call site that cites the paragraph saying so. +// 3. Any guess for an unknown spelling. `value_of` returns nullopt; a default +// buried here is a default nobody reviews. +// +// Reference: ASAM OpenDRIVE 1.9.0 §11.7, §11.9 Tables 47-48, §13.2 Table 92, +// §10.2 Table 23. Local copies: third_party/asam/. + +#include "roadmaker/road/lane.hpp" +#include "roadmaker/road/object.hpp" +#include "roadmaker/road/traffic_rule.hpp" + +#include +#include +#include +#include +#include + +namespace roadmaker::xodr_names { + +/// A spelling table: enum value -> the ASCII spelling(s) OpenDRIVE uses. +/// `const char*` rather than `string_view` so `name_of` drops straight into +/// pugixml's `set_value` with no null-termination question. +template +using Table = std::array, N>; + +/// The spelling the writer emits for `value`: the first row naming it, or +/// `fallback` when the value has no faithful spelling (see rule 1 above). +template +[[nodiscard]] constexpr const char* +name_of(const Table& table, E value, const char* fallback) { + for (const auto& [candidate, name] : table) { + if (candidate == value) { + return name; + } + } + return fallback; +} + +/// The value `spelling` names, or nullopt when no row matches. Never guesses — +/// empty-string and unknown-spelling policy belongs to the caller (rules 2, 3). +template +[[nodiscard]] constexpr std::optional value_of(const Table& table, + std::string_view spelling) { + for (const auto& [value, name] : table) { + if (spelling == name) { + return value; + } + } + return std::nullopt; +} + +/// e_laneType (§11.7, Table 43). `Other` is absent by rule 1; the writer's +/// fallback for it is "none". +inline constexpr Table kLaneType{{ + {LaneType::Driving, "driving"}, + {LaneType::Stop, "stop"}, + {LaneType::Shoulder, "shoulder"}, + {LaneType::Biking, "biking"}, + {LaneType::Sidewalk, "sidewalk"}, + {LaneType::Border, "border"}, + {LaneType::Restricted, "restricted"}, + {LaneType::Parking, "parking"}, + {LaneType::Median, "median"}, + {LaneType::Curb, "curb"}, + {LaneType::None, "none"}, + // Read-only alias. "walking" is the pre-1.6 spelling of "sidewalk" and is + // DEPRECATED — accepted on read, never emitted, which is why it sorts after + // the canonical row rather than replacing it (#476). + {LaneType::Sidewalk, "walking"}, +}}; + +/// e_roadMarkType (§11.9, Table 47). The multi-token spellings carry their +/// single ASCII space verbatim — it is part of the enumerator, not formatting. +/// `Other` is absent by rule 1; the writer's fallback for it is "solid". +inline constexpr Table kRoadMarkType{{ + {RoadMarkType::None, "none"}, + {RoadMarkType::Solid, "solid"}, + {RoadMarkType::Broken, "broken"}, + {RoadMarkType::SolidSolid, "solid solid"}, + {RoadMarkType::SolidBroken, "solid broken"}, + {RoadMarkType::BrokenSolid, "broken solid"}, + {RoadMarkType::BrokenBroken, "broken broken"}, +}}; + +/// e_roadMarkColor (§11.9, Table 48). `Other` is absent by rule 1; the writer's +/// fallback for it is "standard". +inline constexpr Table kRoadMarkColor{{ + {RoadMarkColor::Standard, "standard"}, + {RoadMarkColor::White, "white"}, + {RoadMarkColor::Yellow, "yellow"}, + {RoadMarkColor::Red, "red"}, + {RoadMarkColor::Blue, "blue"}, + {RoadMarkColor::Green, "green"}, + {RoadMarkColor::Orange, "orange"}, +}}; + +/// e_lane_direction (§11.7, 1.8.0+). Every value is spellable, so the writer's +/// fallback is unreachable and the reader propagates nullopt for an unknown. +inline constexpr Table kLaneDirection{{ + {LaneDirection::Standard, "standard"}, + {LaneDirection::Reversed, "reversed"}, + {LaneDirection::Both, "both"}, +}}; + +/// e_objectType (§13.2, Table 92). `Other` is absent by rule 1; the writer's +/// fallback for it is "none" — the exotic spelling lives in `Object::type_str`. +inline constexpr Table kObjectType{{ + {ObjectType::Crosswalk, "crosswalk"}, + {ObjectType::Tree, "tree"}, + {ObjectType::Vegetation, "vegetation"}, + {ObjectType::Pole, "pole"}, + {ObjectType::Barrier, "barrier"}, + {ObjectType::Building, "building"}, + {ObjectType::Obstacle, "obstacle"}, + {ObjectType::None, "none"}, +}}; + +/// `@orientation` on `` and `` (§13). Both readers used to +/// spell this test out by hand, identically (#563). +inline constexpr Table kObjectOrientation{{ + {ObjectOrientation::Plus, "+"}, + {ObjectOrientation::Minus, "-"}, + {ObjectOrientation::None, "none"}, +}}; + +/// e_trafficRule (§10.2, Table 23). Absent means RHT — the spec mandates it — +/// and that policy stays at the call site with the paragraph that says so. +inline constexpr Table kTrafficRule{{ + {TrafficRule::RightHandTraffic, "RHT"}, + {TrafficRule::LeftHandTraffic, "LHT"}, +}}; + +} // namespace roadmaker::xodr_names diff --git a/core/src/xodr/reader.cpp b/core/src/xodr/reader.cpp index 58bfc76f..55757b07 100644 --- a/core/src/xodr/reader.cpp +++ b/core/src/xodr/reader.cpp @@ -28,8 +28,6 @@ #include #include -#include - #include #include #include @@ -45,32 +43,15 @@ #include #include +#include "../xml/xml_common.hpp" +#include "enum_names.hpp" #include "lane_border.hpp" namespace roadmaker { namespace { -/// Locale-independent double parsing; rejects trailing garbage (whitespace -/// is tolerated). std::stod is locale-dependent — never use it for xodr IO. -std::optional to_double(std::string_view text) { - const char* first = text.data(); - const char* last = text.data() + text.size(); - double value{}; - const auto result = fast_float::from_chars(first, last, value); - if (result.ec != std::errc{}) { - return std::nullopt; - } - for (const char* p = result.ptr; p != last; ++p) { - if (*p != ' ' && *p != '\t' && *p != '\r' && *p != '\n') { - return std::nullopt; - } - } - if (!std::isfinite(value)) { - return std::nullopt; - } - return value; -} +using xml_common::to_double; /// Strict decimal integer parsing for the rm:floor sort index (p4-s5, issue /// #320): no whitespace, no sign but a leading '-', no leading zeros, and the @@ -133,13 +114,7 @@ std::string_view trimmed(std::string_view text) { return text.substr(first, text.find_last_not_of(kSpace) - first + 1); } -/// Serializes a node as a self-contained XML fragment (no indentation), for -/// the verbatim-preservation tier (roadmaker/xodr/raw_xml.hpp). -std::string node_to_string(const pugi::xml_node& node) { - std::ostringstream out; - node.print(out, "", pugi::format_raw); - return out.str(); -} +using xml_common::node_to_string; /// Capture everything about `node` that the caller does NOT model into `out` — /// the Preserved tier's one implementation (fmt-f1, #453). @@ -1226,68 +1201,36 @@ class Parser { current_lane_ = {}; } + // The spellings themselves live in ONE table per enum, shared with the + // writer (xodr/enum_names.hpp). What stays here is the per-attribute POLICY + // the spec assigns to an absent or unrecognised value — deliberately not + // folded into the tables, because it differs attribute by attribute and each + // rule cites a different paragraph. + + /// e_laneType (§11.7, Table 43). Empty/absent -> None; an unknown spelling -> + /// Other, whose verbatim text `Lane::type_str` keeps. static LaneType lane_type_from_string(std::string_view name) { - if (name == "driving") - return LaneType::Driving; - if (name == "stop") - return LaneType::Stop; - if (name == "shoulder") - return LaneType::Shoulder; - if (name == "biking") - return LaneType::Biking; - if (name == "sidewalk" || name == "walking") - return LaneType::Sidewalk; - if (name == "border") - return LaneType::Border; - if (name == "restricted") - return LaneType::Restricted; - if (name == "parking") - return LaneType::Parking; - if (name == "median") - return LaneType::Median; - if (name == "curb") - return LaneType::Curb; - if (name == "none" || name.empty()) + if (name.empty()) { return LaneType::None; - return LaneType::Other; + } + return xodr_names::value_of(xodr_names::kLaneType, name).value_or(LaneType::Other); } + /// e_roadMarkType (§11.9, Table 47). Empty/absent -> None; unknown -> Other. static RoadMarkType road_mark_type_from_string(std::string_view name) { - if (name == "none" || name.empty()) + if (name.empty()) { return RoadMarkType::None; - if (name == "solid") - return RoadMarkType::Solid; - if (name == "broken") - return RoadMarkType::Broken; - if (name == "solid solid") - return RoadMarkType::SolidSolid; - if (name == "solid broken") - return RoadMarkType::SolidBroken; - if (name == "broken solid") - return RoadMarkType::BrokenSolid; - if (name == "broken broken") - return RoadMarkType::BrokenBroken; - return RoadMarkType::Other; + } + return xodr_names::value_of(xodr_names::kRoadMarkType, name).value_or(RoadMarkType::Other); } /// e_roadMarkColor (§11.9, Table 48). Empty/absent -> Standard; unknown -> /// Other with a diagnostic at the call site (never dropped). static RoadMarkColor road_mark_color_from_string(std::string_view name) { - if (name == "standard" || name.empty()) + if (name.empty()) { return RoadMarkColor::Standard; - if (name == "white") - return RoadMarkColor::White; - if (name == "yellow") - return RoadMarkColor::Yellow; - if (name == "red") - return RoadMarkColor::Red; - if (name == "blue") - return RoadMarkColor::Blue; - if (name == "green") - return RoadMarkColor::Green; - if (name == "orange") - return RoadMarkColor::Orange; - return RoadMarkColor::Other; + } + return xodr_names::value_of(xodr_names::kRoadMarkColor, name).value_or(RoadMarkColor::Other); } /// e_trafficRule (§10.2 Table 23). Empty/absent -> RHT, which the spec @@ -1295,46 +1238,31 @@ class Parser { /// spelling -> nullopt so the caller can default to RHT AND warn (never /// dropped). The verbatim spelling is kept by the caller either way. static std::optional traffic_rule_from_string(std::string_view name) { - if (name == "RHT" || name.empty()) + if (name.empty()) { return TrafficRule::RightHandTraffic; - if (name == "LHT") - return TrafficRule::LeftHandTraffic; - return std::nullopt; + } + return xodr_names::value_of(xodr_names::kTrafficRule, name); } /// e_lane_direction (1.8.1 Annex A.3.10 Table 173 / 1.9.0 Annex A.3.11 /// Table 180). Empty/absent -> Standard; an unknown spelling -> nullopt so /// the caller can default to Standard AND warn (never dropped). static std::optional lane_direction_from_string(std::string_view name) { - if (name == "standard" || name.empty()) + if (name.empty()) { return LaneDirection::Standard; - if (name == "reversed") - return LaneDirection::Reversed; - if (name == "both") - return LaneDirection::Both; - return std::nullopt; + } + return xodr_names::value_of(xodr_names::kLaneDirection, name); } // --- objects (OpenDRIVE §13) ---------------------------------------------- + /// e_objectType (§13.2, Table 92). Empty/absent -> None; an unknown spelling + /// -> Other, and it survives verbatim in `Object::type_str`. static ObjectType object_type_from_string(std::string_view name) { - if (name == "crosswalk") - return ObjectType::Crosswalk; - if (name == "tree") - return ObjectType::Tree; - if (name == "vegetation") - return ObjectType::Vegetation; - if (name == "pole") - return ObjectType::Pole; - if (name == "barrier") - return ObjectType::Barrier; - if (name == "building") - return ObjectType::Building; - if (name == "obstacle") - return ObjectType::Obstacle; - if (name == "none" || name.empty()) + if (name.empty()) { return ObjectType::None; - return ObjectType::Other; // spelling survives in Object::type_str + } + return xodr_names::value_of(xodr_names::kObjectType, name).value_or(ObjectType::Other); } void @@ -1448,18 +1376,14 @@ class Parser { rules::kObjectOrientation); } const std::string_view orientation_value = orientation.value(); - if (orientation_value == "+") { - object.orientation = ObjectOrientation::Plus; - } else if (orientation_value == "-") { - object.orientation = ObjectOrientation::Minus; - } else { - if (!orientation_value.empty() && orientation_value != "none") { - diag(Severity::Warning, - location, - fmt::format("unknown orientation '{}' mapped to 'none'", orientation_value)); - } - object.orientation = ObjectOrientation::None; + const auto object_orientation = + xodr_names::value_of(xodr_names::kObjectOrientation, orientation_value); + if (!object_orientation.has_value() && !orientation_value.empty()) { + diag(Severity::Warning, + location, + fmt::format("unknown orientation '{}' mapped to 'none'", orientation_value)); } + object.orientation = object_orientation.value_or(ObjectOrientation::None); // Set by the rm:stopline branch of the child loop below: when present this // object is absorbed into a junction record instead of the arena. @@ -2124,18 +2048,14 @@ class Parser { rules::kObjectOrientation); } const std::string_view orientation_value = orientation.value(); - if (orientation_value == "+") { - signal.orientation = ObjectOrientation::Plus; - } else if (orientation_value == "-") { - signal.orientation = ObjectOrientation::Minus; - } else { - if (!orientation_value.empty() && orientation_value != "none") { - diag(Severity::Warning, - location, - fmt::format("unknown orientation '{}' mapped to 'none'", orientation_value)); - } - signal.orientation = ObjectOrientation::None; + const auto signal_orientation = + xodr_names::value_of(xodr_names::kObjectOrientation, orientation_value); + if (!signal_orientation.has_value() && !orientation_value.empty()) { + diag(Severity::Warning, + location, + fmt::format("unknown orientation '{}' mapped to 'none'", orientation_value)); } + signal.orientation = signal_orientation.value_or(ObjectOrientation::None); signal.type = node.attribute("type").value(); signal.subtype = node.attribute("subtype").value(); diff --git a/core/src/xodr/writer.cpp b/core/src/xodr/writer.cpp index 8445c1fd..4a2e4922 100644 --- a/core/src/xodr/writer.cpp +++ b/core/src/xodr/writer.cpp @@ -43,20 +43,24 @@ #include #include +#include "../xml/xml_common.hpp" +#include "enum_names.hpp" #include "junction_export.hpp" namespace roadmaker { namespace { -/// Shortest-precision round-trippable formatting; locale-independent. -std::string num(double value) { - std::string text = fmt::format("{}", value); - return text == "-0" ? "0" : text; -} +using xml_common::num; +using xml_common::set_num; +using xml_common::set_optional_num; -void set_num(pugi::xml_node node, const char* name, double value) { - node.append_attribute(name).set_value(num(value).c_str()); +/// Appends a preserved fragment verbatim. NOT the OpenSCENARIO writer's +/// `append_fragment` despite the shared name: that one passes +/// `pugi::parse_fragment`, this one takes pugixml's defaults, and the two are +/// not interchangeable — which is why they are not shared (#563). +void append_fragment(pugi::xml_node parent, const std::string& fragment) { + parent.append_buffer(fragment.data(), fragment.size()); } /// The sidecar name a field is referenced by and written to (p5-s2, #232). @@ -97,125 +101,34 @@ char state_char(SignalState state) { return 'r'; } +// The spellings live in ONE table per enum, shared with the reader +// (xodr/enum_names.hpp) so the two directions cannot drift apart (#476, #563). +// What each wrapper still owns is the FALLBACK for a value the format cannot +// spell faithfully — always an `*::Other` parsed from an exotic input, whose +// real spelling the Preserved tier is holding in the matching `*_str` field. + const char* lane_type_name(LaneType type) { - switch (type) { - case LaneType::Driving: - return "driving"; - case LaneType::Stop: - return "stop"; - case LaneType::Shoulder: - return "shoulder"; - case LaneType::Biking: - return "biking"; - case LaneType::Sidewalk: - return "sidewalk"; - case LaneType::Border: - return "border"; - case LaneType::Restricted: - return "restricted"; - case LaneType::Parking: - return "parking"; - case LaneType::Median: - return "median"; - case LaneType::Curb: - return "curb"; - case LaneType::None: - return "none"; - case LaneType::Other: - return "none"; // parsed-as-other exotic types have no faithful name - } - return "none"; + return xodr_names::name_of(xodr_names::kLaneType, type, "none"); } const char* lane_direction_name(LaneDirection direction) { - switch (direction) { - case LaneDirection::Standard: - return "standard"; - case LaneDirection::Reversed: - return "reversed"; - case LaneDirection::Both: - return "both"; - } - return "standard"; + return xodr_names::name_of(xodr_names::kLaneDirection, direction, "standard"); } const char* object_type_name(ObjectType type) { - switch (type) { - case ObjectType::Crosswalk: - return "crosswalk"; - case ObjectType::Tree: - return "tree"; - case ObjectType::Vegetation: - return "vegetation"; - case ObjectType::Pole: - return "pole"; - case ObjectType::Barrier: - return "barrier"; - case ObjectType::Building: - return "building"; - case ObjectType::Obstacle: - return "obstacle"; - case ObjectType::None: - case ObjectType::Other: // Other always carries its spelling in type_str - return "none"; - } - return "none"; + return xodr_names::name_of(xodr_names::kObjectType, type, "none"); } const char* orientation_name(ObjectOrientation orientation) { - switch (orientation) { - case ObjectOrientation::Plus: - return "+"; - case ObjectOrientation::Minus: - return "-"; - case ObjectOrientation::None: - return "none"; - } - return "none"; + return xodr_names::name_of(xodr_names::kObjectOrientation, orientation, "none"); } const char* road_mark_name(RoadMarkType type) { - switch (type) { - case RoadMarkType::None: - return "none"; - case RoadMarkType::Solid: - return "solid"; - case RoadMarkType::Broken: - return "broken"; - case RoadMarkType::SolidSolid: - return "solid solid"; - case RoadMarkType::SolidBroken: - return "solid broken"; - case RoadMarkType::BrokenSolid: - return "broken solid"; - case RoadMarkType::BrokenBroken: - return "broken broken"; - case RoadMarkType::Other: - return "solid"; - } - return "none"; + return xodr_names::name_of(xodr_names::kRoadMarkType, type, "solid"); } const char* road_mark_color_name(RoadMarkColor color) { - switch (color) { - case RoadMarkColor::Standard: - return "standard"; - case RoadMarkColor::White: - return "white"; - case RoadMarkColor::Yellow: - return "yellow"; - case RoadMarkColor::Red: - return "red"; - case RoadMarkColor::Blue: - return "blue"; - case RoadMarkColor::Green: - return "green"; - case RoadMarkColor::Orange: - return "orange"; - case RoadMarkColor::Other: - return "standard"; // parsed-as-other exotic colors have no faithful name - } - return "standard"; + return xodr_names::name_of(xodr_names::kRoadMarkColor, color, "standard"); } /// Structural defects the writer refuses to serialize. Findings are @@ -499,26 +412,19 @@ void write_lane(pugi::xml_node side, const Lane& lane) { mark_node.append_attribute(name.c_str()).set_value(value.c_str()); } for (const std::string& fragment : mark.preserved.children) { - // append_fragment is defined below this function, so inlined here — the - // same accommodation write_lane already makes for its own preserved tier. - mark_node.append_buffer(fragment.data(), fragment.size()); + append_fragment(mark_node, fragment); } } // records (§11.8.2) — XSD sequence puts them after // and before the g_additionalData (speed/access/…/userData) group. Canonical // attr order sOffset, friction?, roughness?, surface?, then preserved attrs; // optionals omit when unset so a foreign file that lacked @friction stays - // byte-identical. (set_optional_num/append_fragment are defined below this - // function, so the optional writes are inlined here.) + // byte-identical. for (const LaneMaterial& material : lane.materials) { pugi::xml_node material_node = lane_node.append_child("material"); set_num(material_node, "sOffset", material.s_offset); - if (material.friction.has_value()) { - set_num(material_node, "friction", *material.friction); - } - if (material.roughness.has_value()) { - set_num(material_node, "roughness", *material.roughness); - } + set_optional_num(material_node, "friction", material.friction); + set_optional_num(material_node, "roughness", material.roughness); if (material.surface.has_value()) { material_node.append_attribute("surface").set_value(material.surface->c_str()); } @@ -531,13 +437,7 @@ void write_lane(pugi::xml_node side, const Lane& lane) { // preceded re-canonicalizes to this order — the accepted // limitation shared by every modeled element. for (const std::string& fragment : lane.preserved.children) { - lane_node.append_buffer(fragment.data(), fragment.size()); - } -} - -void set_optional_num(pugi::xml_node node, const char* name, std::optional value) { - if (value.has_value()) { - set_num(node, name, *value); + append_fragment(lane_node, fragment); } } @@ -574,10 +474,6 @@ void write_repeat(pugi::xml_node object_node, } } -void append_fragment(pugi::xml_node parent, const std::string& fragment) { - parent.append_buffer(fragment.data(), fragment.size()); -} - /// One line (§13.8, Table 99). Canonical attribute order is fixed so /// the writer output is idempotent; @side/@weight/@width/@zOffset omit when /// unset. cornerReferences (§13.8.1.3) follow, then any preserved unknowns. diff --git a/core/tests/CMakeLists.txt b/core/tests/CMakeLists.txt index 05d36f43..94bbc6e2 100644 --- a/core/tests/CMakeLists.txt +++ b/core/tests/CMakeLists.txt @@ -51,6 +51,7 @@ add_executable(roadmaker_core_tests test_gltf.cpp test_export_preview.cpp test_round_trip.cpp + test_enum_names.cpp test_corner_persistence.cpp test_stopline_persistence.cpp test_junction_lock_persistence.cpp @@ -236,3 +237,4 @@ target_link_libraries(roadmaker_scale_bench PRIVATE target_include_directories(roadmaker_scale_bench PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/scale ${CMAKE_SOURCE_DIR}/core/src) + diff --git a/core/tests/test_enum_names.cpp b/core/tests/test_enum_names.cpp new file mode 100644 index 00000000..9a8f2ee2 --- /dev/null +++ b/core/tests/test_enum_names.cpp @@ -0,0 +1,301 @@ +/* + * Copyright 2026 Robomous + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// The independent oracle for core/src/xodr/enum_names.hpp (#563). +// +// ★ THE OBVIOUS TEST IS WORTHLESS HERE, WHICH IS WHY THIS SPELLS IT ALL OUT. +// +// `value_of(name_of(v)) == v` over every value is one loop and total coverage, +// and it is a TAUTOLOGY: one table serves both directions, so any *consistent* +// relabelling satisfies it. Measured, not assumed — a differential probe +// re-emitting all 143 tracked .xodr/.xosc fixtures reported ZERO byte +// differences with `{Solid,"broken"}, {Broken,"solid"}` compiled in, for two +// compounding reasons: +// +// 1. A round trip is invariant under a consistent permutation. Read "+" as +// Minus, write Minus as "+": identical bytes, every object facing backwards. +// 2. The preserved tier shields the writer. `Lane/RoadMark/Object::type_str` +// hold the verbatim spelling for every PARSED element (the #476 fix), so +// re-emitting a parsed file never consults these tables at all. +// +// The tables are reached by networks the editor, the bindings and the importers +// BUILD — which no fixture round trip exercises. So the oracle must come from +// outside the table: the literals below are transcribed from the ASAM OpenDRIVE +// 1.9.0 tables cited per case, not derived from the code. + +#include "roadmaker/road/lane.hpp" +#include "roadmaker/road/object.hpp" +#include "roadmaker/road/traffic_rule.hpp" + +#include + +#include +#include + +#include "xodr/enum_names.hpp" + +namespace { + +using namespace roadmaker; +using roadmaker::xodr_names::name_of; +using roadmaker::xodr_names::value_of; + +// --- what the WRITER emits (§11.7 Table 43) --------------------------------- + +TEST(XodrEnumNames, LaneTypeSpellings) { + const auto name = [](LaneType t) { + return std::string(name_of(xodr_names::kLaneType, t, "none")); + }; + EXPECT_EQ(name(LaneType::Driving), "driving"); + EXPECT_EQ(name(LaneType::Stop), "stop"); + EXPECT_EQ(name(LaneType::Shoulder), "shoulder"); + EXPECT_EQ(name(LaneType::Biking), "biking"); + EXPECT_EQ(name(LaneType::Sidewalk), "sidewalk"); + EXPECT_EQ(name(LaneType::Border), "border"); + EXPECT_EQ(name(LaneType::Restricted), "restricted"); + EXPECT_EQ(name(LaneType::Parking), "parking"); + EXPECT_EQ(name(LaneType::Median), "median"); + EXPECT_EQ(name(LaneType::Curb), "curb"); + EXPECT_EQ(name(LaneType::None), "none"); + // Other has no row: it is the unmodeled bucket, and the fallback stands in. + EXPECT_EQ(name(LaneType::Other), "none"); + // ...and NEVER the deprecated alias, which is read-only. Emitting "walking" + // for a sidewalk is the exact corruption #476 was filed for. + EXPECT_NE(name(LaneType::Sidewalk), "walking"); +} + +TEST(XodrEnumNames, LaneTypeParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kLaneType, s); }; + EXPECT_EQ(value("driving"), LaneType::Driving); + EXPECT_EQ(value("stop"), LaneType::Stop); + EXPECT_EQ(value("shoulder"), LaneType::Shoulder); + EXPECT_EQ(value("biking"), LaneType::Biking); + EXPECT_EQ(value("sidewalk"), LaneType::Sidewalk); + EXPECT_EQ(value("border"), LaneType::Border); + EXPECT_EQ(value("restricted"), LaneType::Restricted); + EXPECT_EQ(value("parking"), LaneType::Parking); + EXPECT_EQ(value("median"), LaneType::Median); + EXPECT_EQ(value("curb"), LaneType::Curb); + EXPECT_EQ(value("none"), LaneType::None); + // The pre-1.6 alias still parses (§11.7) — accepted, never emitted. + EXPECT_EQ(value("walking"), LaneType::Sidewalk); + // No guessing: unknown and empty are the CALLER's policy, not the table's. + EXPECT_FALSE(value("bus").has_value()); + EXPECT_FALSE(value("").has_value()); + EXPECT_FALSE(value("Driving").has_value()) << "e_laneType is case-sensitive"; +} + +// --- e_roadMarkType (§11.9, Table 47) --------------------------------------- + +TEST(XodrEnumNames, RoadMarkTypeSpellings) { + const auto name = [](RoadMarkType t) { + return std::string(name_of(xodr_names::kRoadMarkType, t, "solid")); + }; + EXPECT_EQ(name(RoadMarkType::None), "none"); + EXPECT_EQ(name(RoadMarkType::Solid), "solid"); + EXPECT_EQ(name(RoadMarkType::Broken), "broken"); + // The multi-token spellings carry exactly one ASCII space — it is part of the + // enumerator, and a tab or a double space is a different (invalid) token. + EXPECT_EQ(name(RoadMarkType::SolidSolid), "solid solid"); + EXPECT_EQ(name(RoadMarkType::SolidBroken), "solid broken"); + EXPECT_EQ(name(RoadMarkType::BrokenSolid), "broken solid"); + EXPECT_EQ(name(RoadMarkType::BrokenBroken), "broken broken"); + EXPECT_EQ(name(RoadMarkType::Other), "solid") << "the lossy stand-in, via the fallback"; +} + +TEST(XodrEnumNames, RoadMarkTypeParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kRoadMarkType, s); }; + EXPECT_EQ(value("none"), RoadMarkType::None); + EXPECT_EQ(value("solid"), RoadMarkType::Solid); + EXPECT_EQ(value("broken"), RoadMarkType::Broken); + EXPECT_EQ(value("solid solid"), RoadMarkType::SolidSolid); + EXPECT_EQ(value("solid broken"), RoadMarkType::SolidBroken); + EXPECT_EQ(value("broken solid"), RoadMarkType::BrokenSolid); + EXPECT_EQ(value("broken broken"), RoadMarkType::BrokenBroken); + // The five modelled-as-Other spellings from Table 47 must NOT resolve to a + // value — each has to reach the caller as "unknown" so it lands in Other with + // its text preserved. A row for any of these would repaint a kerb as a line. + for (const std::string_view exotic : {"curb", "grass", "botts dots", "edge", "custom"}) { + EXPECT_FALSE(value(exotic).has_value()) << exotic << " must stay unmodelled"; + } +} + +// --- e_roadMarkColor (§11.9, Table 48) -------------------------------------- + +TEST(XodrEnumNames, RoadMarkColorSpellings) { + const auto name = [](RoadMarkColor c) { + return std::string(name_of(xodr_names::kRoadMarkColor, c, "standard")); + }; + EXPECT_EQ(name(RoadMarkColor::Standard), "standard"); + EXPECT_EQ(name(RoadMarkColor::White), "white"); + EXPECT_EQ(name(RoadMarkColor::Yellow), "yellow"); + EXPECT_EQ(name(RoadMarkColor::Red), "red"); + EXPECT_EQ(name(RoadMarkColor::Blue), "blue"); + EXPECT_EQ(name(RoadMarkColor::Green), "green"); + EXPECT_EQ(name(RoadMarkColor::Orange), "orange"); + EXPECT_EQ(name(RoadMarkColor::Other), "standard"); +} + +TEST(XodrEnumNames, RoadMarkColorParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kRoadMarkColor, s); }; + EXPECT_EQ(value("standard"), RoadMarkColor::Standard); + EXPECT_EQ(value("white"), RoadMarkColor::White); + EXPECT_EQ(value("yellow"), RoadMarkColor::Yellow); + EXPECT_EQ(value("red"), RoadMarkColor::Red); + EXPECT_EQ(value("blue"), RoadMarkColor::Blue); + EXPECT_EQ(value("green"), RoadMarkColor::Green); + EXPECT_EQ(value("orange"), RoadMarkColor::Orange); + EXPECT_FALSE(value("violet").has_value()); + EXPECT_FALSE(value("").has_value()); +} + +// --- e_lane_direction (§11.7, 1.8.0+) --------------------------------------- + +TEST(XodrEnumNames, LaneDirectionSpellings) { + const auto name = [](LaneDirection d) { + return std::string(name_of(xodr_names::kLaneDirection, d, "standard")); + }; + EXPECT_EQ(name(LaneDirection::Standard), "standard"); + EXPECT_EQ(name(LaneDirection::Reversed), "reversed"); + EXPECT_EQ(name(LaneDirection::Both), "both"); +} + +TEST(XodrEnumNames, LaneDirectionParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kLaneDirection, s); }; + EXPECT_EQ(value("standard"), LaneDirection::Standard); + EXPECT_EQ(value("reversed"), LaneDirection::Reversed); + EXPECT_EQ(value("both"), LaneDirection::Both); + EXPECT_FALSE(value("forward").has_value()); +} + +// --- e_objectType (§13.2, Table 92) ----------------------------------------- + +TEST(XodrEnumNames, ObjectTypeSpellings) { + const auto name = [](ObjectType t) { + return std::string(name_of(xodr_names::kObjectType, t, "none")); + }; + EXPECT_EQ(name(ObjectType::Crosswalk), "crosswalk"); + EXPECT_EQ(name(ObjectType::Tree), "tree"); + EXPECT_EQ(name(ObjectType::Vegetation), "vegetation"); + EXPECT_EQ(name(ObjectType::Pole), "pole"); + EXPECT_EQ(name(ObjectType::Barrier), "barrier"); + EXPECT_EQ(name(ObjectType::Building), "building"); + EXPECT_EQ(name(ObjectType::Obstacle), "obstacle"); + EXPECT_EQ(name(ObjectType::None), "none"); + EXPECT_EQ(name(ObjectType::Other), "none"); +} + +TEST(XodrEnumNames, ObjectTypeParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kObjectType, s); }; + EXPECT_EQ(value("crosswalk"), ObjectType::Crosswalk); + EXPECT_EQ(value("tree"), ObjectType::Tree); + EXPECT_EQ(value("vegetation"), ObjectType::Vegetation); + EXPECT_EQ(value("pole"), ObjectType::Pole); + EXPECT_EQ(value("barrier"), ObjectType::Barrier); + EXPECT_EQ(value("building"), ObjectType::Building); + EXPECT_EQ(value("obstacle"), ObjectType::Obstacle); + EXPECT_EQ(value("none"), ObjectType::None); + EXPECT_FALSE(value("streetLamp").has_value()) << "modelled as Other, text preserved"; +} + +// --- @orientation on and (§13) ---------------------------- + +TEST(XodrEnumNames, ObjectOrientationSpellings) { + const auto name = [](ObjectOrientation o) { + return std::string(name_of(xodr_names::kObjectOrientation, o, "none")); + }; + // Sign-inverting these is undetectable by any round trip — read "+" as Minus, + // write Minus as "+", identical bytes, every object facing backwards. This is + // the assertion that catches it. + EXPECT_EQ(name(ObjectOrientation::Plus), "+"); + EXPECT_EQ(name(ObjectOrientation::Minus), "-"); + EXPECT_EQ(name(ObjectOrientation::None), "none"); +} + +TEST(XodrEnumNames, ObjectOrientationParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kObjectOrientation, s); }; + EXPECT_EQ(value("+"), ObjectOrientation::Plus); + EXPECT_EQ(value("-"), ObjectOrientation::Minus); + EXPECT_EQ(value("none"), ObjectOrientation::None); + EXPECT_FALSE(value("").has_value()) << "absent is the reader's policy, not a row"; +} + +// --- e_trafficRule (§10.2, Table 23) ---------------------------------------- + +TEST(XodrEnumNames, TrafficRuleParsing) { + const auto value = [](std::string_view s) { return value_of(xodr_names::kTrafficRule, s); }; + EXPECT_EQ(value("RHT"), TrafficRule::RightHandTraffic); + EXPECT_EQ(value("LHT"), TrafficRule::LeftHandTraffic); + // Upper-case in the standard; the lower-case spelling is not a synonym. + EXPECT_FALSE(value("rht").has_value()); + EXPECT_FALSE(value("").has_value()); +} + +// --- coverage: a new enumerator must not slip through unnamed --------------- +// +// A `switch` with no `default` is warned on (and -Werror'd) when an enumerator +// is added, so these force the author of a new value to visit this file. The +// assertion is that every value the switch enumerates has a real row — Other, +// which deliberately has none, is the single exception per enum. + +TEST(XodrEnumNames, EveryLaneTypeIsNamedExceptOther) { + for (const LaneType type : {LaneType::Driving, + LaneType::Stop, + LaneType::Shoulder, + LaneType::Biking, + LaneType::Sidewalk, + LaneType::Border, + LaneType::Restricted, + LaneType::Parking, + LaneType::Median, + LaneType::Curb, + LaneType::None}) { + const char* sentinel = "!!unnamed!!"; + EXPECT_STRNE(name_of(xodr_names::kLaneType, type, sentinel), sentinel) + << "LaneType value " << static_cast(type) << " has no row in kLaneType"; + } +} + +TEST(XodrEnumNames, EveryRoadMarkTypeIsNamedExceptOther) { + for (const RoadMarkType type : {RoadMarkType::None, + RoadMarkType::Solid, + RoadMarkType::Broken, + RoadMarkType::SolidSolid, + RoadMarkType::SolidBroken, + RoadMarkType::BrokenSolid, + RoadMarkType::BrokenBroken}) { + const char* sentinel = "!!unnamed!!"; + EXPECT_STRNE(name_of(xodr_names::kRoadMarkType, type, sentinel), sentinel) + << "RoadMarkType value " << static_cast(type) << " has no row"; + } +} + +TEST(XodrEnumNames, EveryObjectTypeIsNamedExceptOther) { + for (const ObjectType type : {ObjectType::None, + ObjectType::Crosswalk, + ObjectType::Tree, + ObjectType::Vegetation, + ObjectType::Pole, + ObjectType::Barrier, + ObjectType::Building, + ObjectType::Obstacle}) { + const char* sentinel = "!!unnamed!!"; + EXPECT_STRNE(name_of(xodr_names::kObjectType, type, sentinel), sentinel) + << "ObjectType value " << static_cast(type) << " has no row"; + } +} + +} // namespace diff --git a/editor/src/render/renderer.hpp b/editor/src/render/renderer.hpp index 17ae2a18..dd86c740 100644 --- a/editor/src/render/renderer.hpp +++ b/editor/src/render/renderer.hpp @@ -174,7 +174,6 @@ struct Environment { std::array ground_color{0.4F, 0.38F, 0.35F}; ///< hemisphere down float sun_intensity = 1.0F; float ambient = 0.35F; - bool procedural_sky = true; ///< false → sampled HDRI (later render polish) }; /// The daytime "Textured" render mode: hemisphere sky/ground + a warm sun. @@ -194,7 +193,6 @@ struct Environment { env.ground_color = {1.0F, 1.0F, 1.0F}; // ambient term, so no hemisphere tint env.sun_intensity = 0.65F; env.ambient = 0.35F; - env.procedural_sky = false; return env; } diff --git a/editor/tests/test_scene_builder.cpp b/editor/tests/test_scene_builder.cpp index 423cf57e..c1e6528e 100644 --- a/editor/tests/test_scene_builder.cpp +++ b/editor/tests/test_scene_builder.cpp @@ -294,7 +294,6 @@ TEST(Lighting, SoberPresetReproducesFlatM2Shading) { EXPECT_FLOAT_EQ(sober.ambient, 0.35F); EXPECT_FLOAT_EQ(sober.sun_color[0], 1.0F); EXPECT_FLOAT_EQ(sober.sun_intensity, 0.65F); - EXPECT_FALSE(sober.procedural_sky); // Same sun direction the pre-Environment shader hardcoded. EXPECT_FLOAT_EQ(sober.sun_dir[0], 0.35F); EXPECT_FLOAT_EQ(sober.sun_dir[1], 0.25F); @@ -305,7 +304,6 @@ TEST(Lighting, TexturedPresetIsTheDaytimeDefault) { const Environment textured = textured_lighting(); const Environment defaults; // struct defaults == the textured preset EXPECT_EQ(textured.sky_color, defaults.sky_color); - EXPECT_TRUE(textured.procedural_sky); // Textured sky is a tinted hemisphere, unlike Sober's flat white. EXPECT_NE(textured.sky_color, sober_lighting().sky_color); }