From 953ee5e81bee613680468438e3efe4f7c3ff68f6 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Fri, 10 May 2019 22:42:48 +0300 Subject: [PATCH] [core] Create glyph manager at renderer construction time Avoid unnecessary glyph manager presence check in `Renderer::Impl::()`. --- src/mbgl/renderer/renderer_impl.cpp | 7 ++----- src/mbgl/text/glyph_manager.cpp | 11 +++++------ src/mbgl/text/glyph_manager.hpp | 19 ++++++++++--------- src/mbgl/tile/geometry_tile.cpp | 3 ++- src/mbgl/tile/geometry_tile.hpp | 1 + test/style/source.test.cpp | 2 +- test/text/glyph_manager.test.cpp | 8 ++++---- test/tile/custom_geometry_tile.test.cpp | 2 +- test/tile/geojson_tile.test.cpp | 2 +- test/tile/raster_dem_tile.test.cpp | 2 +- test/tile/raster_tile.test.cpp | 2 +- test/tile/vector_tile.test.cpp | 4 ++-- 12 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp index 3db9439b5..b14a25502 100644 --- a/src/mbgl/renderer/renderer_impl.cpp +++ b/src/mbgl/renderer/renderer_impl.cpp @@ -47,6 +47,7 @@ Renderer::Impl::Impl(gfx::RendererBackend& backend_, , pixelRatio(pixelRatio_) , programCacheDir(std::move(programCacheDir_)) , localFontFamily(std::move(localFontFamily_)) + , glyphManager(std::make_unique(std::make_unique(localFontFamily))) , imageManager(std::make_unique()) , lineAtlas(std::make_unique(Size{ 256, 512 })) , imageImpls(makeMutable>>()) @@ -54,6 +55,7 @@ Renderer::Impl::Impl(gfx::RendererBackend& backend_, , layerImpls(makeMutable>>()) , renderLight(makeMutable()) , placement(std::make_unique(TransformState{}, MapMode::Static, TransitionOptions{}, true)) { + glyphManager->setObserver(this); imageManager->setObserver(this); } @@ -77,12 +79,7 @@ void Renderer::Impl::setObserver(RendererObserver* observer_) { } void Renderer::Impl::render(const UpdateParameters& updateParameters) { - if (!glyphManager) { - glyphManager = std::make_unique(updateParameters.fileSource, std::make_unique(localFontFamily)); - glyphManager->setObserver(this); - } const bool isMapModeContinuous = updateParameters.mode == MapMode::Continuous; - if (!isMapModeContinuous) { // Reset zoom history state. zoomHistory.first = true; diff --git a/src/mbgl/text/glyph_manager.cpp b/src/mbgl/text/glyph_manager.cpp index b947ef72c..daa142e38 100644 --- a/src/mbgl/text/glyph_manager.cpp +++ b/src/mbgl/text/glyph_manager.cpp @@ -11,15 +11,14 @@ namespace mbgl { static GlyphManagerObserver nullObserver; -GlyphManager::GlyphManager(FileSource& fileSource_, std::unique_ptr localGlyphRasterizer_) - : fileSource(fileSource_), - observer(&nullObserver), +GlyphManager::GlyphManager(std::unique_ptr localGlyphRasterizer_) + : observer(&nullObserver), localGlyphRasterizer(std::move(localGlyphRasterizer_)) { } GlyphManager::~GlyphManager() = default; -void GlyphManager::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphDependencies) { +void GlyphManager::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphDependencies, FileSource& fileSource) { auto dependencies = std::make_shared(std::move(glyphDependencies)); // Figure out which glyph ranges need to be fetched. For each range that does need to @@ -47,7 +46,7 @@ void GlyphManager::getGlyphs(GlyphRequestor& requestor, GlyphDependencies glyphD if (it == entry.ranges.end() || !it->second.parsed) { GlyphRequest& request = entry.ranges[range]; request.requestors[&requestor] = dependencies; - requestRange(request, fontStack, range); + requestRange(request, fontStack, range, fileSource); } } } @@ -65,7 +64,7 @@ Glyph GlyphManager::generateLocalSDF(const FontStack& fontStack, GlyphID glyphID return local; } -void GlyphManager::requestRange(GlyphRequest& request, const FontStack& fontStack, const GlyphRange& range) { +void GlyphManager::requestRange(GlyphRequest& request, const FontStack& fontStack, const GlyphRange& range, FileSource& fileSource) { if (request.req) { return; } diff --git a/src/mbgl/text/glyph_manager.hpp b/src/mbgl/text/glyph_manager.hpp index 831d84719..8603a320d 100644 --- a/src/mbgl/text/glyph_manager.hpp +++ b/src/mbgl/text/glyph_manager.hpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include @@ -19,21 +18,25 @@ class Response; class GlyphRequestor { public: - virtual ~GlyphRequestor() = default; virtual void onGlyphsAvailable(GlyphMap) = 0; + +protected: + virtual ~GlyphRequestor() = default; }; -class GlyphManager : public util::noncopyable { +class GlyphManager { public: - GlyphManager(FileSource&, std::unique_ptr = std::make_unique(optional())); + GlyphManager(const GlyphManager&) = delete; + GlyphManager& operator=(const GlyphManager&) = delete; + explicit GlyphManager(std::unique_ptr = std::make_unique(optional())); ~GlyphManager(); // Workers send a `getGlyphs` message to the main thread once they have determined // their `GlyphDependencies`. If all glyphs are already locally available, GlyphManager // will provide them to the requestor immediately. Otherwise, it makes a request on the - // FileSource is made for each range neeed, and notifies the observer when all are + // FileSource is made for each range needed, and notifies the observer when all are // complete. - void getGlyphs(GlyphRequestor&, GlyphDependencies); + void getGlyphs(GlyphRequestor&, GlyphDependencies, FileSource&); void removeRequestor(GlyphRequestor&); void setURL(const std::string& url) { @@ -47,8 +50,6 @@ public: private: Glyph generateLocalSDF(const FontStack& fontStack, GlyphID glyphID); - - FileSource& fileSource; std::string glyphURL; struct GlyphRequest { @@ -64,7 +65,7 @@ private: std::unordered_map entries; - void requestRange(GlyphRequest&, const FontStack&, const GlyphRange&); + void requestRange(GlyphRequest&, const FontStack&, const GlyphRange&, FileSource& fileSource); void processResponse(const Response&, const FontStack&, const GlyphRange&); void notify(GlyphRequestor&, const GlyphDependencies&); diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index d110b2f82..305771217 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -54,6 +54,7 @@ GeometryTile::GeometryTile(const OverscaledTileID& id_, parameters.mode, parameters.pixelRatio, parameters.debugOptions & MapDebugOptions::Collision), + fileSource(parameters.fileSource), glyphManager(parameters.glyphManager), imageManager(parameters.imageManager), mode(parameters.mode), @@ -156,7 +157,7 @@ void GeometryTile::onGlyphsAvailable(GlyphMap glyphs) { } void GeometryTile::getGlyphs(GlyphDependencies glyphDependencies) { - glyphManager.getGlyphs(*this, std::move(glyphDependencies)); + glyphManager.getGlyphs(*this, std::move(glyphDependencies), fileSource); } void GeometryTile::onImagesAvailable(ImageMap images, ImageMap patterns, ImageVersionMap versionMap, uint64_t imageCorrelationID) { diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 975c7aab0..fadc0aab4 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -108,6 +108,7 @@ private: std::shared_ptr mailbox; Actor worker; + FileSource& fileSource; GlyphManager& glyphManager; ImageManager& imageManager; diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index 289e41c75..77a944049 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -57,7 +57,7 @@ public: Style style { fileSource, 1 }; AnnotationManager annotationManager { style }; ImageManager imageManager; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; TileParameters tileParameters { 1.0, diff --git a/test/text/glyph_manager.test.cpp b/test/text/glyph_manager.test.cpp index 3dcf1e204..119378063 100644 --- a/test/text/glyph_manager.test.cpp +++ b/test/text/glyph_manager.test.cpp @@ -68,7 +68,7 @@ public: StubFileSource fileSource; StubGlyphManagerObserver observer; StubGlyphRequestor requestor; - GlyphManager glyphManager{ fileSource, std::make_unique() }; + GlyphManager glyphManager{ std::make_unique() }; void run(const std::string& url, GlyphDependencies dependencies) { // Squelch logging. @@ -76,7 +76,7 @@ public: glyphManager.setURL(url); glyphManager.setObserver(&observer); - glyphManager.getGlyphs(requestor, std::move(dependencies)); + glyphManager.getGlyphs(requestor, std::move(dependencies), fileSource); loop.run(); } @@ -298,7 +298,7 @@ TEST(GlyphManager, ImmediateFileSource) { StubFileSource fileSource = { StubFileSource::ResponseType::Synchronous }; StubGlyphManagerObserver observer; StubGlyphRequestor requestor; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; void run(const std::string& url, GlyphDependencies dependencies) { // Squelch logging. @@ -306,7 +306,7 @@ TEST(GlyphManager, ImmediateFileSource) { glyphManager.setURL(url); glyphManager.setObserver(&observer); - glyphManager.getGlyphs(requestor, std::move(dependencies)); + glyphManager.getGlyphs(requestor, std::move(dependencies), fileSource); loop.run(); } diff --git a/test/tile/custom_geometry_tile.test.cpp b/test/tile/custom_geometry_tile.test.cpp index 50bc7d0e8..39f6e33ca 100644 --- a/test/tile/custom_geometry_tile.test.cpp +++ b/test/tile/custom_geometry_tile.test.cpp @@ -28,7 +28,7 @@ public: style::Style style { fileSource, 1 }; AnnotationManager annotationManager { style }; ImageManager imageManager; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; TileParameters tileParameters { 1.0, diff --git a/test/tile/geojson_tile.test.cpp b/test/tile/geojson_tile.test.cpp index 4f4435182..75e9ccbc7 100644 --- a/test/tile/geojson_tile.test.cpp +++ b/test/tile/geojson_tile.test.cpp @@ -27,7 +27,7 @@ public: style::Style style { fileSource, 1 }; AnnotationManager annotationManager { style }; ImageManager imageManager; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { diff --git a/test/tile/raster_dem_tile.test.cpp b/test/tile/raster_dem_tile.test.cpp index e2e7d44b9..8abd6efe1 100644 --- a/test/tile/raster_dem_tile.test.cpp +++ b/test/tile/raster_dem_tile.test.cpp @@ -22,7 +22,7 @@ public: style::Style style { fileSource, 1 }; AnnotationManager annotationManager { style }; ImageManager imageManager; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { diff --git a/test/tile/raster_tile.test.cpp b/test/tile/raster_tile.test.cpp index 2e35a9402..3437061fa 100644 --- a/test/tile/raster_tile.test.cpp +++ b/test/tile/raster_tile.test.cpp @@ -22,7 +22,7 @@ public: style::Style style { fileSource, 1 }; AnnotationManager annotationManager { style }; ImageManager imageManager; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { diff --git a/test/tile/vector_tile.test.cpp b/test/tile/vector_tile.test.cpp index c610d8c1a..05c415fb2 100644 --- a/test/tile/vector_tile.test.cpp +++ b/test/tile/vector_tile.test.cpp @@ -28,7 +28,7 @@ public: style::Style style { fileSource, 1 }; AnnotationManager annotationManager { style }; ImageManager imageManager; - GlyphManager glyphManager { fileSource }; + GlyphManager glyphManager; Tileset tileset { { "https://example.com" }, { 0, 22 }, "none" }; TileParameters tileParameters { @@ -103,4 +103,4 @@ TEST(VectorTileData, ParseResults) { ASSERT_EQ(properties.at("disputed"), *feature->getValue("disputed")); ASSERT_EQ(feature->getValue("invalid"), nullopt); -} \ No newline at end of file +}