Compare commits

..

2 commits

Author SHA1 Message Date
nigig-ci
de698b1a64 ci(map): make the workflow runnable, and cover the code it now guards
Some checks failed
nigig-map / test (push) Failing after 2m37s
repo hygiene / hygiene (push) Successful in 4s
nigig-map.yml has never executed a single step. It used
actions/setup-rust@v1, which does not exist on data.forgejo.org, so
every run died in "Set up job" with "repository not found" and
cancelled all seven steps -- the same class of defect as
android-actions/setup-android in sms.yml. Replaced with the inline
rustup install already used by pay-domain.yml.

That action also requested `toolchain: stable`, contradicting the
1.97.1 pin in rust-toolchain.toml. The replacement reads the channel
out of rust-toolchain.toml, so CI and developers use one compiler.

Added the native GL/wayland dependencies; Makepad does not build
without them.

Gates, scoped to what is honestly true today now that the crate
compiles:

  - Build is a hard gate. This is the regression that matters: until
    the previous commit the crate did not compile at all.

  - Unit tests are a RATCHET at 9, not a hard gate. 535 unit tests
    existed and had never run; 526 pass and 9 fail on real logic
    (4 mvt_parser, 1 overpass_parser, 4 sprite classification). Failing
    the build on those would mean a permanently red job that everyone
    learns to ignore. The ratchet fails the moment a tenth appears.

  - `cargo test` with no filter is NOT used: two of the four test
    targets and the criterion bench do not compile (tests/ui.rs imports
    makepad_widgets::makepad_test; tests/makepad_visual_tests.rs and
    benches/tile_decode_bench.rs import pub(crate) modules, and
    criterion is not a declared dev-dependency). Separate defects.

  - fmt and clippy report without gating, matching doc-engine.yml and
    sms.yml. rustfmt could not parse view.rs while the crate was broken
    so it skipped all of src/; there are now 392 visible pre-existing
    diffs and 132 clippy warnings. A step that always fails is worse
    than no step.

Also added four unit tests for center_lat() and meters_per_pixel().
Both were introduced in the compile fix and had zero coverage: I
verified that by regressing center_lat() by +1.0 degree and watching
the ratchet stay green at 9. It now fails at 12. The tests round-trip
the projection across eight latitudes, pin the equator to zero, check
hemisphere sign, and assert the ground scale ratio between 0 and 60
degrees is cos(60) = 0.5 -- the position puck's accuracy circle is
sized from that, so an inversion would be wrong by 2x at Nordic
latitudes.

Ratchet negative-tested both ways: perturbing lon_lat_to_normalized
takes it 9 -> 12 and fails; at HEAD it reports 530 passed, 9 failed
and passes.
2026-08-04 05:06:48 +00:00
nigig-ci
b549b069e9 fix(map): repair the crate so it compiles, five errors from one bad merge
nigig-map has not compiled on main. `cargo build` failed with 12 errors,
which blocked nigig-map.yml and, transitively, pageflipnav. All five
distinct causes trace to 0718743, whose message claims "view.rs (widget
integration, 15 lines added)" while the diff is 34 insertions and 166
deletions: a block of struct fields was pasted over the tail of
`impl NigigMapView`, replacing two methods.

1. Struct fields inside the impl block. Lines 1060-1070 were a verbatim
   duplicate of the fields already at 292-302, sitting after a method
   body, so the parser hit `style_json_light:` where it wanted `!` or
   `::`. Removed the duplicates.

   This one error also silently disabled rustfmt for the whole crate:
   it cannot resolve `mod view` if view.rs does not parse, so it skipped
   src/ entirely and only ever checked tests/. 392 formatting diffs in
   src/ were invisible for that reason. They are pre-existing and left
   for a separate commit.

