From b850b1a3d798f4617f78e4d1137bcd253dcf13ae Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Thu, 20 Dec 2018 19:47:29 +0100 Subject: [PATCH 1/3] Fix RasterSource cache: - In createTexture() Textures that may not be added to the cache laster (in getRaster) held a deleter which removed their tileId from the cache. - Instead now it clears the cache by checking whether the cache is the only one holding references to a texture in getRaster() --- core/src/data/rasterSource.cpp | 44 ++++++++++++++++------------------ core/src/data/rasterSource.h | 6 ++--- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/core/src/data/rasterSource.cpp b/core/src/data/rasterSource.cpp index c7f086e6dc..d7be54e338 100644 --- a/core/src/data/rasterSource.cpp +++ b/core/src/data/rasterSource.cpp @@ -4,7 +4,7 @@ #include "tile/tile.h" #include "tile/tileTask.h" #include "util/mapProjection.h" -#include "platform.h" +#include "log.h" namespace Tangram { @@ -77,7 +77,6 @@ RasterSource::RasterSource(const std::string& _name, std::unique_ptr TextureOptions _options, TileSource::ZoomOptions _zoomOptions) : TileSource(_name, std::move(_sources), _zoomOptions), m_texOptions(_options) { - m_textures = std::make_shared(); m_emptyTexture = std::make_shared(m_texOptions); } @@ -90,13 +89,7 @@ std::shared_ptr RasterSource::createTexture(TileID _tile, const std::ve auto data = reinterpret_cast(_rawTileData.data()); auto length = _rawTileData.size(); - std::shared_ptr texture(new Texture(data, length, m_texOptions), - [c = std::weak_ptr(m_textures), _tile](auto t) { - if (auto cache = c.lock()) { cache->erase(_tile); } - delete t; - }); - - return texture; + return std::make_shared(data, length, m_texOptions); } void RasterSource::loadTileData(std::shared_ptr _task, TileTaskCb _cb) { @@ -143,9 +136,9 @@ std::shared_ptr RasterSource::createTask(TileID _tileId, int _subTask) // First try existing textures cache TileID id(_tileId.x, _tileId.y, _tileId.z); - auto texIt = m_textures->find(id); - if (texIt != m_textures->end()) { - task->m_texture = texIt->second.lock(); + auto texIt = m_textures.find(id); + if (texIt != m_textures.end()) { + task->m_texture = texIt->second; if (task->m_texture) { // No more loading needed. @@ -157,20 +150,25 @@ std::shared_ptr RasterSource::createTask(TileID _tileId, int _subTask) return task; } -Raster RasterSource::getRaster(const TileTask& _task) { - const auto& taskTileID = _task.tileId(); - TileID id(taskTileID.x, taskTileID.y, taskTileID.z); +Raster RasterSource::getRaster(const RasterTileTask& _task) { + const auto& tileId = _task.tileId(); + TileID id(tileId.x, tileId.y, tileId.z); - auto texIt = m_textures->find(id); - if (texIt != m_textures->end()) { - auto&& texture = texIt->second.lock(); - return { id, texture }; - } + auto entry = m_textures.emplace(id, _task.m_texture); + std::shared_ptr texture = entry.first->second; + LOGD("Cache: add %d/%d/%d - reused: %d", id.x, id.y, id.z, !entry.second); - auto& task = static_cast(_task); - m_textures->emplace(id, task.m_texture); + // Remove textures that are only held by cache + for(auto it = m_textures.begin(), end = m_textures.end(); it != end; ) { + if (it->second.use_count() == 1) { + LOGD("Cache: remove %d/%d/%d", it->first.x, it->first.y, it->first.z); + it = m_textures.erase(it); + } else { + ++it; + } + } - return { id, task.m_texture }; + return Raster{id, texture}; } } diff --git a/core/src/data/rasterSource.h b/core/src/data/rasterSource.h index 9aa9bb20da..f629785114 100644 --- a/core/src/data/rasterSource.h +++ b/core/src/data/rasterSource.h @@ -15,8 +15,8 @@ class RasterTileTask; class RasterSource : public TileSource { - using Cache = std::map>; - std::shared_ptr m_textures; + using Cache = std::map>; + Cache m_textures; TextureOptions m_texOptions; @@ -42,7 +42,7 @@ class RasterSource : public TileSource { std::shared_ptr createTexture(TileID _tile, const std::vector& _rawTileData); - Raster getRaster(const TileTask& _task); + Raster getRaster(const RasterTileTask& _task); }; From 4301ec02b27a73c5230cee565c410a46c17b58fc Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Thu, 20 Dec 2018 06:45:31 +0100 Subject: [PATCH 2/3] Rasters must be valid - asserted in RasterSource - when tile->rasters are empty uniforms should be set accordingly --- core/src/style/style.cpp | 41 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/core/src/style/style.cpp b/core/src/style/style.cpp index 644c0cf41e..d001cc8a0b 100644 --- a/core/src/style/style.cpp +++ b/core/src/style/style.cpp @@ -438,33 +438,32 @@ bool Style::draw(RenderState& rs, const Tile& _tile) { bool styleMeshDrawn = true; TileID tileID = _tile.getID(); - if (hasRasters() && !_tile.rasters().empty()) { + if (hasRasters()) { UniformTextureArray textureIndexUniform; UniformArray2f rasterSizeUniform; UniformArray3f rasterOffsetsUniform; for (auto& raster : _tile.rasters()) { - if (raster.isValid()) { - auto& texture = raster.texture; - auto texUnit = rs.nextAvailableTextureUnit(); - texture->bind(rs, texUnit); - - textureIndexUniform.slots.push_back(texUnit); - rasterSizeUniform.push_back({texture->width(), texture->height()}); - - if (tileID.z > raster.tileID.z) { - float dz = tileID.z - raster.tileID.z; - float dz2 = powf(2.f, dz); - - rasterOffsetsUniform.push_back({ - fmodf(tileID.x, dz2) / dz2, - (dz2 - 1.f - fmodf(tileID.y, dz2)) / dz2, - 1.f / dz2 - }); - } else { - rasterOffsetsUniform.push_back({0, 0, 1}); - } + + auto& texture = raster.texture; + auto texUnit = rs.nextAvailableTextureUnit(); + texture->bind(rs, texUnit); + + textureIndexUniform.slots.push_back(texUnit); + rasterSizeUniform.push_back({texture->width(), texture->height()}); + + float x = 0.f; + float y = 0.f; + float z = 1.f; + + if (tileID.z > raster.tileID.z) { + float dz = tileID.z - raster.tileID.z; + float dz2 = powf(2.f, dz); + x = fmodf(tileID.x, dz2) / dz2; + y = (dz2 - 1.f - fmodf(tileID.y, dz2)) / dz2; + z = 1.f / dz2; } + rasterOffsetsUniform.emplace_back(x, y, z); } m_shaderProgram->setUniformi(rs, m_mainUniforms.uRasters, textureIndexUniform); From 90bf6230fc02a0d5febc703b77ea0f4df5325392 Mon Sep 17 00:00:00 2001 From: Hannes Janetzek Date: Thu, 20 Dec 2018 20:12:21 +0100 Subject: [PATCH 3/3] Simple fix to not have orphaned cache entries for emptyTexture --- core/src/data/rasterSource.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/data/rasterSource.cpp b/core/src/data/rasterSource.cpp index d7be54e338..08fd454f85 100644 --- a/core/src/data/rasterSource.cpp +++ b/core/src/data/rasterSource.cpp @@ -154,6 +154,10 @@ Raster RasterSource::getRaster(const RasterTileTask& _task) { const auto& tileId = _task.tileId(); TileID id(tileId.x, tileId.y, tileId.z); + if (_task.m_texture == m_emptyTexture) { + return Raster{id, m_emptyTexture}; + } + auto entry = m_textures.emplace(id, _task.m_texture); std::shared_ptr texture = entry.first->second; LOGD("Cache: add %d/%d/%d - reused: %d", id.x, id.y, id.z, !entry.second);