Repository navigation
Vector style support, tile request semantics, and marker tile performance - #8
Merged
Merged
Conversation
bearing を「値を増やすと地図が時計回りに回る」向きに統一し、ネイティブ SDK との変換をコアの CameraBearing に集約する。
The destination came straight out of the projection, so it landed on a fraction of a pixel almost every time, and a non-integer destination makes the canvas resample every marker. Measured in Chromium with 20k markers: 277 ms unaligned against 38 ms aligned. Rounding moves a pin by at most half a pixel, which is not visible at icon scale. The same change is worth 20x on Android and 4.6x on iOS, measured on each. Both draw loops needed it. tile-sw.js carries its own copy of the renderer and is the path that actually runs in a browser, so fixing only the TypeScript one would have left the fix unreachable. Also stop the second query+prepare pass from firing on every tile. The fixed 32px assumption is smaller than the real icons, so the "conservative first pass" never paid off and simply doubled the work; starting from the largest extent a tile has actually needed costs one extra pass in total.
Zoom out far enough and a whole city collapses onto a few hundred pixels: at z6 a dataset of 20k markers resolves to a couple of thousand distinct positions, and the rest are drawn underneath copies of themselves. Keeping the last of each group is what would have been visible anyway, since drawing is in painters order. Only exact agreement counts — same rounded rectangle, same icon — so nothing that could peek out from behind another is dropped. Where markers share a rectangle but not an icon, none are dropped: a different icon may be transparent where the one above it is not, and then the one underneath does show through. This is deliberately not the decluttering the docs describe. That thins markers that are merely close, which changes what the map shows and is a product decision; this only removes drawing that has no effect. A test renders fifty stacked copies and one marker and asserts the two tiles are identical. Per tile at z6 with 20k markers: 414 ms -> 261 ms on an iPad Pro, 852 ms -> 617 ms on a Pixel 5a. The Android gain is smaller because its tile is 1344px against 512px, so fewer markers land on the same pixel. Both draw loops needed it. tile-sw.js carries its own copy of the renderer and is the path that actually runs in a browser.
respondWith intercepts ahead of the HTTP cache, so the Cache-Control header on these responses does nothing and every repeat request re-rendered the tile. On the post office sample, the same tile fetched twice: z10 1.2 ms -> 0.0 ms z8 14.6 ms -> 0.0 ms z6 23.7 ms -> 0.0 ms z5 17.1 ms -> 0.0 ms Panning back over ground already visited re-requests those tiles constantly, and the work lands on the worker's one thread. Budgeted in bytes rather than entries. Counting entries is the mistake this project has already made once — a byte-blind cache of a few hundred tiles is tens of megabytes — and a service worker has no low-memory callback to catch it. A re-registration drops the route's tiles, since its markers have changed and everything drawn for it is now wrong; verified by leaving the page and coming back, which turns a cache hit back into a render. The header cache holds bytes, not Responses: a Response body can only be read once, so each hit builds a fresh one.
…dian The cells were 0.02 degrees. On 144k markers a tile-sized query costs 2.31 ms at that size and 0.06 ms at 0.005, because a 0.02 cell hands back about five times the markers a tile actually needs. The Android and iOS indexes were measured onto 0.005 this week; this is the same number for the same reason. Both callers pad their query by a few pixels — a tile by its border, a click by the icon's half extent. Near 180 degrees that padding walks off the end of the coordinate range, and the result was one of two silent failures: corners that come in reversed iterated no cells at all and returned nothing, and a west or east past ±180 compared as a plain number, so a click at 179.99 could not reach a marker at -179.99. Columns now fold back onto the globe, at insert and at query, and the containment test measures the angle east of `west` rather than comparing raw degrees. Neither threw. Markers just went missing, which is why the test compares against a brute-force scan on every query instead of asserting that something came back.
Seven test files were on disk and invisible to git. The ignore list carries `*.mjs`, `*.js`, `*.cjs` and `*.mts` unanchored to catch build output, so they matched everything under test/ as well — `!src/tileserver/tile-sw.js` sitting a few lines above is the same problem, patched narrowly the last time someone hit it. Nothing here is new: these are the tests that already run under `npm test`, all 69 of them passing, only now they are in the repository rather than on one machine. Fresh clone, `npm test`, seven files short and nobody the wiser. `!test/**` is the narrow fix, matching the exception already in the file. Anchoring the patterns to `dist/` would be the broad one, but that changes what every other pattern matches and belongs in its own change.
Keeping one marker per cell of a few pixels is what makes 144,183 street trees readable below street level: they are planted metres apart along a road, so most of them sit on top of one another, and thinning them halves the tile bytes while showing the same map. android-sdk grew the option when that sample was built; porting the sample to the other two turned up its absence. Zero, the default, keeps the behaviour these renderers already had — drop only markers that agree exactly, same rectangle and same icon, which cannot change the tile. Above zero the group is a cell and the icon stops being part of it: the caller has said markers that close together are interchangeable, so one of them stands for the rest whatever they draw. The value is tile pixels on all three, not points or dp. It is compared against destination rectangles the renderer has already rounded into the tile's own pixel grid, and passing it through unscaled is what makes 14 mean the same density on a 3x iPhone as on Android.
`zoom-golden.txt` records what each platform's providers returned before the migration, bug included -- ios-for-mapbox's missing zoom offset is preserved there on purpose. It is an archive, not a specification, so comparing those three files says nothing about whether the current implementations agree. They did not. Running one set of vectors through all three: before: 4 of 34 rows agreed after: 34 of 34 Three causes, none defensible for a provider-independent calculation: - Android truncated to Float32, because `Offset` is Compose's and packs two Floats. Web Mercator metres reach 2e7, where seven significant digits do not hold the fraction: Tokyo came out 0.18 m off. Fixed on the android side. - Only iOS clamped latitude. This returned 238107693 for `lat=90` and `-Infinity` for `lat=-90` -- the same expression breaking differently depending on sign. - Only iOS wrapped longitude, so `lon=180` came out with opposite signs. This is web's half: clamp to ±85.05112878 and wrap, matching iOS. The test emits this platform's answers and, now that `projection-expected.txt` exists, requires them to match it exactly. That file is byte-identical in all three repos, so any one drifting fails its own CI. The baseline gains WEB_MERCATOR_MAX_LATITUDE. It also absorbs the declutter surface that was already committed here without a dump -- separating them is not possible when the baseline is one file.
…hole Same scheme as android-sdk and ios-sdk, held differently: a JS number carries 53 exact integer bits and its bitwise operators truncate to 32, so every shift is written as arithmetic (measured 12ms to key 144k markers, against 90ms for BigInt) and the index stores level -> Map buckets instead of packing key and position into one word. The geocell conformance fixtures are byte-identical across the three SDKs. The renderer's declutter grid anchors to the world, thinned queries pick winners without looking at the box, and MarkerTileSeam.test.mjs mirrors the other two platforms' seam test.
An empty spot and a render that failed both arrived as null, and both were answered with a transparent tile. A map library keeps what it is given, so a tile that failed for a moment -- data still loading, a worker torn down mid-request, a page that did not answer -- became a hole that was never requested again, with the neighbouring tiles' overhang cut off at its edge. That is the shape of 'the icons are cut at the tile seams', and ios-sdk and android-sdk were fixed for it first; this brings the browser to the same three answers. A provider still says 'empty' by returning null and now says 'failed' by throwing. The service worker answers empty with a transparent PNG at 200 and a failure with 503, no-store, Retry-After -- so the library retries instead of caching the hole. The same distinction reaches the worker-rendering path and, through handleFetchOutcome() and fetchTileOrThrow(), the providers that hand tiles straight to a map library with no HTTP in between. handleFetch() keeps its signature and its meaning for existing callers. A page or worker built before this protocol sends no outcome, and there null only ever meant 'nothing to draw', so it is read as empty. The service worker's decision table is pinned by a test that loads the worker into a vm with stubbed globals and checks the status code of each answer.
… the zoom property
Tile size is the app's choice and 512 is the cheaper default -- the per-tile costs (a round trip through the tile server, a render, a PNG encode, a decode) scale with the count, so fewer, bigger tiles win for the same screen. But some SDKs have an opinion the layer supplying the tiles cannot see. ArcGIS's 3D SceneView is one: it picks the level as though every tile were 256 pixels, so 512-pixel tiles make it fetch one level deeper and cover the screen four times over. The picture is right, which is what makes it easy to miss, and the cost lands on the slowest devices. A provider can now register RasterTilePreference and be answered; saying nothing means it has no opinion and the supplier's default stands. An explicit tile size from the app always wins over both. The API baseline also picks up the private member that went with "hot reload does not work when 0 is set to the zoom property"; the dump had not been rerun since.
BLANK_MAP_STYLE (a MapLibre style with nothing but a background) and BLANK_MAP_STYLE_URL. android-sdk-core and ios-sdk-core carry the same style.
A map that draws vector styles itself registers it; VectorStyleAsDesign makes the style the design and restores the previous one a beat later, so a remount by the change it caused does not loop.
Both sides fixed large icons clipped at tile edges, differently. develop (#5) sizes the query padding from every marker's icon once per zoom, before the query, and pads through tileToGeoBounds. This branch started from a 32 px guess, widened it to what tiles turned out to need and re-queried. develop's way knows the padding before it asks, so the re-query and observedHalfExtentPx go. Kept from this branch: the extra declutter cell on the padding (now passed to tileToGeoBounds as paddingPx + declutterPx) and thinning at the index, whose separation is still measured on the unpadded tile. MarkerTileBounds.test.mjs (from develop) predates two behaviours of this branch, so its expectations move with them: markers are drawn on whole pixels (origin and size rounded), and the service worker answers { kind: 'tile' | 'empty' | 'failed' } instead of a blob or null. The api baseline's export list is the union of both sides, which is what the build emits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
feature/bearing-clockwiseのうち、まだdevelopに入っていない変更を取り込みます。bearing の変更は #2 で、zoom 0 の修正（f74824e と同じ内容）は #4 で、それぞれマージ済みです。カメラ関係のファイル（MapViewState.ts/MapCamera.ts）はdevelopと同じ内容なので、#7 とは独立しています。ベクタースタイル・デザイン
VectorStyleSupportを追加しました。ベクタースタイルを自分で描けるマップはこれを登録します。VectorStyleAsDesignはスタイルをデザインとして使い、少し後に元のスタイルに戻します。BLANK_MAP_STYLE/BLANK_MAP_STYLE_URLを追加しました。各バックエンドの「ベースマップなし」はこれで表します（android/iOS と同じスタイルです）。RasterTilePreferenceを追加しました。プロバイダーが希望のタイルサイズを伝えられます。アプリが明示した値が常に優先されます。ラスタータイルの preference も拡張しました。タイルリクエスト
Retry-After）handleFetchOutcome()とfetchTileOrThrow()を追加しました。HTTP を通さずにタイルを渡すプロバイダーでも、同じ区別ができます。handleFetch()は、シグネチャも意味も変えていません。respondWithは HTTP キャッシュより先に割り込むので、Cache-Controlが効かず、毎回描き直していました。マーカー
declutterPxを追加しました（android-sdk と同じオプションで、値はタイルのピクセル単位です）。tile-sw.js側にも同じ変更を入れています。その他
.gitignoreに!test/**を追加し、テストスイートを git で管理するようにしました。