From 65c54429a7e0b2e340b08ebc867942edc0bb85f4 Mon Sep 17 00:00:00 2001 From: tristan-morris <44310937+tristan-morris@users.noreply.github.com> Date: Sun, 22 Oct 2023 12:55:43 +1100 Subject: [PATCH 1/3] Consistent Tile.destroy() with texture caching --- debug/pathological-flyto.html | 201 +++++++++++++++++++++++++++ src/render/draw_fill.js | 4 +- src/render/draw_fill_extrusion.js | 4 +- src/render/draw_line.js | 8 +- src/render/draw_raster.js | 12 +- src/render/draw_symbol.js | 24 ++-- src/render/painter.js | 18 ++- src/render/program/line_program.js | 4 +- src/render/program/pattern.js | 2 +- src/source/custom_source.js | 2 + src/source/geojson_source.js | 4 +- src/source/raster_dem_tile_source.js | 14 -- src/source/raster_tile_source.js | 22 ++- src/source/tile.js | 121 +++++++++++++++- src/source/vector_tile_source.js | 2 +- src/terrain/draw_terrain_raster.js | 14 +- 16 files changed, 399 insertions(+), 57 deletions(-) create mode 100644 debug/pathological-flyto.html diff --git a/debug/pathological-flyto.html b/debug/pathological-flyto.html new file mode 100644 index 00000000000..bc1c5d3c7c0 --- /dev/null +++ b/debug/pathological-flyto.html @@ -0,0 +1,201 @@ + + + + + + + Mapbox GL JS Pathological FlyTo + + + + + + + +
+ + + + + +
+
+
+
+ + + + + + + \ No newline at end of file diff --git a/src/render/draw_fill.js b/src/render/draw_fill.js index f495de34912..aa8020c27e3 100644 --- a/src/render/draw_fill.js +++ b/src/render/draw_fill.js @@ -89,7 +89,9 @@ function drawFillTiles(painter: Painter, sourceCache: SourceCache, layer: FillSt if (image) { painter.context.activeTexture.set(gl.TEXTURE0); - tile.imageAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + if (tile.imageAtlasTexture) { + tile.imageAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + } programConfiguration.updatePaintBuffers(); } diff --git a/src/render/draw_fill_extrusion.js b/src/render/draw_fill_extrusion.js index a8d30e7a4c5..2465a84e957 100644 --- a/src/render/draw_fill_extrusion.js +++ b/src/render/draw_fill_extrusion.js @@ -340,7 +340,9 @@ function drawExtrusionTiles(painter: Painter, source: SourceCache, layer: FillEx if (image) { painter.context.activeTexture.set(gl.TEXTURE0); - tile.imageAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + if (tile.imageAtlasTexture) { + tile.imageAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + } programConfiguration.updatePaintBuffers(); } const constantPattern = patternProperty.constantOr(null); diff --git a/src/render/draw_line.js b/src/render/draw_line.js index 0ab0a215653..9a88f1f1d65 100644 --- a/src/render/draw_line.js +++ b/src/render/draw_line.js @@ -139,12 +139,16 @@ export default function drawLine(painter: Painter, sourceCache: SourceCache, lay } if (dasharray) { context.activeTexture.set(gl.TEXTURE0); - tile.lineAtlasTexture.bind(gl.LINEAR, gl.REPEAT); + if (tile.lineAtlasTexture) { + tile.lineAtlasTexture.bind(gl.LINEAR, gl.REPEAT); + } programConfiguration.updatePaintBuffers(); } if (image) { context.activeTexture.set(gl.TEXTURE0); - tile.imageAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + if (tile.imageAtlasTexture) { + tile.imageAtlasTexture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + } programConfiguration.updatePaintBuffers(); } diff --git a/src/render/draw_raster.js b/src/render/draw_raster.js index 822ca505095..be01412ea62 100644 --- a/src/render/draw_raster.js +++ b/src/render/draw_raster.js @@ -149,22 +149,26 @@ function drawRaster(painter: Painter, sourceCache: SourceCache, layer: RasterSty let parentScaleBy, parentTL; context.activeTexture.set(gl.TEXTURE0); - tile.texture.bind(textureFilter, gl.CLAMP_TO_EDGE); + if (tile.texture) { + tile.texture.bind(textureFilter, gl.CLAMP_TO_EDGE); + } context.activeTexture.set(gl.TEXTURE1); if (parentTile) { - parentTile.texture.bind(textureFilter, gl.CLAMP_TO_EDGE); + if (parentTile.texture) { + parentTile.texture.bind(textureFilter, gl.CLAMP_TO_EDGE); + } parentScaleBy = Math.pow(2, parentTile.tileID.overscaledZ - tile.tileID.overscaledZ); parentTL = [tile.tileID.canonical.x * parentScaleBy % 1, tile.tileID.canonical.y * parentScaleBy % 1]; - } else { + } else if (tile.texture) { tile.texture.bind(textureFilter, gl.CLAMP_TO_EDGE); } // Enable trilinear filtering on tiles only beyond 20 degrees pitch, // to prevent it from compromising image crispness on flat or low tilted maps. - if (tile.texture.useMipmap && context.extTextureFilterAnisotropic && painter.transform.pitch > 20) { + if (tile.texture && tile.texture.useMipmap && context.extTextureFilterAnisotropic && painter.transform.pitch > 20) { gl.texParameterf(gl.TEXTURE_2D, context.extTextureFilterAnisotropic.TEXTURE_MAX_ANISOTROPY_EXT, context.extTextureFilterAnisotropicMax); } diff --git a/src/render/draw_symbol.js b/src/render/draw_symbol.js index cc2f81a5b40..5ba09143160 100644 --- a/src/render/draw_symbol.js +++ b/src/render/draw_symbol.js @@ -33,7 +33,7 @@ import type Painter from './painter.js'; import type SourceCache from '../source/source_cache.js'; import type SymbolStyleLayer from '../style/style_layer/symbol_style_layer.js'; import type SymbolBucket, {SymbolBuffers} from '../data/bucket/symbol_bucket.js'; -import type Texture from '../render/texture.js'; +import Texture from '../render/texture.js'; import type ColorMode from '../gl/color_mode.js'; import {OverscaledTileID} from '../source/tile_id.js'; import type {UniformValues} from './uniform_binding.js'; @@ -49,7 +49,7 @@ type SymbolTileRenderState = { program: any, buffers: SymbolBuffers, uniformValues: any, - atlasTexture: Texture, + atlasTexture: Texture | null, atlasTextureIcon: Texture | null, atlasInterpolation: any, atlasInterpolationIcon: any, @@ -327,27 +327,27 @@ function drawLayerSymbols(painter: Painter, sourceCache: SourceCache, layer: Sym let texSize: [number, number]; let texSizeIcon: [number, number] = [0, 0]; - let atlasTexture; + let atlasTexture: Texture | null; let atlasInterpolation; - let atlasTextureIcon = null; + let atlasTextureIcon: Texture | null = null; let atlasInterpolationIcon; if (isText) { - atlasTexture = tile.glyphAtlasTexture; + atlasTexture = tile.glyphAtlasTexture ? tile.glyphAtlasTexture : null; atlasInterpolation = gl.LINEAR; - texSize = tile.glyphAtlasTexture.size; + texSize = tile.glyphAtlasTexture ? tile.glyphAtlasTexture.size : [0, 0]; if (bucket.iconsInText) { - texSizeIcon = tile.imageAtlasTexture.size; - atlasTextureIcon = tile.imageAtlasTexture; + texSizeIcon = tile.imageAtlasTexture ? tile.imageAtlasTexture.size : [0, 0]; + atlasTextureIcon = tile.imageAtlasTexture ? tile.imageAtlasTexture : null; const zoomDependentSize = sizeData.kind === 'composite' || sizeData.kind === 'camera'; atlasInterpolationIcon = transformed || painter.options.rotating || painter.options.zooming || zoomDependentSize ? gl.LINEAR : gl.NEAREST; } } else { const iconScaled = layer.layout.get('icon-size').constantOr(0) !== 1 || bucket.iconsNeedLinear; - atlasTexture = tile.imageAtlasTexture; + atlasTexture = tile.imageAtlasTexture ? tile.imageAtlasTexture : null; atlasInterpolation = isSDF || painter.options.rotating || painter.options.zooming || iconScaled || transformed ? gl.LINEAR : gl.NEAREST; - texSize = tile.imageAtlasTexture.size; + texSize = tile.imageAtlasTexture ? tile.imageAtlasTexture.size : [0, 0]; } const bucketIsGlobeProjection = bucket.projection.name === 'globe'; @@ -463,7 +463,9 @@ function drawLayerSymbols(painter: Painter, sourceCache: SourceCache, layer: Sym painter.terrain.setupElevationDraw(state.tile, state.program, options); } context.activeTexture.set(gl.TEXTURE0); - state.atlasTexture.bind(state.atlasInterpolation, gl.CLAMP_TO_EDGE); + if (state.atlasTexture) { + state.atlasTexture.bind(state.atlasInterpolation, gl.CLAMP_TO_EDGE); + } if (state.atlasTextureIcon) { context.activeTexture.set(gl.TEXTURE1); if (state.atlasTextureIcon) { diff --git a/src/render/painter.js b/src/render/painter.js index 54bd352f51b..33eb2a0ce7f 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -1114,13 +1114,25 @@ class Painter { return translatedMatrix; } - saveTileTexture(texture: Texture) { - const textures = this._tileTextures[texture.size[0]]; + /** + * Saves the tile texture for re-use when another tile is loaded. + * + * @returns true if the tile was cached, false if the tile was not cached and should be destroyed. + * @private + */ + saveTileTexture(texture: Texture): boolean { + const tileSize = texture.size[0]; + const textures = this._tileTextures[tileSize]; if (!textures) { - this._tileTextures[texture.size[0]] = [texture]; + this._tileTextures[tileSize] = [texture]; } else { + if (textures.length >= 64) { + return false; + } textures.push(texture); } + + return true; } getTileTexture(size: number): null | Texture { diff --git a/src/render/program/line_program.js b/src/render/program/line_program.js index 46a0bf55ca8..887068a1e18 100644 --- a/src/render/program/line_program.js +++ b/src/render/program/line_program.js @@ -93,7 +93,7 @@ const lineUniformValues = ( 'u_dash_image': 0, 'u_gradient_image': 1, 'u_image_height': imageHeight, - 'u_texsize': hasDash(layer) ? tile.lineAtlasTexture.size : [0, 0], + 'u_texsize': hasDash(layer) ? (tile.lineAtlasTexture ? tile.lineAtlasTexture.size : [0, 0]) : [0, 0], 'u_tile_units_to_pixels': calculateTileRatio(tile, painter.transform), 'u_alpha_discard_threshold': 0.0, 'u_trim_offset': trimOffset, @@ -111,7 +111,7 @@ const linePatternUniformValues = ( const transform = painter.transform; return { 'u_matrix': calculateMatrix(painter, tile, layer, matrix), - 'u_texsize': tile.imageAtlasTexture.size, + 'u_texsize': tile.imageAtlasTexture ? tile.imageAtlasTexture.size : [0, 0], // camera zoom ratio 'u_pixels_to_tile_units': transform.calculatePixelsToTileUnitsMatrix(tile), 'u_device_pixel_ratio': pixelRatio, diff --git a/src/render/program/pattern.js b/src/render/program/pattern.js index fd29eb2605f..f366fe628c9 100644 --- a/src/render/program/pattern.js +++ b/src/render/program/pattern.js @@ -43,7 +43,7 @@ function patternUniformValues(painter: Painter, tile: Tile): UniformValues> 16, pixelY >> 16], diff --git a/src/source/custom_source.js b/src/source/custom_source.js index 289f1a2299e..ccd11459b51 100644 --- a/src/source/custom_source.js +++ b/src/source/custom_source.js @@ -327,6 +327,8 @@ class CustomSource extends Evented implements Source { this._implementation.unloadTile({x, y, z}); } + tile.destroy(); + callback(); } diff --git a/src/source/geojson_source.js b/src/source/geojson_source.js index a51c95e9b53..0c4ce939b9e 100644 --- a/src/source/geojson_source.js +++ b/src/source/geojson_source.js @@ -368,7 +368,7 @@ class GeoJSONSource extends Evented implements Source { tile.request = this.actor.send(message, params, (err, data) => { delete tile.request; - tile.unloadVectorData(); + tile.destroy(); if (tile.aborted) { return callback(null); @@ -395,8 +395,8 @@ class GeoJSONSource extends Evented implements Source { // $FlowFixMe[method-unbinding] unloadTile(tile: Tile) { - tile.unloadVectorData(); this.actor.send('removeTile', {uid: tile.uid, type: this.type, source: this.id, scope: this.scope}); + tile.destroy(); } // $FlowFixMe[method-unbinding] diff --git a/src/source/raster_dem_tile_source.js b/src/source/raster_dem_tile_source.js index 67e7515d294..6b8e10842bf 100644 --- a/src/source/raster_dem_tile_source.js +++ b/src/source/raster_dem_tile_source.js @@ -124,20 +124,6 @@ class RasterDEMTileSource extends RasterTileSource implements Source { return neighboringTiles; } - - // $FlowFixMe[method-unbinding] - unloadTile(tile: Tile) { - if (tile.demTexture) this.map.painter.saveTileTexture(tile.demTexture); - if (tile.hillshadeFBO) { - tile.hillshadeFBO.destroy(); - delete tile.hillshadeFBO; - } - if (tile.dem) delete tile.dem; - delete tile.neighboringTiles; - - tile.state = 'unloaded'; - } - } export default RasterDEMTileSource; diff --git a/src/source/raster_tile_source.js b/src/source/raster_tile_source.js index 5fa11c3f6d3..0b7d10f7a3f 100644 --- a/src/source/raster_tile_source.js +++ b/src/source/raster_tile_source.js @@ -19,11 +19,12 @@ import type Dispatcher from '../util/dispatcher.js'; import type Tile from './tile.js'; import type {Callback} from '../types/callback.js'; import type {Cancelable} from '../types/cancelable.js'; -import type {TextureImage} from '../render/texture.js'; +import type {TextureImage} from "../render/texture.js"; import type { RasterSourceSpecification, RasterDEMSourceSpecification } from '../style-spec/types.js'; +import Texture from '../render/texture.js'; /** * A source containing raster tiles. @@ -221,10 +222,9 @@ class RasterTileSource extends Evented implements Source { tile.setTexture(data, painter); } + // eslint-disable-next-line no-unused-vars static unloadTileData(tile: Tile, painter: Painter) { - if (tile.texture) { - painter.saveTileTexture(tile.texture); - } + // Texture caching on unload occurs in unloadTile } // $FlowFixMe[method-unbinding] @@ -238,7 +238,19 @@ class RasterTileSource extends Evented implements Source { // $FlowFixMe[method-unbinding] unloadTile(tile: Tile, callback: Callback) { - if (tile.texture) this.map.painter.saveTileTexture(tile.texture); + // Cache the tile texture to avoid re-allocating Textures if they'll just be reloaded + if (tile.texture && tile.texture instanceof Texture) { + + // Try and save the texture to the cache. If the cache is full, + // then the texture should also be destroyed + const wasTextureCached = this.map.painter.saveTileTexture(tile.texture); + + // Single call to destroy, to prevent other allocations not being cleaned up (fbo). + tile.destroy(wasTextureCached); + } else { + tile.destroy(); + } + callback(); } diff --git a/src/source/tile.js b/src/source/tile.js index 91a3c7984da..df1701fe950 100644 --- a/src/source/tile.js +++ b/src/source/tile.js @@ -56,7 +56,7 @@ import type {QueryResult} from '../data/feature_index.js'; import type Painter from '../render/painter.js'; import type {QueryFeature} from '../util/vectortile_to_geojson.js'; import type {Vec3} from 'gl-matrix'; -import type {TextureImage} from '../render/texture.js'; +import type {UserManagedTexture, TextureImage} from '../render/texture.js'; import type {VectorTileLayer} from '@mapbox/vector-tile'; const CLOCK_SKEW_RETRY_TIMEOUT = 30000; @@ -102,11 +102,11 @@ class Tile { latestFeatureIndex: ?FeatureIndex; latestRawTileData: ?ArrayBuffer; imageAtlas: ?ImageAtlas; - imageAtlasTexture: Texture; + imageAtlasTexture: ?Texture; lineAtlas: ?LineAtlas; - lineAtlasTexture: Texture; + lineAtlasTexture: ?Texture; glyphAtlasImage: ?AlphaImage; - glyphAtlasTexture: Texture; + glyphAtlasTexture: ?Texture; expirationTime: any; expiredRequestCount: number; state: TileState; @@ -129,7 +129,7 @@ class Tile { needsHillshadePrepare: ?boolean; needsDEMTextureUpload: ?boolean; request: ?Cancelable; - texture: any; + texture: ?Texture | ?UserManagedTexture; hillshadeFBO: ?Framebuffer; demTexture: ?Texture; refreshedUponExpiration: boolean; @@ -407,7 +407,7 @@ class Tile { } prepare(imageManager: ImageManager, painter: ?Painter, scope: string) { - if (this.imageAtlas) { + if (this.imageAtlas && this.imageAtlasTexture) { this.imageAtlas.patchUpdatedImages(imageManager, this.imageAtlasTexture, scope); } @@ -639,7 +639,7 @@ class Tile { const context = painter.context; const gl = context.gl; this.texture = this.texture || painter.getTileTexture(img.width); - if (this.texture) { + if (this.texture && this.texture instanceof Texture) { this.texture.update(img, {useMipmap: true}); } else { this.texture = new Texture(context, img, gl.RGBA, {useMipmap: true}); @@ -871,6 +871,113 @@ class Tile { this._globeTileDebugTextBuffer = context.createVertexBuffer(extraGlobe, posAttributesGlobeExt.members); this._tileDebugTextSegments = SegmentVector.simpleSegment(0, 0, totalVertices, totalTriangles); } + + /** + * Release data and WebGL resources referenced by this tile. + * @returns {undefined} + * @private + */ + destroy(preserveTexture: boolean = false) { + for (const id in this.buckets) { + this.buckets[id].destroy(); + } + + this.buckets = {}; + + if (this.imageAtlas) { + this.imageAtlas = null; + } + + if (this.lineAtlas) { + this.lineAtlas = null; + } + + if (this.imageAtlasTexture) { + this.imageAtlasTexture.destroy(); + delete this.imageAtlasTexture; + } + + if (this.glyphAtlasTexture) { + this.glyphAtlasTexture.destroy(); + delete this.glyphAtlasTexture; + } + + if (this.lineAtlasTexture) { + this.lineAtlasTexture.destroy(); + delete this.lineAtlasTexture; + } + + if (this._tileBoundsBuffer) { + this._tileBoundsBuffer.destroy(); + this._tileBoundsIndexBuffer.destroy(); + this._tileBoundsSegments.destroy(); + this._tileBoundsBuffer = null; + } + + if (this._tileDebugBuffer) { + this._tileDebugBuffer.destroy(); + this._tileDebugSegments.destroy(); + this._tileDebugBuffer = null; + } + + if (this._tileDebugIndexBuffer) { + this._tileDebugIndexBuffer.destroy(); + this._tileDebugIndexBuffer = null; + } + + if (this._globeTileDebugBorderBuffer) { + this._globeTileDebugBorderBuffer.destroy(); + this._globeTileDebugBorderBuffer = null; + } + + if (this._tileDebugTextBuffer) { + this._tileDebugTextBuffer.destroy(); + this._tileDebugTextSegments.destroy(); + this._tileDebugTextIndexBuffer.destroy(); + this._tileDebugTextBuffer = null; + } + + if (this._globeTileDebugTextBuffer) { + this._globeTileDebugTextBuffer.destroy(); + this._globeTileDebugTextBuffer = null; + } + + if (!preserveTexture && this.texture && this.texture instanceof Texture) { + this.texture.destroy(); + delete this.texture; + } + + if (this.hillshadeFBO) { + this.hillshadeFBO.destroy(); + delete this.hillshadeFBO; + } + + if (this.dem) { + delete this.dem; + } + + if (this.neighboringTiles) { + delete this.neighboringTiles; + } + + if (this.demTexture) { + this.demTexture.destroy(); + delete this.demTexture; + } + + Debug.run(() => { + if (this.queryGeometryDebugViz) { + this.queryGeometryDebugViz.unload(); + delete this.queryGeometryDebugViz; + } + if (this.queryBoundsDebugViz) { + this.queryBoundsDebugViz.unload(); + delete this.queryBoundsDebugViz; + } + }); + this.latestFeatureIndex = null; + this.state = 'unloaded'; + } } export default Tile; diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js index 7a42b534e42..73265fb1a6b 100644 --- a/src/source/vector_tile_source.js +++ b/src/source/vector_tile_source.js @@ -312,10 +312,10 @@ class VectorTileSource extends Evented implements Source { // $FlowFixMe[method-unbinding] unloadTile(tile: Tile) { - tile.unloadVectorData(); if (tile.actor) { tile.actor.send('removeTile', {uid: tile.uid, type: this.type, source: this.id, scope: this.scope}); } + tile.destroy(); } hasTransition(): boolean { diff --git a/src/terrain/draw_terrain_raster.js b/src/terrain/draw_terrain_raster.js index a6c7eea4fe2..197942712e0 100644 --- a/src/terrain/draw_terrain_raster.js +++ b/src/terrain/draw_terrain_raster.js @@ -191,7 +191,9 @@ function drawTerrainForGlobe(painter: Painter, terrain: Terrain, sourceCache: So // Bind the main draped texture context.activeTexture.set(gl.TEXTURE0); - tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + if (tile.texture) { + tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + } const morph = vertexMorphing.getMorphValuesForProxy(coord.key); const shaderMode = morph ? SHADER_MORPHING : SHADER_DEFAULT; @@ -246,7 +248,9 @@ function drawTerrainForGlobe(painter: Painter, terrain: Terrain, sourceCache: So // Bind the main draped texture context.activeTexture.set(gl.TEXTURE0); - tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + if (tile.texture) { + tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + } let poleMatrix = globePoleMatrixForTile(z, x, tr); const normalizeMatrix = globeNormalizeECEF(globeTileBounds(coord.canonical)); @@ -331,7 +335,11 @@ function drawTerrainRaster(painter: Painter, terrain: Terrain, sourceCache: Sour // Bind the main draped texture context.activeTexture.set(gl.TEXTURE0); - tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE, gl.LINEAR_MIPMAP_NEAREST); + if (tile.texture) { + // This call seemed invalid with 3 args? + // tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE, gl.LINEAR_MIPMAP_NEAREST); + tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); + } const morph = vertexMorphing.getMorphValuesForProxy(coord.key); const shaderMode = morph ? SHADER_MORPHING : SHADER_DEFAULT; From 2c968b847399d68a832836205a3acee444b31745 Mon Sep 17 00:00:00 2001 From: tristan-morris <44310937+tristan-morris@users.noreply.github.com> Date: Sun, 22 Oct 2023 14:25:36 +1100 Subject: [PATCH 2/3] Fix up potential race --- src/render/painter.js | 7 +------ src/source/raster_tile_source.js | 13 +++++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/render/painter.js b/src/render/painter.js index 33eb2a0ce7f..9e0205e494e 100644 --- a/src/render/painter.js +++ b/src/render/painter.js @@ -1120,19 +1120,14 @@ class Painter { * @returns true if the tile was cached, false if the tile was not cached and should be destroyed. * @private */ - saveTileTexture(texture: Texture): boolean { + saveTileTexture(texture: Texture) { const tileSize = texture.size[0]; const textures = this._tileTextures[tileSize]; if (!textures) { this._tileTextures[tileSize] = [texture]; } else { - if (textures.length >= 64) { - return false; - } textures.push(texture); } - - return true; } getTileTexture(size: number): null | Texture { diff --git a/src/source/raster_tile_source.js b/src/source/raster_tile_source.js index 0b7d10f7a3f..d3ff5412f8e 100644 --- a/src/source/raster_tile_source.js +++ b/src/source/raster_tile_source.js @@ -240,13 +240,14 @@ class RasterTileSource extends Evented implements Source { unloadTile(tile: Tile, callback: Callback) { // Cache the tile texture to avoid re-allocating Textures if they'll just be reloaded if (tile.texture && tile.texture instanceof Texture) { + // Clean everything else up owned by the tile, but preserve the texture. + // Destroy first to prevent racing with the texture cache being popped. + tile.destroy(true); - // Try and save the texture to the cache. If the cache is full, - // then the texture should also be destroyed - const wasTextureCached = this.map.painter.saveTileTexture(tile.texture); - - // Single call to destroy, to prevent other allocations not being cleaned up (fbo). - tile.destroy(wasTextureCached); + // Save the texture to the cache + if (tile.texture && tile.texture instanceof Texture) { + this.map.painter.saveTileTexture(tile.texture); + } } else { tile.destroy(); } From e0b0e8ebcc40896b2b92e0e9c558ebb509f4cff8 Mon Sep 17 00:00:00 2001 From: Tristan <44310937+tristan-morris@users.noreply.github.com> Date: Wed, 1 Nov 2023 10:23:34 +1100 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Volodymyr Agafonkin --- src/render/program/line_program.js | 2 +- src/terrain/draw_terrain_raster.js | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/render/program/line_program.js b/src/render/program/line_program.js index 887068a1e18..e92aaf587d3 100644 --- a/src/render/program/line_program.js +++ b/src/render/program/line_program.js @@ -93,7 +93,7 @@ const lineUniformValues = ( 'u_dash_image': 0, 'u_gradient_image': 1, 'u_image_height': imageHeight, - 'u_texsize': hasDash(layer) ? (tile.lineAtlasTexture ? tile.lineAtlasTexture.size : [0, 0]) : [0, 0], + 'u_texsize': hasDash(layer) && tile.lineAtlasTexture ? tile.lineAtlasTexture.size : [0, 0], 'u_tile_units_to_pixels': calculateTileRatio(tile, painter.transform), 'u_alpha_discard_threshold': 0.0, 'u_trim_offset': trimOffset, diff --git a/src/terrain/draw_terrain_raster.js b/src/terrain/draw_terrain_raster.js index 197942712e0..6fcef850582 100644 --- a/src/terrain/draw_terrain_raster.js +++ b/src/terrain/draw_terrain_raster.js @@ -336,8 +336,6 @@ function drawTerrainRaster(painter: Painter, terrain: Terrain, sourceCache: Sour // Bind the main draped texture context.activeTexture.set(gl.TEXTURE0); if (tile.texture) { - // This call seemed invalid with 3 args? - // tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE, gl.LINEAR_MIPMAP_NEAREST); tile.texture.bind(gl.LINEAR, gl.CLAMP_TO_EDGE); }