Skip to content

ImageManager.updatedImages is write-only: every style switch with a sprite permanently inflates per-frame cost (fps stairs 60→30→20→12 until page reload) #8052

Closed

Description

@77-1812-MC

Summary

ImageManager#updatedImages is written in exactly one place and never cleared anywhere — not per frame, not in removeImage(), not even in destroy(). ImageAtlas#patchUpdatedImages enumerates this dictionary per tile, per frame. Because Style#_loadSprite routes every already-known icon id through the updateImage() branch, returning to a previously visited style (plain map.setStyle(url) on the default diff path) marks that style's entire sprite as updated, forever.

On a map that switches between two styles with large sprites, the set saturates after one or two round-trips and every subsequent frame pays a full enumeration of ~2000 dead entries on every in-view vector tile. On our production map (RTX 5060 Ti, 2544×1220 canvas, ~70 vector tiles in a pitch-60 view) this permanently degrades a resting 60/30 fps scene to a 30/20/12 fps vsync staircase until the page is reloaded. Reload heals it because a rebuilt Style gets a fresh ImageManager.

We believe this is unreported: searching this repo for patchUpdatedImages in issues/PRs returns zero results. PR #7954 (animated icons) touches the same code path but does not clear the set either.

Environment

  • maplibre-gl-js 6.0.0 (dist); the same code shape is present in main and at least as far back as 4.7.1
  • Chromium 140-level (Brave) / Windows 11 / RTX 5060 Ti — but the mechanism is engine-independent

Steps to reproduce

  1. Load style A that has a sprite (we used a basemap style whose sprite has 1665 icons; any large sprite works).
  2. map.setStyle(styleB) where B has a different sprite URL (ours had 264 icons), then map.setStyle(styleA) again. Default options — the diff path keeps the same Style/ImageManager instance.
  3. Rotate the camera in a tile-heavy view (high pitch makes it obvious) and watch frame times.
  4. Inspect Object.keys(map.style.imageManager.updatedImages).length — after step 2 it equals |spriteA| and never decreases; after one more round-trip it is |spriteA| + |spriteB|.
  5. Control: run the same camera work without any setStyle — no degradation, confirming the cost is the stale set, not the animation. Setting map.style.imageManager.updatedImages = {} on an idle, degraded map instantly restores the original frame times (we measured the per-frame getImage call count going from ~82k/frame to 0).

Analysis (with code references, 6.0.0 dist ≡ main sources)

Write side — src/render/image_manager.ts:

updateImage(id, image, validate = true) {
    // ...
    image.version = oldImage.version + 1;
    this.images[id] = image;
    this.updatedImages[id] = true;   // <-- only write; no code path ever deletes or resets this
}

beginFrame() clears only callbackDispatchedThisFrame. removeImage() deletes from images and patterns but not from updatedImages. destroy() doesn't touch it either.

Read side — src/render/image_atlas.ts, called from Painter.render → tileManager.prepare → tile.prepare for every in-view tile that has an imageAtlas, every frame:

patchUpdatedImages(imageManager, texture) {
    imageManager.dispatchRenderCallbacks(this.haveRenderCallbacks);
    for (const name in imageManager.updatedImages) {
        this.patchUpdatedImage(this.iconPositions[name], imageManager.getImage(name), texture);
        this.patchUpdatedImage(this.patternPositions[name], imageManager.getImage(name), texture);
    }
}

patchUpdatedImage early-returns on position.version === image.version, so the steady-state cost is pure enumeration + 2 × getImage per entry per tile per frame — no texture uploads (we verified GPU texture counts stay flat while fps degrades). A micro-benchmark of this exact loop shape gives ~0.24 ms per atlas-carrying tile per frame at ~1900 entries; at 60–120 such tiles that is 15–30 ms/frame, which matches the measured staircase. Note that worker-built imageAtlas is attached to every vector tile regardless of whether it uses any icons, so even icon-free overlay sources pay full price.

