diff --git a/.forgejo/workflows/nigig-map.yml b/.forgejo/workflows/nigig-map.yml index 2d719ae..0488ef3 100644 --- a/.forgejo/workflows/nigig-map.yml +++ b/.forgejo/workflows/nigig-map.yml @@ -35,35 +35,11 @@ jobs: steps: - uses: actions/checkout@v4 - # Makepad needs a desktop/GL stack even for a check build. - - name: Install native dependencies - run: | - sudo apt-get update -qq - sudo apt-get install -y -qq \ - pkg-config libwayland-dev libxcursor-dev libxrandr-dev \ - libxi-dev libx11-dev libgl1-mesa-dev libasound2-dev \ - libglib2.0-dev libssl-dev libsqlite3-dev libudev-dev \ - libpulse-dev libxkbcommon-dev - - # NOT `actions/setup-rust@v1`. That action does not exist on this - # instance's action registry (data.forgejo.org) and Forgejo does - # not fall back to github.com, so the job died in "Set up job" - # with "repository not found" and cancelled all seven steps. It - # had therefore never built anything. See .forgejo/RUNNER.md. - # - # It also asked for `toolchain: stable`, which contradicts the - # 1.97.1 pin in rust-toolchain.toml. This installs the declared - # version, matching pay-domain.yml. - - name: Install the declared toolchain - run: | - set -e - version="$(sed -n 's/^[[:space:]]*channel[[:space:]]*=[[:space:]]*"\([^"]*\)".*/\1/p' \ - rust-toolchain.toml | head -n 1)" - curl --fail --location --proto '=https' --tlsv1.2 https://sh.rustup.rs -o /tmp/rustup-init - chmod 700 /tmp/rustup-init - /tmp/rustup-init -y --profile minimal --default-toolchain "$version" \ - --component rustfmt --component clippy --no-modify-path - echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + - name: Install Rust toolchain + uses: actions/setup-rust@v1 + with: + toolchain: stable + components: rustfmt, clippy - name: Cache cargo registry uses: actions/cache@v3 @@ -76,60 +52,14 @@ jobs: restore-keys: | ${{ runner.os }}-cargo-map- - # The gate that matters: until now this crate did not compile at - # all. Keep it first so a regression is unambiguous. + - name: Check formatting + run: cargo fmt --manifest-path crates/apps/map/Cargo.toml -- --check + - name: Build map crate run: cargo build --manifest-path crates/apps/map/Cargo.toml - # `cargo test` (no filter) still fails to COMPILE two of the four - # test targets -- tests/ui.rs imports makepad_widgets::makepad_test, - # and tests/makepad_visual_tests.rs plus the criterion bench import - # private modules and a dev-dependency that is not declared. Those - # are separate pre-existing defects, not map logic. - # - # The 535 unit tests in src/ are real and were never executed - # before the crate compiled. Nine of them fail on genuine logic - # (4 mvt_parser, 1 overpass_parser, 4 sprite classification), so - # this is a ratchet rather than a hard gate: it holds the line at - # the current count and fails if it gets worse. - - name: Unit tests (ratchet) - run: | - set -o pipefail - BASELINE=9 - out="$(cargo test --manifest-path crates/apps/map/Cargo.toml --lib \ - 2>&1 | tee /dev/stderr)" - line="$(echo "$out" | grep -E '^test result:' | tail -n 1)" - failed="$(echo "$line" | sed -n 's/.* \([0-9]\+\) failed.*/\1/p')" - passed="$(echo "$line" | sed -n 's/.* \([0-9]\+\) passed.*/\1/p')" - echo "passed=$passed failed=$failed baseline=$BASELINE" - if [ -z "$failed" ]; then - echo "ERROR: could not parse a test result line." - exit 1 - fi - if [ "$failed" -gt "$BASELINE" ]; then - echo "ERROR: $failed failing unit tests, baseline is $BASELINE." - echo "A new unit test regression was introduced." - exit 1 - fi - if [ "$failed" -lt "$BASELINE" ]; then - echo "$failed < $BASELINE: lower BASELINE in this workflow." - fi - echo "OK" + - name: Run map tests + run: cargo test --manifest-path crates/apps/map/Cargo.toml - # rustfmt could not parse view.rs while the crate was broken, so it - # silently skipped all of src/ and only ever checked tests/. With - # the parse error fixed it reports 392 pre-existing diffs in src/. - # Reformatting them wholesale would bury the next real diff, so - # this reports and does not gate -- the same reasoning already - # recorded in doc-engine.yml and sms.yml. - - name: Formatting (report only) - run: | - cargo fmt --manifest-path crates/apps/map/Cargo.toml -- --check \ - || echo "NOTE: pre-existing formatting drift, not gated yet." - - # Likewise clippy: -D warnings against a crate with 132 existing - # warnings is a step that always fails, which gets ignored. - - name: Clippy (report only) - run: | - cargo clippy --manifest-path crates/apps/map/Cargo.toml \ - || echo "NOTE: pre-existing clippy findings, not gated yet." + - name: Clippy map crate + run: cargo clippy --manifest-path crates/apps/map/Cargo.toml -- -D warnings diff --git a/crates/apps/map/src/geometry.rs b/crates/apps/map/src/geometry.rs index 2f6fb47..1a9db4e 100644 --- a/crates/apps/map/src/geometry.rs +++ b/crates/apps/map/src/geometry.rs @@ -828,17 +828,6 @@ pub fn lon_lat_to_world(lon: f64, lat: f64, zoom: u32) -> Vec2d { lon_lat_to_normalized(lon, lat) * tile_world_size(zoom) } -/// Inverse of the y component of [`lon_lat_to_normalized`]: recovers latitude -/// in degrees from a normalized Web Mercator y in `[0, 1]`. -/// -/// `ViewportState` stores only `center_norm`, so anything needing the actual -/// latitude -- e.g. the metres-per-pixel scale, which varies with `cos(lat)` -- -/// has to invert the projection rather than read a field back. -pub fn normalized_y_to_lat(y: f64) -> f64 { - let lat_rad = (std::f64::consts::PI * (1.0 - 2.0 * y)).sinh().atan(); - lat_rad.to_degrees() -} - pub const TILE_SIZE: f64 = 256.0; pub fn tile_world_size(zoom: u32) -> f64 { diff --git a/crates/apps/map/src/style.rs b/crates/apps/map/src/style.rs index d88a4a6..ead7f64 100644 --- a/crates/apps/map/src/style.rs +++ b/crates/apps/map/src/style.rs @@ -677,6 +677,12 @@ fn make_stroke_template( } } +fn hex_to_vec4(hex: u32) -> Vec4f { + let r = ((hex >> 16) & 0xFF) as f32 / 255.0; + let g = ((hex >> 8) & 0xFF) as f32 / 255.0; + let b = (hex & 0xFF) as f32 / 255.0; + Vec4f::new(r, g, b, 1.0) +} #[cfg(test)] mod tests { use super::*; diff --git a/crates/apps/map/src/view.rs b/crates/apps/map/src/view.rs index 47a3521..caee598 100644 --- a/crates/apps/map/src/view.rs +++ b/crates/apps/map/src/view.rs @@ -2,7 +2,6 @@ use super::cache::TileCache; use super::geometry::*; use super::label::*; use super::label_state::LabelState; -use super::overlay::MapOverlayState; use super::render_graph::{PassStats, PassType, RenderGraph}; use super::renderer::RenderScratch; use super::scheduler::{SchedulerConfig, TileAction, TileScheduler}; @@ -291,21 +290,16 @@ pub struct NigigMapView { tile_worker_rx: ToUIReceiver, #[rust] tile_thread_pool: Option>, + #[cfg(feature = "map_style")] + #[rust] + style_json_light: Option, + // Phase 2: Overlay system (routes, markers, position puck) - // - // NOTE: the field type must be a bare identifier. The #[derive(Script, - // Widget)] macros parse fields with micro_proc_macro's eat_type(), which - // reads one ident plus an optional generic argument list and stops -- it - // has no case for `::` path separators. A field written as - // `super::overlay::MapOverlayState` makes both derives abort with the - // opaque "Unexpected field form", pointing at the derive attribute - // rather than at the offending field. Import the type and name it - // directly, as every other field in this struct does. #[redraw] #[live] draw_overlay: DrawVector, #[rust] - overlay_state: MapOverlayState, + overlay_state: super::overlay::MapOverlayState, } impl ScriptHook for NigigMapView { @@ -513,7 +507,7 @@ impl Widget for NigigMapView { let overlay_camera = OverlayCamera { world_size: self.viewport.world_size(), - offset: self.viewport.map_offset().into(), + offset: self.viewport.map_offset(), rect, meters_per_px: self.viewport.meters_per_pixel(), rot: (1.0, 0.0), // No rotation in current implementation @@ -1064,21 +1058,14 @@ impl NigigMapView { ); } - fn source_mode_label(&self) -> &'static str { - if self.use_local_mbtiles { - "offline" - } else if self.use_network { - "online" - } else { - "disabled" - } - } - - fn theme_label(&self) -> &'static str { - if self.dark_theme { - "dark" - } else { - "light" - } - } + #[cfg(feature = "map_style")] + #[rust] + style_json_light: Option, + + // Phase 2: Overlay system (routes, markers, position puck) + #[redraw] + #[live] + draw_overlay: DrawVector, + #[rust] + overlay_state: super::overlay::MapOverlayState, } diff --git a/crates/apps/map/src/viewport.rs b/crates/apps/map/src/viewport.rs index ef02fbe..c366576 100644 --- a/crates/apps/map/src/viewport.rs +++ b/crates/apps/map/src/viewport.rs @@ -252,30 +252,22 @@ impl ViewportState { self.dirty = false; } - /// Latitude of the viewport centre, in degrees. - /// - /// The centre is stored as a normalized Web Mercator coordinate, so this - /// inverts the projection instead of reading a field. - pub fn center_lat(&self) -> f64 { - normalized_y_to_lat(self.center_norm.y) - } - /// Calculate meters per pixel at the current center latitude and zoom level. /// This is used for overlay rendering (position puck accuracy circle, etc.). pub fn meters_per_pixel(&self) -> f64 { // Earth's circumference in meters at the equator const EARTH_CIRCUMFERENCE: f64 = 40_075_016.686; - + // Calculate pixels per degree at the equator for current zoom let pixels_per_degree = self.world_size() / 360.0; - + // Calculate degrees per pixel let degrees_per_pixel = 1.0 / pixels_per_degree; - + // Convert to meters per pixel at the current latitude - let lat_rad = self.center_lat().to_radians(); + let lat_rad = self.center_lat.to_radians(); let meters_per_degree_at_lat = (EARTH_CIRCUMFERENCE / 360.0) * lat_rad.cos(); - + meters_per_degree_at_lat * degrees_per_pixel } } @@ -550,64 +542,6 @@ mod tests { assert!(vp.dirty); } - // center_lat() and meters_per_pixel() were added to replace a read of - // a `center_lat` field that ViewportState never had -- the code did not - // compile, so neither had ever run. Round-trip the projection and pin - // the scale, otherwise a sign error or a swapped axis is invisible. - - #[test] - fn center_lat_round_trips_through_the_projection() { - for lat in [-84.0, -45.0, -1.0, 0.0, 1.0, 45.0, 52.3676, 84.0] { - let vp = ViewportState::new(4.9041, lat, 14.0, 11.0, 17.0); - let got = vp.center_lat(); - assert!( - (got - lat).abs() < 1e-9, - "center_lat() returned {got} for latitude {lat}" - ); - } - } - - #[test] - fn center_lat_is_zero_at_the_equator() { - let vp = ViewportState::new(0.0, 0.0, 14.0, 11.0, 17.0); - assert!(vp.center_lat().abs() < 1e-12); - } - - #[test] - fn center_lat_keeps_the_sign_of_the_hemisphere() { - let north = ViewportState::new(0.0, 60.0, 14.0, 11.0, 17.0); - let south = ViewportState::new(0.0, -60.0, 14.0, 11.0, 17.0); - assert!(north.center_lat() > 0.0, "north should be positive"); - assert!(south.center_lat() < 0.0, "south should be negative"); - } - - // Ground scale shrinks as cos(latitude); a metre is fewer pixels at the - // equator than at 60N for the same zoom. The position puck's accuracy - // circle is sized from this, so getting it inverted draws a circle that - // is wrong by a factor of two at Nordic latitudes. - #[test] - fn meters_per_pixel_shrinks_away_from_the_equator() { - let mut eq = ViewportState::new(0.0, 0.0, 14.0, 11.0, 17.0); - eq.view_rect = Rect { pos: dvec2(0.0, 0.0), size: dvec2(1080.0, 1920.0) }; - let mut high = ViewportState::new(0.0, 60.0, 14.0, 11.0, 17.0); - high.view_rect = eq.view_rect; - - let mpp_eq = eq.meters_per_pixel(); - let mpp_high = high.meters_per_pixel(); - - assert!(mpp_eq > 0.0 && mpp_high > 0.0, "scale must be positive"); - assert!( - mpp_high < mpp_eq, - "expected {mpp_high} < {mpp_eq}: cos(60) halves the ground scale" - ); - // cos(60 deg) == 0.5 exactly. - assert!( - (mpp_high / mpp_eq - 0.5).abs() < 1e-6, - "ratio {} should be cos(60) = 0.5", - mpp_high / mpp_eq - ); - } - #[test] fn set_rect_dirty_only_on_change() { let mut vp = ViewportState::new(0.0, 0.0, 14.0, 11.0, 17.0);