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

Đang mở Phù hợp với người mới
  • Có gì không ổn?
#13,752 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
82/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
typescript
Lĩnh vực
performance

Hướng nghiên cứu

Đọc các phần triển khai của FillBucket.upload() và uploadPending(), sau đó so sánh chúng với LineBucket và CircleBucket như issue mô tả. Tái hiện lỗi bằng cách kiểm tra xem fill bucket đã được tải lên có còn pending sau khi upload hay không; xác nhận bản sửa lỗi bằng cách kiểm tra rằng nó không còn pending khi không cần tải lên lại và các màu được render vẫn không thay đổi.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.

Ngôn ngữ chính
TypeScript
Star
12.5k
Fork
2.4k
Không có pull request nào được merge trong 30 ngày

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của mapbox/mapbox-gl-js

Tất cả issue của mapbox/mapbox-gl-js

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.