Filling side — src/style/style.ts, _loadSprite: every id already present in imageManager.images goes through the updateImage branch, so a sprite reload marks the whole sprite updated. And ids are practically always already present because of a second bug:

Related bug: dead cleanup in _loadSprite

this._spritesImagesIds[spriteId] = [];   // <-- reset FIRST...
const imagesToRemove = this._spritesImagesIds[spriteId]
    ? this._spritesImagesIds[spriteId].filter(id => !(id in images))   // <-- ...then filter the now-empty array
    : [];

imagesToRemove is always [], so icons of a replaced sprite are never removed from the ImageManager (also note the predicate compares icon ids against sprite namespace keys, which would be wrong even if the array weren't empty). This guarantees the updateImage branch on every style return and leaks decoded RGBA of all previously seen sprites. Style#setSprite only calls _unloadSprite() when the new style has no sprite at all.

Third, minor: wasted synchronous decode in updateImage

updateImage calls this.getImage(id) only to read .version — and getImage force-decodes spriteData via canvas getImageData for a record that the very next line replaces. On a sprite reload that is up to |sprite| wasted synchronous canvas readbacks inside the style-switch window. const oldImage = this.images[id] would avoid it.

Suggested fixes

  1. Clear updatedImages once all tile atlases have been patched (or track a per-atlas patched version), and delete this.updatedImages[id] in removeImage().
  2. Fix the dead imagesToRemove filter in _loadSprite (capture the previous array before resetting; de-namespace ids before the in check). Note addImage throws on an existing id, so fix 2 should land together with tests over style-switch round-trips.
  3. In updateImage, read the old version without getImage's decode side effect.

We're happy to provide more measurements (CPU profiles showing patchUpdatedImages at 44% self-time in the degraded state and absent after reload) or test the fix against our reproduction.

Activity

  1. HarelM commented on Jul 29, 2026

    @HarelM
    Collaborator

    Thanks for opening this issue.
    Please read our AI policy.
    If you think there's something worth fixing a PR is a much better tool for that specific problems...

  2. 77-1812-MC commented on Aug 1, 2026

    @77-1812-MC
    Author

    Thank you for the reply, and I've read the AI policy - I owe you a disclosure I should have made upfront: I'm not a programmer. I build my sound-map website with the help of AI (Claude), and both the original issue text and this reply were drafted by it. I apologize for not marking that clearly from the start.

    What I can vouch for personally is that the problem is real: the measurements in the issue come from my actual production map, where switching styles a few times permanently drops a smooth 60 fps scene to a choppy one until the page is reloaded, and clearing updatedImages manually instantly heals it.

    Regarding a PR: honestly, I'm not able to author one myself, and I don't want to submit AI-written code under my name that I couldn't defend in review - that seems to be exactly what your policy is against.

  3. HarelM commented on Aug 7, 2026

    @HarelM
    Collaborator

    Can you have the AI generate a minimal reproduction of this issue using the latest version? I've seen a few PRs related to this area recently, so there might be an open PR to solve this already...

    Bottom line, even for an AI, following the issue template is the best approach.

  4. HarelM commented on Aug 8, 2026

    @HarelM
    Collaborator
  5. HarelM commented on Aug 8, 2026

    @HarelM
    Collaborator

    @lucaswoj you just recently changed code in this area, is the linked PR and your work solves this issue?

  6. 77-1812-MC commented on Aug 8, 2026

    @77-1812-MC
    Author

    Thanks for looking at this! I had the AI dig into both questions; same disclosure as before — the analysis and text below are AI-drafted (Claude), I ran the measurements myself on my machine.

    Does #8094 solve this one?

    Unfortunately no — related in name only:

    On current main (871bb78, i.e. after #8094 and after #7954) the mechanism is unchanged:

    • the only write, never cleared: image_manager.ts#L192 — and removeImage() doesn't delete the entry either;
    • enumerated per atlas-carrying tile per frame: image_atlas.ts#L143-L149;
    • the dead imagesToRemove cleanup that guarantees the updateImage branch on every style return: style.ts#L542-L546 (the array is reset on L542, then the filter on L545 reads the freshly-emptied array → always []).

    #7954 doesn't clear the dictionary either — for its own (legitimate) purposes it adds one more write path (addImage() now routes WebGL-backed images through updateImage()). We also couldn't find any open PR that clears it, and no PR references this issue.

    Minimal reproduction on 6.2.0 (latest release)

    maplibre-gl-js version: 6.2.0 (the release changes none of the three code sites above — verified against the dist bundle) · browser: Chromium 140-level (Edge/Brave), Windows 11 — the mechanism itself is engine-independent.

    One self-contained page — the full source is in the collapsed section at the bottom of this comment (happy to also put it on jsbin if a clickable link is easier). Design: style A = OpenFreeMap Liberty (sprite: 264 icons as loaded); style B = the same style object with only the sprite URL swapped (Carto's), so map.setStyle() stays on the default diff path and the only diff operation is setSprite — isolating the mechanism (switching between two entirely different styles does the same thing). The HUD shows the updatedImages size, ImageManager.getImage() calls per rendered frame (the per-stale-entry × per-tile work patchUpdatedImages does), and frame time; map.repaint = true keeps the render loop hot the way a continuous pan/rotate would.

    Steps: open the page (a 1440×900 window gives ~32 atlas-carrying tiles at pitch 60), wait for load, click "Style round-trip A→B→A" once, compare the HUD; then click "Heal".

    Expected: once the sprite reload settles and every in-view atlas has been patched, per-frame work returns to the pre-switch level (no permanent per-frame cost from a completed style switch).

    Actual (measured):

    updatedImages entries getImage() calls / frame
    before the first switch 0 0
    after ONE round-trip 264 — the whole sprite, forever 16 896 (= 264 × 2 × 32 tiles)
    after a second round-trip 265 (adds the other sprite's id; never decreases) 16 960
    after "Heal" (updatedImages = {} on an idle map) 0 0, instantly

    The counters are hardware-independent; the wall-clock cost scales with entries × tiles. At this small repro scale a desktop GPU still hides it inside the 60 fps budget, but the identical run under software rendering already shows the EMA frame time recovering 41.8 → 35.9 ms at the moment of "Heal". At our production scale — two real sprites totalling 1929 ids and ~70 tiles at pitch 60 — this is exactly the 60→30→20→12 staircase from the report, and a page reload heals it because a rebuilt Style gets a fresh ImageManager

    repro.html (self-contained, maplibre-gl@6.2.0 from unpkg)
    <!doctype html>
    <!--
      Minimal reproduction for maplibre-gl-js issue #8052:
      "ImageManager.updatedImages is write-only: every style switch with a sprite
       permanently inflates per-frame cost (fps stairs until page reload)"
    
      maplibre-gl 6.2.0 (latest release), loaded from unpkg. Serve this file over
      HTTP (any static server) and open it — everything else is fetched remotely.
    
      What it demonstrates
      - Style A = OpenFreeMap Liberty (sprite with ~264 icons as loaded). Style B =
        the SAME style object with ONLY the sprite URL swapped (Carto Positron's
        sprite), so map.setStyle() stays on the default diff path and the only diff
        operation is setSprite -> Style._loadSprite. This isolates the mechanism;
        switching between two entirely different styles does the same thing.
      - One A->B->A round-trip routes every icon id of sprite A through
        ImageManager.updateImage() (they are all still resident, because
        _loadSprite's imagesToRemove filter is dead — see the issue), and
        updateImage sets updatedImages[id] = true. No code path ever clears or
        deletes those entries again.
      - ImageAtlas.patchUpdatedImages() then enumerates the whole dictionary — plus
        TWO ImageManager.getImage() calls per entry — for every atlas-carrying tile,
        every rendered frame. The "getImage calls / frame" counter in the HUD is the
        hardware-independent measure of that cost: 0 before the first round-trip,
        (2 x entries x tiles) after it, forever.
      - "Heal" applies the one-line workaround (updatedImages = {}) on an idle map:
        calls/frame drops back to 0 instantly and frame times recover. A page
        reload heals too (fresh Style => fresh ImageManager) — which is exactly the
        signature reported in the issue.
    
      Everything uses the public API except the two marked reads of
      map.style.imageManager (required to observe the private dictionary this issue
      is about) and map.repaint (the debug flag, used to keep the render loop hot
      the way a continuous pan/rotate would).
    -->
    <html lang="en">
    <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <title>maplibre-gl-js #8052 repro — write-only ImageManager.updatedImages</title>
    <link rel="stylesheet" href="https://unpkg.com/maplibre-gl@6.2.0/dist/maplibre-gl.css">
    <style>
      html, body { margin: 0; height: 100%; }
      #map { position: absolute; inset: 0; }
      #hud {
        position: absolute; top: 10px; left: 10px; z-index: 10;
        background: rgba(255,255,255,.94); border: 1px solid #999; border-radius: 6px;
        padding: 10px 12px; font: 12px/1.5 Consolas, monospace; max-width: 560px;
      }
      #stats { white-space: pre; margin-bottom: 8px; }
      #hud button { font: inherit; margin-right: 8px; padding: 4px 10px; cursor: pointer; }
      #log { margin-top: 8px; max-height: 160px; overflow-y: auto; color: #333; }
      #log div { border-top: 1px dotted #bbb; padding: 1px 0; }
    </style>
    </head>
    <body>
    <div id="map"></div>
    <div id="hud">
      <div id="stats">loading…</div>
      <button id="rt">Style round-trip A→B→A</button>
      <button id="heal">Heal: updatedImages = {}</button>
      <div id="log"></div>
    </div>
    <script type="module">
    // Note: the 6.x ESM build has no default export — use a namespace import.
    import * as maplibregl from 'https://unpkg.com/maplibre-gl@6.2.0/dist/maplibre-gl.mjs';
    const glVersion = () => (maplibregl.getVersion ? maplibregl.getVersion() : maplibregl.version);
    
    const statsEl = document.getElementById('stats');
    const logEl = document.getElementById('log');
    const log = (m) => {
      console.log('[repro]', m);
      const d = document.createElement('div');
      d.textContent = m;
      logEl.prepend(d);
    };
    
    // Style A: a public style whose sprite has many icons.
    const STYLE_A_URL = 'https://tiles.openfreemap.org/styles/liberty';
    // Style B = style A with ONLY this sprite URL swapped in (any reachable sprite
    // with a DIFFERENT URL works — the content is irrelevant, the reload is what
    // matters). Note: two styles of the same publisher may share one sprite URL
    // (OpenFreeMap liberty/bright do today), which is why B borrows Carto's.
    const SPRITE_B_URL = 'https://tiles.basemaps.cartocdn.com/gl/positron-gl-style/sprite';
    
    const styleA = await (await fetch(STYLE_A_URL)).json();
    const styleB = Object.assign(structuredClone(styleA), { sprite: SPRITE_B_URL });
    
    const map = new maplibregl.Map({
      container: 'map',
      style: structuredClone(styleA),
      center: [21.017, 52.237],  // Warsaw — densely tiled area
      zoom: 15.2,
      pitch: 60,                 // pitched view => many tiles in view
      bearing: 20,
    });
    
    // ── Instrumentation ────────────────────────────────────────────────────────
    // Count ImageManager.getImage() calls per rendered frame: this is what
    // ImageAtlas.patchUpdatedImages() spends per stale entry per tile per frame.
    let getImageCalls = 0;
    function armCounter() {
      const im = map.style && map.style.imageManager;      // private, observed read-only
      if (!im || im.getImage.__counted) return;
      const orig = im.getImage.bind(im);
      const wrapped = (id) => { getImageCalls++; return orig(id); };
      wrapped.__counted = true;
      im.getImage = wrapped;
    }
    map.on('styledata', armCounter);
    
    const imgr = () => (map.style && map.style.imageManager) || null;
    const updatedCount = () => { const im = imgr(); return im ? Object.keys(im.updatedImages).length : 0; };
    const residentCount = () => { const im = imgr(); return im ? Object.keys(im.images).length : 0; };
    
    let emaMs = 0, worstMs = 0, callsPerFrame = 0, lastT = 0;
    function tick(t) {
      if (lastT) {
        const dt = t - lastT;
        emaMs = emaMs ? emaMs * 0.95 + dt * 0.05 : dt;
        worstMs = Math.max(worstMs * 0.98, dt);
      }
      lastT = t;
      callsPerFrame = getImageCalls;
      getImageCalls = 0;
      statsEl.textContent =
        `maplibre-gl v${glVersion()}\n` +
        `updatedImages entries : ${updatedCount()}   (never decreases)\n` +
        `resident images       : ${residentCount()}\n` +
        `getImage calls / frame: ${callsPerFrame}\n` +
        `frame ms (EMA / worst): ${emaMs.toFixed(1)} / ${worstMs.toFixed(1)}`;
      requestAnimationFrame(tick);
    }
    requestAnimationFrame(tick);
    
    // ── Actions ────────────────────────────────────────────────────────────────
    function setStyleAwaitIdle(style) {
      return new Promise((resolve) => {
        map.once('idle', resolve);
        map.setStyle(structuredClone(style), { diff: true }); // diff:true is the default; spelled out for clarity
      });
    }
    
    let busy = false;
    async function roundTrip() {
      if (busy) return; busy = true;
      const wasRepaint = map.repaint;
      map.repaint = false;                    // let 'idle' fire during the switches
      log('switch A→B (sprite → Carto) …');
      await setStyleAwaitIdle(styleB);
      log(`… B idle: updatedImages=${updatedCount()}; switch B→A …`);
      await setStyleAwaitIdle(styleA);
      map.repaint = wasRepaint;
      log(`round-trip done: updatedImages=${updatedCount()}, resident images=${residentCount()}`);
      busy = false;
    }
    
    function heal() {
      const im = imgr();
      if (!im) return;
      im.updatedImages = {};                  // the one-line workaround this issue proposes to make unnecessary
      log(`healed: updatedImages=${updatedCount()} — calls/frame drops to 0 next frame`);
    }
    
    document.getElementById('rt').addEventListener('click', roundTrip);
    document.getElementById('heal').addEventListener('click', heal);
    
    map.on('load', () => {
      armCounter();
      map.repaint = true;                     // keep rendering every frame, like a continuous pan/rotate
      log(`loaded maplibre-gl v${glVersion()}; sprite A resident images: ${residentCount()}`);
      log('click "Style round-trip" once, then watch getImage calls/frame');
    });
    
    // Hook for automated runs (used by the local headless driver; harmless in a browser).
    window.__repro = {
      roundTrip, heal,
      stats: () => ({
        version: glVersion(),
        updated: updatedCount(),
        resident: residentCount(),
        callsPerFrame,
        emaMs: +emaMs.toFixed(1),
        worstMs: +worstMs.toFixed(1),
        loaded: map.loaded(),
      }),
    };
    </script>
    </body>
    </html>

    Happy to re-run this repro (and our production-scale measurement) against any candidate fix PR.

    Image Image
  7. HarelM commented on Aug 8, 2026

    @HarelM
    Collaborator

    The attached HTML is a bad minimal reproduction as I have no clue what the problem is and what it should be showing without reading a lot of code and trying to understand what the AI wrote here.
    If this is something that you need fixed, I suggest hire someone for a short period of time to solve this.
    Sorry...

  8. 77-1812-MC commented on Aug 8, 2026

    @77-1812-MC
    Author

    No problem, I don't need it fixed. I posted it in case it was helpful to you as a developer. I understand it isn't, so I won't post more of these. Claude fixes such issues for me on the fly and recheck them against each new Maplibre release, so I'm fine either way. Thanks for your time.

  9. HarelM commented on Aug 11, 2026

    @HarelM
    Collaborator
  10. added
    bugSomething isn't working
    and removed
    need more infoFurther information is requested
    on Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions