FillBucket.uploadPending() is always true: `this.uploaded` is never set, so every fill bucket re-uploads on every frame

Aperta Adatta ai principianti
  • Qualcosa non va?
#13,752 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
82/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
typescript
Ambito
performance

Direzione di ricerca

Leggi le implementazioni di FillBucket.upload() e uploadPending(), quindi confrontale con LineBucket e CircleBucket come descritto nell’issue. Riproduci il problema verificando se un fill bucket caricato rimane in sospeso dopo il caricamento; conferma la correzione verificando che non sia più in sospeso quando non è necessario caricarlo di nuovo e che i colori renderizzati rimangano invariati.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug :lady_beetle: performance :zap:
mapbox-gl-js version

3.33.0 and 3.34.2 (latest on npm; same code in both)

Browser

Chrome (desktop)

What happens

FillBucket.uploadPending() reads this.uploaded, but FillBucket.upload() never sets it. Only the inner FillBufferData sets its own uploaded flag. The bucket therefore reports "pending" forever.

From dist/mapbox-gl-dev.js (3.33.0):

// class FillBucket
uploadPending() {
  return !this.uploaded || this.bufferData.needsUpload() || this.hdExt != null && this.hdExt.needsUpload();
}
upload(context) {
  this.bufferData.upload(context);   // sets bufferData.uploaded, not this.uploaded
  if (this.hdExt) {
    this.hdExt.upload(context);
  }
}

Compare LineBucket and CircleBucket, whose upload() ends with this.uploaded = true.

Because the bucket is always pending, Tile.upload() takes its slow branch for every fill bucket on every call:

// Tile.upload
if (bucket.uploadPending()) {
  ...
  featureStates = sourceCache._state.getState(sourceLayerId, void 0);
  ...
  bucket.upload(context, this.tileID.canonical, featureStates, availableImages, globalProperties);
}

getState(sourceLayer, undefined) builds a new object with Object.assign({}, base, changes) over every feature that has state in that source layer. So the cost per frame is (visible fill tiles) x (features with feature-state), even when nothing changed.

Impact

On a map with a vector fill layer and many features carrying feature-state, the main thread spends most of a pan inside Tile.upload. Measured on a phone-size map (390x789, zoom 16) with roughly 5,000 to 7,000 features holding state, 7 pans over the same ground with the tile cache cleared first:

Tile.upload main thread over 7 pans long frames (> 34 ms) in the worst pan
unpatched 3,545 ms 129 ms
patched (below) 118 to 125 ms 0 to 40 ms

The cost grows with the number of features holding state. With few of them the bug is still there but cheap.

Steps to reproduce
  1. Add a vector source and a fill layer on it.
  2. Call map.setFeatureState for a few thousand features of that source layer.
  3. After the map is idle, take a fill bucket from a loaded tile and call bucket.uploadPending(). It returns true, and bucket.uploaded is never set.
  4. Pan, and profile Tile.prototype.upload: getState runs for every fill tile on every call.
Expected

uploadPending() returns false once the bucket has uploaded and nothing needs a re-upload.

Suggested fix

Set the flag in FillBucket.upload(), as the other buckets do:

upload(context) {
  this.bufferData.upload(context);
  if (this.hdExt) {
    this.hdExt.upload(context);
  }
  this.uploaded = true;
}

Or have uploadPending() read this.bufferData.uploaded.

Workaround

Wrap FillBucket.prototype.upload at runtime so it sets this.uploaded = true after the original call. Rendered colours were unchanged with the wrap in place.

Lingua principale
TypeScript
Stelle
12.5k
Fork
2.4k
Nessuna PR unita negli ultimi 30g

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di mapbox/mapbox-gl-js

Tutte le issue di mapbox/mapbox-gl-js

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.