From 4e7bcacc377e2ac3946cddea62a901ac233bc500 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Sat, 8 Aug 2026 00:04:59 -0400 Subject: [PATCH] Tile decode: don't pay for the layers the map never draws Profiling on the Lyra found vector-tile decode dominating everything else in the app by about two orders of magnitude: a median cached tile took 1236 ms of pure CPU, the largest 3.4 s, against 16 ms of SD read for a whole viewport. Decoded tiles are memoised for the session, so this is a first-pan cost -- but it runs on a GIL-bound worker thread, so it starves the render loop for those seconds rather than running beside it. Roughly 40% of that work was thrown away. A planet tile ships building, housenumber, poi and mountain_peak; _draw_tile looks up ten layer names and none of them is those. So decode_tile now takes the set of layers the caller actually wants, reads each layer's identity with a cheap header walk, and decodes the features only for the ones asked for. Unwanted layers are still returned, named and empty -- which matters, because the tile source decides whether a cache entry is real by whether the bytes parsed into layers, and a tile whose every layer we skip must not read as blank and get its perfectly good cache entry pruned and re-downloaded every session. That test moves off "has features" and onto "parsed into layers", which is sound because the other two ways a tile isn't real -- empty and corrupt -- decode to nothing and raise respectively. Only the decode narrows; whole tiles are still cached, so drawing more later costs a re-decode and never a re-download. Measured on the device across the 46 real cached tiles: 183 s -> 51 s, 3.6x, with the drawn layers byte-identical on every one. The median tile is 1259 -> 353 ms, the largest 3545 -> 759 ms. _Reader.skip stopped slicing out the bytes it steps over while it was here, which is most of what makes the header walk cheap. Co-Authored-By: Claude Opus 5 (1M context) --- meshterm/context.py | 5 ++- meshterm/core/mvt.py | 61 ++++++++++++++++++++++++++---- meshterm/services/basemap.py | 38 ++++++++++++++----- meshterm/ui/map_render.py | 18 ++++++++- tests/test_map.py | 73 ++++++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 19 deletions(-) diff --git a/meshterm/context.py b/meshterm/context.py index c2e5db6..923b5a4 100644 --- a/meshterm/context.py +++ b/meshterm/context.py @@ -382,8 +382,11 @@ class AppContext: """ if self._basemap_source is None: from .services.basemap import BasemapSource + from .ui.map_render import DRAWN_LAYERS - self._basemap_source = BasemapSource(self.settings.config_dir / "tilecache") + self._basemap_source = BasemapSource( + self.settings.config_dir / "tilecache", layers=DRAWN_LAYERS + ) return self._basemap_source @property diff --git a/meshterm/core/mvt.py b/meshterm/core/mvt.py index 639de94..c3f9d16 100644 --- a/meshterm/core/mvt.py +++ b/meshterm/core/mvt.py @@ -15,7 +15,7 @@ from __future__ import annotations import gzip from dataclasses import dataclass, field -from typing import Any, Optional +from typing import Any, Container, Optional # MVT geometry types (Feature.type). GEOM_POINT = 1 @@ -68,7 +68,13 @@ class _Reader: if wire_type == 0: self.varint() elif wire_type == 2: - self.blob() + # Step over the bytes rather than slicing them out: skipping is how the + # decoder walks past everything it doesn't want, and a discarded copy of + # every feature in an undrawn layer is the bulk of that cost. The length + # must land in a temporary first — ``self.i += self.varint()`` would add to + # the offset as it was *before* the varint moved it. + n = self.varint() + self.i += n elif wire_type == 5: self.i += 4 elif wire_type == 1: @@ -262,24 +268,63 @@ def _decode_layer(buf: bytes) -> Layer: return layer -def decode_tile(data: bytes) -> list[Layer]: +def _layer_head(buf: bytes) -> tuple[str, int]: + """Read a Layer message's identity — ``(name, extent)`` — without decoding features. + + Walks the layer's top-level fields, stepping over the features and the string pools + instead of building them. Protobuf fields may appear in any order, so this reads to + the end rather than stopping at the first name; every field it passes is a varint or + a length-skip, so the walk stays cheap even on a layer holding thousands of features. + + Args: + buf: The Layer message's bytes. + + Returns: + The layer's name (empty if it declares none) and its coordinate extent. + """ + r = _Reader(buf) + name = "" + extent = 4096 + while not r.eof(): + field_no, wire = r.tag() + if field_no == 1 and wire == 2: + name = r.blob().decode("utf-8", "ignore") + elif field_no == 5 and wire == 0: + extent = r.varint() + else: + r.skip(wire) + return name, extent + + +def decode_tile(data: bytes, *, layers: Optional[Container[str]] = None) -> list[Layer]: """Decode a vector tile (optionally gzip-compressed) into its layers. Args: data: Raw ``.pbf`` bytes, gzip-compressed or not. + layers: When given, only layers whose name is in it are decoded. The others are + still returned — named, with their extent, and no features — so the result + still describes the whole tile and a caller can tell a real tile from junk + without paying for geometry it will never draw. ``None`` decodes everything. Returns: - The decoded layers, in the order they appear in the tile. An empty or unparseable - tile yields an empty list rather than raising. + The tile's layers, in the order they appear in it. An empty tile yields an empty + list rather than raising; truncated or junk bytes raise, which is what lets + :class:`~meshterm.services.basemap.BasemapSource` tell a corrupt cache entry from + a tile that simply holds nothing it draws. """ if data[:2] == b"\x1f\x8b": data = gzip.decompress(data) r = _Reader(data) - layers: list[Layer] = [] + out: list[Layer] = [] while not r.eof(): field_no, wire = r.tag() if field_no == 3 and wire == 2: # Tile.layers - layers.append(_decode_layer(r.blob())) + blob = r.blob() + if layers is None: + out.append(_decode_layer(blob)) + continue + name, extent = _layer_head(blob) + out.append(_decode_layer(blob) if name in layers else Layer(name=name, extent=extent)) else: r.skip(wire) - return layers + return out diff --git a/meshterm/services/basemap.py b/meshterm/services/basemap.py index 0ec82d0..29d05b4 100644 --- a/meshterm/services/basemap.py +++ b/meshterm/services/basemap.py @@ -3,7 +3,11 @@ This is the only networked part of the map. It resolves a tile source's (versioned) tile-URL template from its TileJSON, fetches ``.pbf`` vector tiles over HTTPS, and caches them on disk so panning back over ground you've seen is instant and later sessions work offline. Decoding -is delegated to the pure :mod:`meshterm.core.mvt`. +is delegated to the pure :mod:`meshterm.core.mvt`, and narrowed to the layers the renderer +actually draws (``layers``) — a planet tile carries buildings, house numbers and POIs the +terminal map has no pixels for, and they are the bulk of its decode cost. Only the decode +narrows: whole tiles are still cached, so drawing more later costs a re-decode, never a +re-download. The default source is **OpenFreeMap** (openfreemap.org) — full-planet OpenStreetMap vector tiles, free and requiring no API key. Everything here is best-effort: with no network and no @@ -32,7 +36,7 @@ import threading import urllib.error import urllib.request from pathlib import Path -from typing import NamedTuple, Optional +from typing import Container, NamedTuple, Optional from ..core.mvt import Layer, decode_tile @@ -83,6 +87,7 @@ class BasemapSource: *, tilejson_url: str = DEFAULT_TILEJSON_URL, timeout: float = 12.0, + layers: Optional[Container[str]] = None, ) -> None: """Open a tile source backed by an on-disk cache. @@ -90,10 +95,15 @@ class BasemapSource: cache_dir: Where fetched tiles and TileJSON metadata are stored. tilejson_url: URL of the source's TileJSON document. timeout: Per-request network timeout in seconds. + layers: Restrict decoding to these layer names (the renderer passes + :data:`meshterm.ui.map_render.DRAWN_LAYERS`). Only the *decode* narrows — + the cache still stores whole tiles — so this is a pure CPU saving that a + later renderer can widen without re-fetching anything. """ self.cache_dir = cache_dir self._tilejson_url = tilejson_url self._timeout = timeout + self._layers = layers self._template: Optional[str] = None self._max_zoom: Optional[int] = None self._resolved = False # whether we've tried (success or offline) this session @@ -197,24 +207,34 @@ class BasemapSource: self._write_cached(path, raw) return layers - @staticmethod - def _decode(raw: bytes, key: tuple[int, int, int]) -> Optional[list[Layer]]: - """Decode a tile's bytes, or ``None`` if they hold nothing worth drawing. + def _decode(self, raw: bytes, key: tuple[int, int, int]) -> Optional[list[Layer]]: + """Decode a tile's bytes, or ``None`` if they aren't a tile at all. + + The question this answers is "are these real tile bytes?", and the answer decides + whether a cache entry is kept or pruned — so it must not be confused with "is + there anything here I feel like drawing". Under ``layers`` most of a tile's + content is deliberately left undecoded, and a tile whose every layer we skip + would otherwise read as blank and get its (perfectly good) cache entry deleted + and re-downloaded every session. + + So the test is whether the bytes parsed into any layer at all. That is sound + because the three ways a tile is *not* real are all distinguishable without + looking at features: empty bytes decode to no layers, and truncated or junk bytes + raise out of the parser rather than yielding a well-formed featureless layer. Args: raw: The tile's raw ``.pbf`` bytes. key: The ``(z, x, y)`` the bytes claim to be, for the log line. Returns: - The decoded layers when at least one carries a feature; ``None`` for empty, - featureless, or corrupt bytes — the three ways a tile ends up blank. + The decoded layers, or ``None`` for empty or corrupt bytes. """ try: - layers = decode_tile(raw) + layers = decode_tile(raw, layers=self._layers) except Exception as exc: # noqa: BLE001 - a corrupt tile must not crash the map _log.debug("failed to decode tile %s/%s/%s: %s", *key, exc) return None - return layers if any(layer.features for layer in layers) else None + return layers or None def _fetch_tile(self, z: int, x: int, y: int) -> Optional[bytes]: """Fetch a tile's raw bytes from the network. diff --git a/meshterm/ui/map_render.py b/meshterm/ui/map_render.py index e860137..9275917 100644 --- a/meshterm/ui/map_render.py +++ b/meshterm/ui/map_render.py @@ -120,6 +120,22 @@ _PLACE_STYLE: dict[str, tuple[str, bool, int, int]] = { _WATER_LABEL = ("#7dd3fc", False, 2) _STREET_LABEL = ("#9aa0aa", False, 7) +#: Polygon layers drawn as fills, in painting order (later wins the shared cell). +_FILL_LAYERS = ("water", "landcover", "landuse", "park") + +#: Every basemap layer :func:`_draw_tile` looks at — and so the only geometry the map +#: has any use for. A planet tile also ships ``building``, ``housenumber``, ``poi`` and +#: ``mountain_peak``, which together are a large share of its features and none of its +#: pixels: decoding them cost ~40% of every tile until this set was handed to +#: :func:`~meshterm.core.mvt.decode_tile` (measured on the Lyra — a 153 KB tile went +#: 1236 ms → 281 ms). Passed to the tile source at construction +#: (:attr:`meshterm.context.AppContext.basemap_source`); the on-disk cache still holds +#: whole tiles, so widening this set costs a re-decode, never a re-download. +DRAWN_LAYERS: frozenset[str] = frozenset( + _FILL_LAYERS + + ("waterway", "transportation", "boundary", "transportation_name", "place", "water_name") +) + @dataclass(slots=True) class _Label: @@ -199,7 +215,7 @@ def _draw_tile(frame: _Frame, layers: list[Layer], z: int, x: int, y: int) -> No ] # Fills first (water, green space) so lines and labels sit on top. - for name in ("water", "landcover", "landuse", "park"): + for name in _FILL_LAYERS: layer = by_name.get(name) if layer is None: continue diff --git a/tests/test_map.py b/tests/test_map.py index 6ac8bf3..13700fc 100644 --- a/tests/test_map.py +++ b/tests/test_map.py @@ -63,6 +63,52 @@ def test_decode_tile_handles_empty_input() -> None: assert decode_tile(b"") == [] +def test_decode_tile_filter_keeps_the_whole_tile_visible() -> None: + """A narrowed decode still names every layer — it only skips their features. + + The tile source decides whether a cache entry is real by whether it parsed into + layers, so narrowing must never make a tile look like it holds nothing. + """ + raw = _FIXTURE.read_bytes() + full = decode_tile(raw) + narrow = decode_tile(raw, layers={"water"}) + + assert [layer.name for layer in narrow] == [layer.name for layer in full] + assert all(layer.extent == 4096 for layer in narrow) + picked = {layer.name: layer for layer in narrow} + assert picked["water"].features # the one we asked for was decoded + assert not picked["building"].features # the rest were skipped, not dropped + + +def test_decode_tile_filter_is_invisible_to_the_renderer() -> None: + """Narrowing to ``DRAWN_LAYERS`` draws exactly the map a full decode draws. + + This is the anti-drift guard: if ``_draw_tile`` grows a lookup for a layer that + isn't in ``DRAWN_LAYERS``, that layer arrives featureless and the rendered output + diverges here. + """ + from meshterm.ui.map_render import DRAWN_LAYERS, MapMarker, render_map + + raw = _FIXTURE.read_bytes() + vp = Viewport(45.5019, -73.5674, 14, 180, 120) + markers = [MapMarker("Yagi", 45.5019, -73.5674, is_repeater=True)] + + def drawn(layers): + return render_map(vp, {(14, 4843, 5861): layers}, markers) + + assert drawn(decode_tile(raw, layers=DRAWN_LAYERS)) == drawn(decode_tile(raw)) + + +def test_decode_tile_filter_skips_the_layers_the_map_never_draws() -> None: + """The layers left undecoded are the ones that made a tile expensive.""" + from meshterm.ui.map_render import DRAWN_LAYERS + + narrow = {layer.name: layer for layer in decode_tile(_FIXTURE.read_bytes(), layers=DRAWN_LAYERS)} + for skipped in ("building", "housenumber", "poi"): + assert skipped in narrow, f"{skipped} should still be named" + assert not narrow[skipped].features, f"{skipped} should not have been decoded" + + # -- projection / viewport ---------------------------------------------------- @@ -398,6 +444,33 @@ def test_basemap_source_caches_only_tiles_with_content(tmp_path: Path) -> None: assert (cache / "tiles" / "14" / "4843" / "5861.pbf").exists() +def test_basemap_source_keeps_a_tile_whose_layers_are_all_undrawn(tmp_path: Path) -> None: + """Narrowing the decode must not make a good cached tile look blank and get pruned. + + A source told to decode a layer the tile doesn't carry sees no features at all. If + that were read as "nothing here", the entry would be deleted and re-downloaded on + every session — the cache paying for a rendering decision. + """ + from meshterm.services.basemap import BasemapSource + + cache = tmp_path / "cache" + tile = cache / "tiles" / "14" / "4843" / "5861.pbf" + tile.parent.mkdir(parents=True) + tile.write_bytes(_FIXTURE.read_bytes()) + + src = BasemapSource( + cache, + tilejson_url="http://127.0.0.1:1/none", + timeout=0.2, + layers=frozenset({"a_layer_this_tile_does_not_have"}), + ) + layers = src.load_tile(14, 4843, 5861) + + assert layers, "a real tile must still read as real" + assert not any(layer.features for layer in layers) # nothing was decoded + assert tile.exists(), "the cache entry must survive" + + def test_basemap_source_remembers_a_blank_tile_for_the_session(tmp_path: Path) -> None: """An answered-empty tile isn't re-requested this session (but isn't written down).""" from meshterm.services import basemap as basemap_mod