From 1be8e6327bfaf66ab0c0a858a7e81acec12c0bf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Minh=20Nguy=E1=BB=85n?= Date: Thu, 19 Mar 2015 14:40:44 -0700 Subject: [PATCH 01/21] Prepared podspec for beta 1 This podspec was originally lifted from mapbox/mapbox-ios-sdk, which was structured and built quite differently. We could have the podspec build this repo as is, using scripts/package_ios.sh, but that would require developers to install the same version of Xcode as Travis is using (see #977). So this podspec instead pulls a prebuilt library. Much simpler. Also package a LICENSE with the archive to satisfy a CocoaPods warning. Fixes #829. --- ios/MapboxGL.podspec | 16 ++++++++-------- scripts/package_ios.sh | 1 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/ios/MapboxGL.podspec b/ios/MapboxGL.podspec index 6cdec1826..617966744 100644 --- a/ios/MapboxGL.podspec +++ b/ios/MapboxGL.podspec @@ -8,25 +8,25 @@ Pod::Spec.new do |m| m.homepage = 'https://www.mapbox.com/blog/mapbox-gl/' m.license = 'BSD' m.author = { 'Mapbox' => 'mobile@mapbox.com' } - m.screenshot = 'https://raw.githubusercontent.com/mapbox/mapbox-gl-cocoa/master/pkg/screenshot.png' - m.social_media_url = 'https://twitter.com/Mapbox' + m.screenshot = 'https://raw.githubusercontent.com/mapbox/mapbox-gl-native/master/ios/screenshot.png' + m.social_media_url = 'https://twitter.com/mapbox' - m.source = { :git => 'https://github.com/mapbox/mapbox-gl-cocoa.git', :tag => m.version.to_s } + m.source = { :http => "http://mapbox.s3.amazonaws.com/mapbox-gl-native/ios/mapbox-gl-ios-#{m.version.to_s}.zip" } m.platform = :ios m.ios.deployment_target = '7.0' - m.source_files = 'dist/static/Headers/*.h' + m.source_files = 'Headers/*.h' m.requires_arc = true - m.resource_bundle = { 'MapboxGL' => 'dist/static/MapboxGL.bundle/*' } + m.resource_bundle = { 'MapboxGL' => 'MapboxGL.bundle/*' } - m.frameworks = 'CoreLocation', 'Foundation', 'GLKit', 'SystemConfiguration', 'UIKit' + m.frameworks = 'CoreLocation', 'GLKit', 'ImageIO', 'MobileCoreServices', 'SystemConfiguration' - m.libraries = 'MapboxGL', 'c++', 'sqlite3', 'z' + m.libraries = 'c++', 'sqlite3', 'z' - m.vendored_libraries = 'dist/static/libMapboxGL.a' + m.vendored_library = 'libMapboxGL.a' m.xcconfig = { 'OTHER_CPLUSPLUSFLAGS' => '-std=gnu++11 -stdlib=libc++' } diff --git a/scripts/package_ios.sh b/scripts/package_ios.sh index 89aac5ee7..588ff6d94 100755 --- a/scripts/package_ios.sh +++ b/scripts/package_ios.sh @@ -70,6 +70,7 @@ cp -pv include/mbgl/ios/* "${OUTPUT}/static/Headers" # complications between faked GYP bundles-as-executables, device build # dependencies, and code signing. step "Copying Resources..." +cp -pv LICENSE.md "${OUTPUT}/static" mkdir -p "${OUTPUT}/static/${NAME}.bundle" cp -pv platform/ios/resources/* "${OUTPUT}/static/${NAME}.bundle" cp -prv styles/styles "${OUTPUT}/static/${NAME}.bundle/styles" From 490c172b35c9f48d95b4c1a38003a00520ba27b8 Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Wed, 18 Mar 2015 02:05:02 +0200 Subject: [PATCH 02/21] Delete the GLFWView object at exit Also destroy the glfwWindow we create. This leak was probably harmless but a nice cleanup helps us debug other serious memory leaks. --- linux/main.cpp | 5 +++-- platform/default/glfw_view.cpp | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/linux/main.cpp b/linux/main.cpp index 697941164..4d5474c02 100644 --- a/linux/main.cpp +++ b/linux/main.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -13,7 +14,7 @@ #include -GLFWView *view = nullptr; +std::unique_ptr view; void quit_handler(int) { if (view) { @@ -62,7 +63,7 @@ int main(int argc, char *argv[]) { sigIntHandler.sa_flags = 0; sigaction(SIGINT, &sigIntHandler, NULL); - view = new GLFWView(); + view = mbgl::util::make_unique(); mbgl::SQLiteCache cache("/tmp/mbgl-cache.db"); mbgl::DefaultFileSource fileSource(&cache); diff --git a/platform/default/glfw_view.cpp b/platform/default/glfw_view.cpp index 601ae0ec2..fd9c701f9 100644 --- a/platform/default/glfw_view.cpp +++ b/platform/default/glfw_view.cpp @@ -11,6 +11,7 @@ GLFWView::GLFWView(bool fullscreen_) : fullscreen(fullscreen_) { } GLFWView::~GLFWView() { + glfwDestroyWindow(window); glfwTerminate(); } From ceb7049389abffa5e2e4b60239c2af189eb8420d Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Wed, 18 Mar 2015 02:14:23 +0200 Subject: [PATCH 03/21] Delete the data buffer of LineAtlas Data buffer is allocated but never deleted. --- src/mbgl/geometry/line_atlas.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mbgl/geometry/line_atlas.cpp b/src/mbgl/geometry/line_atlas.cpp index bf6944fbc..8be1a8d53 100644 --- a/src/mbgl/geometry/line_atlas.cpp +++ b/src/mbgl/geometry/line_atlas.cpp @@ -19,6 +19,7 @@ LineAtlas::~LineAtlas() { std::lock_guard lock(mtx); MBGL_CHECK_ERROR(glDeleteTextures(1, &texture)); + delete[] data; texture = 0; } From 1ffe2dd615a9cdca3cd785e19dd5a63fb1b5181c Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Wed, 18 Mar 2015 16:24:55 +0200 Subject: [PATCH 04/21] Add a reference Valgrind suppresion file When using Valgrind for debugging memory leaks and access violations, we often see a lot of false positives, specially when dealing with OpenGL that does it own buffer management. This file is an attempt to filter these false positives. Note that we are only filtering for the open source Radeon driver and the list will probably grow to accommodate other setups. To use the Valgrind with the suppression file for hunting memory leaks, try something like: valgrind --leak-check=full --track-origins=yes --suppressions=[path/to/valgrind.sup] ./mapbox-gl --- scripts/valgrind.sup | 91 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 scripts/valgrind.sup diff --git a/scripts/valgrind.sup b/scripts/valgrind.sup new file mode 100644 index 000000000..fcde049d9 --- /dev/null +++ b/scripts/valgrind.sup @@ -0,0 +1,91 @@ +{ + Valgrind gets confused with buffers managed by the graphics driver + Memcheck:Leak + ... + obj:/usr/*/dri/r600_dri.so + ... +} +{ + GLFW _glfwCreateCursor() + Memcheck:Leak + match-leak-kinds: reachable + ... + obj:/usr/*/libX11.so.6.3.0 + ... + fun:_glfwCreateCursor + ... +} +{ + GLFW _glfwPlatform*() + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:_glfwPlatform* + ... +} +{ + Buffer managed by OpenGL? + Memcheck:Cond + ... + fun:_ZN4mbgl*Uniform* + ... + fun:_ZN8GLFWView10invalidateEv + ... +} +{ + Buffer managed by OpenGL? + Memcheck:Cond + fun:_ZNSt7__equalILb0EE5equalIPKfS3_EEbT_S4_T0_ + ... + fun:_ZN4mbgl7Painter11renderLayerERKNS_10StyleLayerEPKNS_4Tile2IDEPKSt5arrayIfLm16EE +} +{ + Valgrind doesn't like our make_unique (C++14 polyfill) + Memcheck:Leak + match-leak-kinds: reachable + fun:malloc + ... + fun:_ZN4mbgl4util11make_unique* + ... +} +{ + dlopen doing its magic + Memcheck:Leak + match-leak-kinds: reachable + ... + fun:_dl_open + ... +} +{ + X11 false positive + Memcheck:Leak + match-leak-kinds: reachable + ... + obj:/usr/*/libX11.so.6.3.0 + ... + fun:_XrmInitParseInfo + ... +} +{ + OpenSSL false positive + Memcheck:Leak + ... + fun:CRYPTO_malloc + ... +} +{ + OpenSSL false positive + Memcheck:Leak + ... + fun:CRYPTO_realloc + ... +} +{ + Libcurl false positive + Memcheck:Leak + fun:malloc + ... + obj:/usr/*/libcurl.so.4.3.0 + fun:curl_multi_socket_action + ... +} From 61fa43619334ac68c66d45d38aaf681f2a21fa96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Konstantin=20K=C3=A4fer?= Date: Wed, 18 Mar 2015 15:37:06 +0100 Subject: [PATCH 05/21] coalesce buffer deletions and run them in bulk during rendering On iOS, the main thread does framebuffer operations just before it requests a rerender of the view contents. This means that GL operations on the Map thread will interfere with that and cause a crash. While most GL operations happen throughout rendering (which is synchronized with the main thread), deletion of shared_ptrs release in turn their OpenGL objects. This may happen at any time and isn't synced with rendering. This patch changes this so instead of deleting all objects immediately, we are collecting all abandoned IDs and delete them in bulk once the main thread has given us permission to use OpenGL calls (i.e. during render()). --- include/mbgl/map/map.hpp | 10 +++--- src/mbgl/geometry/buffer.hpp | 3 +- src/mbgl/geometry/line_atlas.cpp | 6 ++-- src/mbgl/geometry/sprite_atlas.cpp | 4 +-- src/mbgl/geometry/vao.cpp | 3 +- src/mbgl/map/environment.cpp | 55 ++++++++++++++++++++++++++++-- src/mbgl/map/environment.hpp | 42 ++++++++++++++++++----- src/mbgl/map/map.cpp | 23 +++++++++++-- src/mbgl/map/tile_data.cpp | 2 +- src/mbgl/util/texture_pool.cpp | 15 +++----- 10 files changed, 127 insertions(+), 36 deletions(-) diff --git a/include/mbgl/map/map.hpp b/include/mbgl/map/map.hpp index 86c89f769..06ae39d87 100644 --- a/include/mbgl/map/map.hpp +++ b/include/mbgl/map/map.hpp @@ -37,6 +37,7 @@ class GlyphAtlas; class SpriteAtlas; class LineAtlas; class Environment; +class EnvironmentScope; class AnnotationManager; class Map : private util::noncopyable { @@ -192,6 +193,7 @@ private: Mode mode = Mode::None; const std::unique_ptr env; + std::unique_ptr scope; View &view; private: @@ -223,13 +225,13 @@ private: FileSource& fileSource; util::ptr