-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: add getViewportTileProgress() API and viewporttilesloaded event #7767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2586,3 +2586,50 @@ describe('TileManager / etag', () => { | |
| expect(tile.etag).toBe(tileEtag); | ||
| }); | ||
| }); | ||
|
|
||
| describe('TileManager.getViewportTileProgress', () => { | ||
| test('returns zero counts and complete=true when no tiles are in view', () => { | ||
| const tileManager = createTileManager(); | ||
| const progress = tileManager.getViewportTileProgress(); | ||
| expect(progress).toEqual({loaded: 0, loading: 0, failed: 0, total: 0, complete: true}); | ||
| }); | ||
|
|
||
| test('classifies tiles by state', () => { | ||
| const tileManager = createTileManager(); | ||
| // Use distinct tile coords at z=3 (8x8 grid) to get unique keys. | ||
| 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) { | ||
| const tileID = new OverscaledTileID(3, 0, 3, x, y); | ||
| const tile = new Tile(tileID, undefined); | ||
| tile.state = state; | ||
| tileManager._inViewTiles.setTile(tileID.key, tile); | ||
| } | ||
| const progress = tileManager.getViewportTileProgress(); | ||
| // 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); | ||
|
Comment on lines
+2615
to
+2620
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please assert on the full api shape instead, so |
||
| }); | ||
|
|
||
| test('complete=true when only loaded and errored tiles remain', () => { | ||
| const tileManager = createTileManager(); | ||
| const aID = new OverscaledTileID(1, 0, 1, 0, 0); | ||
| const bID = new OverscaledTileID(1, 0, 1, 1, 0); | ||
| const a = new Tile(aID, undefined); a.state = 'loaded'; | ||
| const b = new Tile(bID, undefined); b.state = 'errored'; | ||
| tileManager._inViewTiles.setTile(aID.key, a); | ||
| tileManager._inViewTiles.setTile(bID.key, b); | ||
| const progress = tileManager.getViewportTileProgress(); | ||
| expect(progress.complete).toBe(true); | ||
| expect(progress.loading).toBe(0); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -991,4 +991,35 @@ export class TileManager extends Evented { | |
| } | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Returns counts of tiles in the current viewport by their loading state. | ||
| * Unlike {@link areTilesLoaded}, this also exposes loading/failed counts so | ||
| * callers (e.g. video export pipelines) can wait on viewport completion with | ||
| * fine-grained visibility, without inspecting private fields. | ||
| */ | ||
| 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, | ||
| }; | ||
|
Comment on lines
+1001
to
+1023
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a few questions.
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -192,6 +192,17 @@ export type MapEventType = { | |
| * Fired immediately after the map has been resized. | ||
| */ | ||
| resize: MapLibreEvent; | ||
| /** | ||
| * 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; | ||
|
Comment on lines
+196
to
+205
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets go back to the basics. 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 not after label placement? Furthermore, what is up with the viewport naming? |
||
| /** | ||
| * Fired when the WebGL context is lost. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -585,6 +585,9 @@ export class Map extends Camera { | |
|
|
||
| _loaded: boolean; | ||
| _idleTriggered = false; | ||
| // tracks whether the most recent emitted viewport-tile state was 'complete', | ||
| // so we only fire viewporttilesloaded on rising-edge transitions | ||
| _viewportTilesComplete = true; | ||
| // accounts for placement finishing as well | ||
| _fullyLoaded: boolean; | ||
| _trackResize: boolean; | ||
|
|
@@ -2439,6 +2442,37 @@ export class Map extends Camera { | |
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Returns aggregate counts of tiles needed for the current viewport, broken | ||
| * down by loading state. This complements {@link Map.areTilesLoaded} for use | ||
| * cases (e.g. headless video export, screenshot capture) that need fine-grained | ||
| * visibility into how many tiles are still in flight versus painted. | ||
| * | ||
| * @returns Counts of viewport tiles by state, plus a `complete` flag that is | ||
| * true when no tiles remain pending. When the style has no tile sources, | ||
| * `total` is 0 and `complete` is true. | ||
| * @example | ||
| * ```ts | ||
| * const progress = map.getViewportTileProgress(); | ||
| * console.log(`${progress.loaded}/${progress.total} viewport tiles loaded`); | ||
| * if (progress.complete) capture(); | ||
|
Comment on lines
+2456
to
+2458
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you want |
||
| * ``` | ||
| */ | ||
| getViewportTileProgress(): {loaded: number; loading: number; failed: number; total: number; complete: boolean} { | ||
| const result = {loaded: 0, loading: 0, failed: 0, total: 0, complete: true}; | ||
| const tileManagers = this.style?.tileManagers; | ||
| if (!tileManagers) return result; | ||
| for (const tileManager of Object.values(tileManagers)) { | ||
| const sub = tileManager.getViewportTileProgress(); | ||
| result.loaded += sub.loaded; | ||
| result.loading += sub.loading; | ||
| result.failed += sub.failed; | ||
| result.total += sub.total; | ||
| } | ||
| result.complete = result.loading === 0; | ||
| return result; | ||
| } | ||
|
|
||
| /** | ||
| * Removes a source from the map's style. | ||
| * | ||
|
|
@@ -3703,6 +3737,18 @@ export class Map extends Camera { | |
|
|
||
| this.fire(new Event('render')); | ||
|
|
||
| // 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; | ||
| } | ||
|
Comment on lines
+3740
to
+3750
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am confused. Why is this named This is essentially the same code that @bjperson already said. |
||
|
|
||
| if (this.loaded() && !this._loaded) { | ||
| this._loaded = true; | ||
| this.fire(new Event('load')); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,6 +143,63 @@ describe('Map', () => { | |
| map.style.tileManagers.geojson._inViewTiles.getTileById(fakeTileId.key).state = 'loaded'; | ||
| expect(map.areTilesLoaded()).toBe(true); | ||
| }); | ||
|
|
||
| test('Map.getViewportTileProgress returns zero counts before any source is added', async () => { | ||
| const style = createStyle(); | ||
| const map = createMap({style}); | ||
| await map.once('load'); | ||
| const progress = map.getViewportTileProgress(); | ||
| expect(progress).toEqual({loaded: 0, loading: 0, failed: 0, total: 0, complete: true}); | ||
| }); | ||
|
|
||
| test('Map.getViewportTileProgress aggregates across sources', async () => { | ||
| const style = createStyle(); | ||
| const map = createMap({style}); | ||
| await map.once('load'); | ||
| map.addSource('geojson', createStyleSource()); | ||
| const aID = new OverscaledTileID(1, 0, 1, 0, 0); | ||
| const bID = new OverscaledTileID(1, 0, 1, 1, 0); | ||
| const aTile = new Tile(aID, undefined); aTile.state = 'loading'; | ||
| const bTile = new Tile(bID, undefined); bTile.state = 'loaded'; | ||
| map.style.tileManagers.geojson._inViewTiles.setTile(aID.key, aTile); | ||
| map.style.tileManagers.geojson._inViewTiles.setTile(bID.key, bTile); | ||
|
|
||
| const progress = map.getViewportTileProgress(); | ||
| expect(progress.total).toBe(2); | ||
| expect(progress.loaded).toBe(1); | ||
| expect(progress.loading).toBe(1); | ||
| expect(progress.failed).toBe(0); | ||
| expect(progress.complete).toBe(false); | ||
| }); | ||
|
|
||
| test('Map fires viewporttilesloaded on rising-edge transitions', async () => { | ||
| const style = createStyle(); | ||
| const map = createMap({style}); | ||
| await map.once('load'); | ||
|
|
||
| map.addSource('geojson', createStyleSource()); | ||
| const fakeTileId = new OverscaledTileID(0, 0, 0, 0, 0); | ||
| const tile = new Tile(fakeTileId, undefined); | ||
| tile.state = 'loading'; | ||
| map.style.tileManagers.geojson._inViewTiles.setTile(fakeTileId.key, tile); | ||
|
|
||
| // Force one render with a pending tile so the next completion is | ||
| // a rising-edge transition. | ||
|
Comment on lines
+186
to
+187
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is a rising edge transition? |
||
| map.redraw(); | ||
| expect(map.areTilesLoaded()).toBe(false); | ||
|
|
||
| const spy = vi.fn(); | ||
| map.on('viewporttilesloaded', spy); | ||
|
Comment on lines
+191
to
+192
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
| // Promote tile to loaded and re-render. Event must fire exactly once. | ||
| map.style.tileManagers.geojson._inViewTiles.getTileById(fakeTileId.key).state = 'loaded'; | ||
| map.redraw(); | ||
| expect(spy).toHaveBeenCalledTimes(1); | ||
|
|
||
| // No rising-edge if we render again with everything still loaded. | ||
| map.redraw(); | ||
| expect(spy).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
|
|
||
| test('remove', () => { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x,y are trivialy derivable from array index or constant. please simplify