From 5508285424b68c6f0a26e70ede8e389715d02deb Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 11:00:31 -0400 Subject: [PATCH 1/8] prefer c++ style memory alloc --- src/util/image.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/image.cpp b/src/util/image.cpp index fd0d34de2..4cbecc8c2 100644 --- a/src/util/image.cpp +++ b/src/util/image.cpp @@ -139,7 +139,8 @@ Image::Image(const std::string &data, bool flip) { png_size_t rowbytes = png_get_rowbytes(png, info); assert(width * 4 == rowbytes); - img = (char *)std::malloc(width * height * 4); + img = static_cast(::operator new(width * height * 4)); + char *surface = img; assert(surface); @@ -162,7 +163,7 @@ Image::Image(const std::string &data, bool flip) { fprintf(stderr, "loading PNG failed: %s\n", e.what()); png_destroy_read_struct(&png, &info, nullptr); if (img) { - std::free(img); + ::operator delete(img); img = nullptr; } width = 0; @@ -172,7 +173,7 @@ Image::Image(const std::string &data, bool flip) { Image::~Image() { if (img) { - std::free(img); + ::operator delete(img); img = nullptr; } } From 9364fd7322166578f6f0b7b57273db5a20bea2f7 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 12:35:17 -0400 Subject: [PATCH 2/8] make addGlyphsToAtlas static method of SymbolBucket + add addGlyphs method to minimise number of mutex locking --- include/mbgl/geometry/glyph_atlas.hpp | 4 ++++ include/mbgl/renderer/symbol_bucket.hpp | 4 ++-- src/geometry/glyph_atlas.cpp | 25 ++++++++++++++++++++++++- src/renderer/symbol_bucket.cpp | 17 ++++------------- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/include/mbgl/geometry/glyph_atlas.hpp b/include/mbgl/geometry/glyph_atlas.hpp index 5b09cbcd6..639c6cc51 100644 --- a/include/mbgl/geometry/glyph_atlas.hpp +++ b/include/mbgl/geometry/glyph_atlas.hpp @@ -24,6 +24,8 @@ private: std::set ids; }; + Rect addGlyph_impl(uint64_t tile_id, const std::string& face_name, + const SDFGlyph& glyph); public: GlyphAtlas(uint16_t width, uint16_t height); ~GlyphAtlas(); @@ -31,6 +33,8 @@ public: Rect addGlyph(uint64_t tile_id, const std::string& face_name, const SDFGlyph& glyph); + void addGlyphs(uint64_t tileid, std::u32string const& text, std::string const& stackname, + FontStack const& fontStack, GlyphPositions & face); void removeGlyphs(uint64_t tile_id); void bind(); diff --git a/include/mbgl/renderer/symbol_bucket.hpp b/include/mbgl/renderer/symbol_bucket.hpp index 42682401e..9a5da1d01 100644 --- a/include/mbgl/renderer/symbol_bucket.hpp +++ b/include/mbgl/renderer/symbol_bucket.hpp @@ -86,8 +86,8 @@ private: void addSymbols(Buffer &buffer, const PlacedGlyphs &symbols, float scale, PlacementRange placementRange); // Adds glyphs to the glyph atlas so that they have a left/top/width/height coordinates associated to them that we can use for writing to a buffer. - void addGlyphsToAtlas(uint64_t tileid, const std::string stackname, const std::u32string &string, - const FontStack &fontStack, GlyphAtlas &glyphAtlas, GlyphPositions &face); + static void addGlyphsToAtlas(uint64_t tileid, const std::string stackname, const std::u32string &string, + const FontStack &fontStack, GlyphAtlas &glyphAtlas, GlyphPositions &face); public: const StyleBucketSymbol &properties; diff --git a/src/geometry/glyph_atlas.cpp b/src/geometry/glyph_atlas.cpp index 6bee475da..72c961fda 100644 --- a/src/geometry/glyph_atlas.cpp +++ b/src/geometry/glyph_atlas.cpp @@ -22,9 +22,15 @@ GlyphAtlas::~GlyphAtlas() { } Rect GlyphAtlas::addGlyph(uint64_t tile_id, const std::string& face_name, - const SDFGlyph& glyph) { + const SDFGlyph& glyph) +{ std::lock_guard lock(mtx); + return addGlyph_impl(tile_id, face_name, glyph); +} +Rect GlyphAtlas::addGlyph_impl(uint64_t tile_id, const std::string& face_name, + const SDFGlyph& glyph) +{ // Use constant value for now. const uint8_t buffer = 3; @@ -83,6 +89,23 @@ Rect GlyphAtlas::addGlyph(uint64_t tile_id, const std::string& face_na return rect; } +void GlyphAtlas::addGlyphs(uint64_t tileid, std::u32string const& text, std::string const& stackname, FontStack const& fontStack, GlyphPositions & face) +{ + std::lock_guard lock(mtx); + + std::map const& sdfs = fontStack.getSDFs(); + for (uint32_t chr : text) + { + auto sdf_it = sdfs.find(chr); + if (sdf_it != sdfs.end()) + { + SDFGlyph const& sdf = sdf_it->second; + Rect rect = addGlyph_impl(tileid, stackname, sdf); + face.emplace(chr, Glyph{rect, sdf.metrics}); + } + } +} + void GlyphAtlas::removeGlyphs(uint64_t tile_id) { std::lock_guard lock(mtx); diff --git a/src/renderer/symbol_bucket.cpp b/src/renderer/symbol_bucket.cpp index 47cc4db44..7d5036eea 100644 --- a/src/renderer/symbol_bucket.cpp +++ b/src/renderer/symbol_bucket.cpp @@ -34,18 +34,9 @@ bool SymbolBucket::hasTextData() const { return !text.groups.empty(); } bool SymbolBucket::hasIconData() const { return !icon.groups.empty(); } void SymbolBucket::addGlyphsToAtlas(uint64_t tileid, const std::string stackname, - const std::u32string &string, const FontStack &fontStack, + const std::u32string &text, const FontStack &fontStack, GlyphAtlas &glyphAtlas, GlyphPositions &face) { - const std::map &sdfs = fontStack.getSDFs(); - // Loop through all characters and add glyph to atlas, positions. - for (uint32_t chr : string) { - auto sdf_it = sdfs.find(chr); - if (sdf_it != sdfs.end()) { - const SDFGlyph &sdf = sdf_it->second; - const Rect rect = glyphAtlas.addGlyph(tileid, stackname, sdf); - face.emplace(chr, Glyph{rect, sdf.metrics}); - } - } + glyphAtlas.addGlyphs(tileid, text, stackname, fontStack,face); } std::vector SymbolBucket::processFeatures(const VectorTileLayer &layer, @@ -150,8 +141,8 @@ void SymbolBucket::addFeatures(const VectorTileLayer &layer, const FilterExpress // Add the glyphs we need for this label to the glyph atlas. if (shaping.size()) { - addGlyphsToAtlas(id.to_uint64(), properties.text.font, feature.label, fontStack, - glyphAtlas, face); + SymbolBucket::addGlyphsToAtlas(id.to_uint64(), properties.text.font, feature.label, fontStack, + glyphAtlas, face); } } From e4294ec29446a6b1944d4d977d162d0a6bb4573f Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 13:34:22 -0400 Subject: [PATCH 3/8] use prefix operator++ --- src/style/style_layer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/style/style_layer.cpp b/src/style/style_layer.cpp index 4d64386be..ded73c044 100644 --- a/src/style/style_layer.cpp +++ b/src/style/style_layer.cpp @@ -20,7 +20,7 @@ void StyleLayer::setClasses(const std::vector &class_names, const t std::set already_applied; // Reverse iterate through all class names and apply them last to first. - for (auto it = class_names.rbegin(); it != class_names.rend(); it++) { + for (auto it = class_names.rbegin(); it != class_names.rend(); ++it) { const std::string &class_name = *it; // From here on, we're only dealing with IDs to avoid comparing strings all the time. const ClassID class_id = ClassDictionary::Get().lookup(class_name); From 95213cd3829fb2565c8b492c7eeb44a421df7bc7 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 13:55:14 -0400 Subject: [PATCH 4/8] c++ style memory allocations/deallocations --- src/geometry/sprite_atlas.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/geometry/sprite_atlas.cpp b/src/geometry/sprite_atlas.cpp index 10922790c..22e6a29a0 100644 --- a/src/geometry/sprite_atlas.cpp +++ b/src/geometry/sprite_atlas.cpp @@ -53,7 +53,7 @@ bool SpriteAtlas::resize(const float newRatio) { } } - free(old_data); + ::operator delete(old_data); dirty = true; // Mark all sprite images as in need of update @@ -154,7 +154,8 @@ void SpriteAtlas::allocate() { if (!data) { dimension w = static_cast(width * pixelRatio); dimension h = static_cast(height * pixelRatio); - data = (uint32_t *)calloc(w * h, sizeof(uint32_t)); + data = static_cast(::operator new(w * h * sizeof(uint32_t))); + std::fill(data, data + w * h, 0); } } @@ -268,9 +269,5 @@ SpriteAtlas::~SpriteAtlas() { glDeleteTextures(1, &texture); texture = 0; - - if (data) { - free(data); - data = nullptr; - } + ::operator delete(data), data = nullptr; } From efeaba832d955e2a31add33e87b7c8cb8aa053f3 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 13:56:32 -0400 Subject: [PATCH 5/8] c++ - don't test if pointer is nullptr before deleting --- src/util/image.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/image.cpp b/src/util/image.cpp index 4cbecc8c2..ffa65bf0b 100644 --- a/src/util/image.cpp +++ b/src/util/image.cpp @@ -172,8 +172,5 @@ Image::Image(const std::string &data, bool flip) { } Image::~Image() { - if (img) { - ::operator delete(img); - img = nullptr; - } + ::operator delete(img),img = nullptr; } From dcf643fb4b27f7dd1249514e4644d85d17cb1eb6 Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 14:16:38 -0400 Subject: [PATCH 6/8] add new line at the end --- src/util/io.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/io.cpp b/src/util/io.cpp index b88956743..76f7c35ad 100644 --- a/src/util/io.cpp +++ b/src/util/io.cpp @@ -31,4 +31,4 @@ std::string read_file(const std::string &filename) { } } -} \ No newline at end of file +} From c3f3b9ddd52a3f1540eb6eb6711caca7efc4a8fb Mon Sep 17 00:00:00 2001 From: artemp Date: Mon, 27 Oct 2014 14:17:27 -0400 Subject: [PATCH 7/8] canonical move ctor --- src/util/sqlite3.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/sqlite3.cpp b/src/util/sqlite3.cpp index 19e0ce9c7..787db8399 100644 --- a/src/util/sqlite3.cpp +++ b/src/util/sqlite3.cpp @@ -15,9 +15,8 @@ Database::Database(const std::string &filename, int flags) { } } -Database::Database(Database &&other) { - *this = std::move(other); -} +Database::Database(Database &&other) + : db(std::move(other.db)) {} Database &Database::operator=(Database &&other) { std::swap(db, other.db); From 071f92079082aa0676772946d8d40184dcc3d40d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20Ka=CC=88fer?= Date: Mon, 27 Oct 2014 12:34:10 -0700 Subject: [PATCH 8/8] mark as standalone static libraries so they get placed into the correct directory on non-osx targets --- gyp/mbgl-ios.gypi | 1 + gyp/mbgl-linux.gypi | 1 + gyp/mbgl-osx.gypi | 1 + gyp/mbgl.gypi | 1 + 4 files changed, 4 insertions(+) diff --git a/gyp/mbgl-ios.gypi b/gyp/mbgl-ios.gypi index dbad60086..4398f04eb 100644 --- a/gyp/mbgl-ios.gypi +++ b/gyp/mbgl-ios.gypi @@ -28,6 +28,7 @@ { 'target_name': 'mbgl-ios', 'product_name': 'mbgl-ios', 'type': 'static_library', + 'standalone_static_library': 1, 'hard_dependency': 1, 'include_dirs': [ '../include', diff --git a/gyp/mbgl-linux.gypi b/gyp/mbgl-linux.gypi index 39a175556..6d8631e8d 100644 --- a/gyp/mbgl-linux.gypi +++ b/gyp/mbgl-linux.gypi @@ -3,6 +3,7 @@ { 'target_name': 'mbgl-linux', 'product_name': 'mbgl-linux', 'type': 'static_library', + 'standalone_static_library': 1, 'hard_dependency': 1, 'include_dirs': [ '../include' diff --git a/gyp/mbgl-osx.gypi b/gyp/mbgl-osx.gypi index ba117e5a5..80af5b97a 100644 --- a/gyp/mbgl-osx.gypi +++ b/gyp/mbgl-osx.gypi @@ -3,6 +3,7 @@ { 'target_name': 'mbgl-osx', 'product_name': 'mbgl-osx', 'type': 'static_library', + 'standalone_static_library': 1, 'hard_dependency': 1, 'include_dirs': [ '../include', diff --git a/gyp/mbgl.gypi b/gyp/mbgl.gypi index f268fd625..0d914f46f 100644 --- a/gyp/mbgl.gypi +++ b/gyp/mbgl.gypi @@ -3,6 +3,7 @@ { 'target_name': 'mbgl', 'product_name': 'mbgl', 'type': 'static_library', + 'standalone_static_library': 1, 'hard_dependency': 1, 'dependencies': [ 'shaders',