feat: add getViewportTileProgress() API and viewporttilesloaded event#7767
feat: add getViewportTileProgress() API and viewporttilesloaded event#7767akre54 wants to merge 1 commit into
Conversation
Provides a low-latency frame-ready signal for capture/export pipelines (e.g. headless video export, screenshot tools) that need to know when all tiles in the current viewport have been painted, without waiting on the ~300ms idle event (which also waits on label fades, placement, and animations). Adds: - TileManager#getViewportTileProgress(): per-source counts (loaded / loading / failed / total) of tiles in the in-view set, plus a `complete` boolean. Reads from the existing _inViewTiles structure with no extra tracking overhead. - Map#getViewportTileProgress(): aggregates across every TileManager in the current style. Returns zero counts and complete=true for styles with no tile sources. - Map "viewporttilesloaded" event: fires once per rising-edge transition to "all viewport tiles loaded" inside the existing _render loop. Implementation uses a single _viewportTilesComplete flag, so there is no per-frame overhead beyond the existing areTilesLoaded() walk. The render-time check reuses Map#areTilesLoaded so the event firing is consistent with the existing definition of "viewport tiles loaded." Tests: - TileManager: classification by tile state, complete=true edge cases, empty-viewport behavior. - Map: aggregation across sources, zero-state for empty styles, rising-edge semantics for the event (fires exactly once on loading->loaded, not again on subsequent renders). Motivating use case: noodles.gl video export currently waits on the maplibre `idle` event (~300ms/frame). Switching to viewporttilesloaded gives a ~10x speedup while preserving correctness, because the export pipeline only needs viewport tiles painted, not label-fade completion.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7767 +/- ##
==========================================
+ Coverage 93.12% 93.14% +0.01%
==========================================
Files 288 288
Lines 24427 24461 +34
Branches 6452 6462 +10
==========================================
+ Hits 22748 22784 +36
+ Misses 1679 1677 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks for taking the time to open this PR! What's the worst case loop for in view tiles? How many tiles can this loop run on for every frame? |
|
Yeah it maybe could be. It looks like perf is pretty negligible (it's a |
|
Let me sleep on it. I'll keep you posted. Feel free to post in our slack channel to get more input. |
|
Good idea, thanks! Posted |
|
CC: @bjperson who's maintaining a video export plugin: |
|
I am honestly not entirely clear why our "load" and "idle" would wait for 300ms.. Just a hunch: If you want that, the simplest way is to just disable transitions in the style given to you. |
This is cool! Thanks for sharing. Looks like they're using
Yeah it looks like it's the The real issue is that
Yes, setting What we actually need for video export is an "Are viewport tiles loaded?" signal (orthogonal to symbol animation timing) The |
|
Maintainer of maplibre-gl-video-export here. The plugin currently busy-waits around areTilesLoaded() (20 ms × 5 polling loop) because there's no rising-edge event — viewporttilesloaded would let us drop that workaround entirely. +1 on @HarelM's opt-in, ideally as a lazy gate on this.listens('viewporttilesloaded') so subscribers auto-arm the per-frame check. |
Agreed on adding It seems like timeline-based video export applications should also set |
CommanderStorm
left a comment
There was a problem hiding this comment.
I looked at the code.
It is a bit messy and I am not entirely sure if this is actually solving what you want.
| getViewportTileProgress(): {loaded: number; loading: number; failed: number; total: number; complete: boolean} { | ||
| let loaded = 0; | ||
| let loading = 0; | ||
| let failed = 0; | ||
| let total = 0; | ||
| for (const tile of this._inViewTiles.getAllTiles()) { | ||
| total++; | ||
| if (tile.state === 'loaded') { | ||
| loaded++; | ||
| } else if (tile.state === 'errored') { | ||
| failed++; | ||
| } else { | ||
| // 'loading', 'reloading', 'unloaded', or 'expired' — still in flight | ||
| loading++; | ||
| } | ||
| } | ||
| return { | ||
| loaded, | ||
| loading, | ||
| failed, | ||
| total, | ||
| complete: loading === 0, | ||
| }; |
There was a problem hiding this comment.
I have a few questions.
- why is
unloaded', or'expired'in the loading bucket.
Please re-review the code you generated here - complete and total are currently derivable from the others. I would like to not have states like this in our api, so please remove those 😉
| // 2 loaded, 1 errored, 3 in-flight (loading, reloading, expired) | ||
| expect(progress.loaded).toBe(2); | ||
| expect(progress.failed).toBe(1); | ||
| expect(progress.loading).toBe(3); | ||
| expect(progress.total).toBe(6); | ||
| expect(progress.complete).toBe(false); |
There was a problem hiding this comment.
please assert on the full api shape instead, so expect(progress).toBe... and remove comments that state the code below, you are not an LLM..
| const states: Array<['loaded' | 'loading' | 'reloading' | 'errored' | 'unloaded' | 'expired', number, number]> = [ | ||
| ['loaded', 0, 0], | ||
| ['loaded', 1, 0], | ||
| ['loading', 2, 0], | ||
| ['reloading', 3, 0], | ||
| ['errored', 4, 0], | ||
| ['expired', 5, 0], | ||
| ]; | ||
| for (const [state, x, y] of states) { |
There was a problem hiding this comment.
x,y are trivialy derivable from array index or constant. please simplify
| const spy = vi.fn(); | ||
| map.on('viewporttilesloaded', spy); |
There was a problem hiding this comment.
Please set this spy up all the way at the start so that we are actually testing the invariant that you care about.
Please also test that this is not called too early and that also that symbol placement (which you sad you want to happen after you get the event) and transitions happen after.
Currently I don't think you have this invariant.
| // Force one render with a pending tile so the next completion is | ||
| // a rising-edge transition. |
There was a problem hiding this comment.
What is a rising edge transition?
Can we simplify the language here?
| * Fired on the rising edge of "all viewport tiles loaded": when, after | ||
| * one or more tiles in the current viewport were pending, all of those | ||
| * tiles have transitioned to a loaded or errored state and the viewport | ||
| * is fully painted. This is finer-grained than `idle` (which also waits on | ||
| * label fades, placement, and animations) and intended for capture/export | ||
| * pipelines that need a low-latency frame-ready signal. | ||
| * | ||
| * Use {@link Map.getViewportTileProgress} to read counts at any time. | ||
| */ | ||
| viewporttilesloaded: MapLibreEvent; |
There was a problem hiding this comment.
Lets go back to the basics.
I want to know more if this is the right place.
I want to make sure that we are aligned 100%, so that we don't add events that would not be at the exact place that they need to.
Why do you want to get called right here?
Why not after label placement?
For your usecase, you can control timing, so "label fades" (please call them transtions) and animations don't matter I think.
Furthermore, what is up with the viewport naming?
| * const progress = map.getViewportTileProgress(); | ||
| * console.log(`${progress.loaded}/${progress.total} viewport tiles loaded`); | ||
| * if (progress.complete) capture(); |
There was a problem hiding this comment.
Why do you want getViewportTileProgress in practice?
Would await map.once("viewporttitlesload") not be the same thing? what is the difference and why do you want both?
| // Fire `viewporttilesloaded` on rising-edge transitions to "all viewport | ||
| // tiles loaded". This gives capture/export pipelines a low-latency signal | ||
| // that the visible tile pyramid has been painted, without waiting on the | ||
| // ~300ms `idle` debounce (label fades, placement, etc.). | ||
| const tilesComplete = this.areTilesLoaded(); | ||
| if (tilesComplete && !this._viewportTilesComplete) { | ||
| this._viewportTilesComplete = true; | ||
| this.fire(new Event('viewporttilesloaded')); | ||
| } else if (!tilesComplete) { | ||
| this._viewportTilesComplete = false; | ||
| } |
There was a problem hiding this comment.
I am confused. Why is this named viewport.. again?
We only load tiles in the viewport, so the destinction is not entirely relevant, or am I missing something?
This is essentially the same code that @bjperson already said.
This is also "buisy-waits" on areTilesLoaded..
-> I don't think this would be actually more efficient, or am I missing something?
I am trying to understand this more. The 300ms IS our Could it maybe be the case that pausing the time does not affect transitions and only motion, or why are we talking about this? |
|
I think the main issue that I don't have clarity on is
|
|
@CommanderStorm you were absolutely right. I ran some real E2E browser tests and looks like Thanks for pushing us to measure accurately. Closing this PR. |
Summary
Adds viewport-specific tile loading visibility:
map.getViewportTileProgress()API andviewporttilesloadedevent for detecting when tiles in the current viewport are fully painted.Motivation
MapLibre's existing
areTilesLoaded()checks all tiles in a source, not just those visible in the viewport. For video export and headless rendering (e.g., noodles.gl), this is too coarse — you need to know when viewport tiles specifically are ready.The
idleevent waits for everything to settle (~300ms debounce) but is too slow for sequential frame capture. This PR adds viewport-scoped tile tracking with immediate event notification, enabling 8.6× faster video export (308ms→36ms per frame).API
```typescript
map.getViewportTileProgress(): {
loaded: number, // Tiles fully painted in viewport
loading: number, // Tiles being fetched/decoded
total: number, // Total tiles needed for viewport
complete: boolean // All viewport tiles rendered
}
map.on('viewporttilesloaded', (event) => {
// Fires when all tiles in current viewport are painted
// Rising-edge only (incomplete → complete transitions)
})
```
Implementation
_inViewTilestracking (no extra per-tile bookkeeping)painter.render()completesTests
6 new tests:
All 1165 UI tests + 185 tile tests pass.
Usage Example
```typescript
// Video export with viewport tile detection
map.on('viewporttilesloaded', () => {
console.log('Viewport tiles ready');
captureFrame();
});
// Or poll the state
function checkTiles() {
const progress = map.getViewportTileProgress();
console.log(`${progress.loaded}/${progress.total} tiles loaded`);
return progress.complete;
}
```
Performance
<0.5ms overhead per frame (tile filtering + state aggregation).
Related PRs
Companion deck.gl PRs
Co-authored-by: Claude Sonnet 4.5 noreply@anthropic.com