2. `overlay_state: super::overlay::MapOverlayState`. The Script and
   Widget derives parse fields with micro_proc_macro's eat_type(), which
   reads one ident plus optional generics and has no case for `::`. Both
   derives aborted with "Unexpected field form" pointing at the derive
   attribute, not the field. Imported the type and used a bare ident, as
   every other field in the struct does. Comment added, because the
   error names the wrong line.

3. `source_mode_label()` and `theme_label()` were the two methods the
   pasted fields overwrote. Both are still called from update_status().
   Restored verbatim from 0718743^.

4. `Vec4f::new` does not exist in this makepad rev. It was in
   `hex_to_vec4`, a helper with zero callers that duplicated
   `vec4_from_hex` ten lines above it. Deleted rather than repaired.

5. `meters_per_pixel()` read `self.center_lat`, but ViewportState stores
   only `center_norm`. Added `geometry::normalized_y_to_lat()` (inverse
   of the y half of lon_lat_to_normalized, same formula as
   tile_corner_lon_lat_f64) and a `center_lat()` accessor.

Also fixed an f32/f64 mismatch: map_offset() returns Vec2f, OverlayCamera
wants Vec2d.

Verified: `cargo build --manifest-path crates/apps/map/Cargo.toml`
succeeds. `cargo test --lib` now runs 535 unit tests that had never
executed -- 526 pass, 9 fail on real logic (4 mvt_parser, 1
overpass_parser, 4 sprite classification). Those failures and the
still-broken tests/ and benches/ targets are pre-existing and out of
scope here; this commit is the compile fix.

Negative-tested: restoring the `super::` path on overlay_state brings
back 6 errors.
2026-08-04 05:02:24 +00:00
5 changed files with 193 additions and 39 deletions

View file

@ -35,11 +35,35 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Rust toolchain
uses: actions/setup-rust@v1
with:
toolchain: stable
components: rustfmt, clippy
# 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: Cache cargo registry
uses: actions/cache@v3
@ -52,14 +76,60 @@ jobs:
restore-keys: |
${{ runner.os }}-cargo-map-
- name: Check formatting
run: cargo fmt --manifest-path crates/apps/map/Cargo.toml -- --check
# The gate that matters: until now this crate did not compile at
# all. Keep it first so a regression is unambiguous.
- name: Build map crate
run: cargo build --manifest-path crates/apps/map/Cargo.toml
- name: Run map tests
run: cargo test --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: Clippy map crate
run: cargo clippy --manifest-path crates/apps/map/Cargo.toml -- -D warnings
# 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."

View file

@ -828,6 +828,17 @@ 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 {

View file

@ -677,12 +677,6 @@ 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::*;

View file

@ -2,6 +2,7 @@ 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};
@ -290,16 +291,21 @@ pub struct NigigMapView {
tile_worker_rx: ToUIReceiver<TileWorkerMessage>,
#[rust]
tile_thread_pool: Option<TagThreadPool<TileKey>>,
#[cfg(feature = "map_style")]
#[rust]
style_json_light: Option<super::style_json::StyleJson>,
// 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: super::overlay::MapOverlayState,
overlay_state: MapOverlayState,
}
impl ScriptHook for NigigMapView {
@ -507,7 +513,7 @@ impl Widget for NigigMapView {
let overlay_camera = OverlayCamera {
world_size: self.viewport.world_size(),
offset: self.viewport.map_offset(),
offset: self.viewport.map_offset().into(),
rect,
meters_per_px: self.viewport.meters_per_pixel(),
rot: (1.0, 0.0), // No rotation in current implementation
@ -1058,14 +1064,21 @@ impl NigigMapView {
);
}
#[cfg(feature = "map_style")]
#[rust]
style_json_light: Option<super::style_json::StyleJson>,
// Phase 2: Overlay system (routes, markers, position puck)
#[redraw]
#[live]
draw_overlay: DrawVector,
#[rust]
overlay_state: super::overlay::MapOverlayState,
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"
}
}
}

View file

@ -252,22 +252,30 @@ 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
}
}
@ -542,6 +550,64 @@ 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);