diff options
44 files changed, 449 insertions, 384 deletions
diff --git a/.mason b/.mason -Subproject ec5ff7cc35d1dec325a00f4d8feff7fa3f6da2a +Subproject 0d06890655b5f2995682ea5b73aeb94010f8c89 diff --git a/.travis.yml b/.travis.yml index eb779e2454..ddc0c3ee5e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,13 +18,13 @@ addons_shortcuts: addons_qt4: &qt4 apt: sources: [ 'ubuntu-toolchain-r-test' ] - packages: [ 'gdb', 'g++-5', 'gcc-5', + packages: [ 'g++-5', 'gcc-5', 'mesa-utils', 'qt4-default' ] addons_qt5: &qt5 apt: sources: [ 'ubuntu-toolchain-r-test' ] - packages: [ 'gdb', 'g++-5', 'gcc-5', - 'mesa-utils', 'qt5-default', 'libqt5opengl5-dev', 'qtdeclarative5-dev', 'qtpositioning5-dev', 'qtlocation5-dev' ] + packages: [ 'g++-5', 'gcc-5', + 'mesa-utils', 'libc6-dbg', 'qt5-default', 'libqt5opengl5-dev', 'qtdeclarative5-dev', 'qtpositioning5-dev', 'qtlocation5-dev' ] env: global: @@ -109,7 +109,8 @@ matrix: env: BUILDTYPE=Release _CXX=g++-5 _CC=gcc-5 addons: *qt4 script: - - make qt-app test-qt + - make qt-app + - make test-qt # Qt 5 - Release - os: linux @@ -120,7 +121,9 @@ matrix: env: BUILDTYPE=Release _CXX=g++-5 _CC=gcc-5 addons: *qt5 script: - - make qt-app qt-qml-app test-qt + - make qt-app + - make qt-qml-app + - make test-valgrind-qt cache: directories: diff --git a/INSTALL.md b/INSTALL.md index d0f063840d..b16dcafb40 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -54,3 +54,7 @@ See the relevant SDK documentation for next steps: * [Mapbox Qt SDK](platform/qt/) * [Mapbox GL Native on Linux](platform/linux/) * [node-mapbox-gl-native](platform/node/) + +## 4: Keeping up to date + +This repository uses Git submodules, which should be automatically checked out when you first run a `make` command for one of the above platforms. These submodules are not updated automatically and we recommended that you run `git submodule update` after pulling down new commits to this repository. @@ -239,6 +239,13 @@ run-qt-app: qt-app run-qt-qml-app: qt-qml-app cd $(QT_OUTPUT_PATH)/$(BUILDTYPE) && ./qquickmapboxgl +test-valgrind-qt: $(QT_MAKEFILE) node_modules + $(QT_ENV) $(MAKE) -j$(JOBS) -C $(QT_OUTPUT_PATH) test + ./scripts/valgrind.sh $(QT_OUTPUT_PATH)/$(BUILDTYPE)/test --gtest_catch_exceptions=0 --gtest_filter=-*.Load + +run-valgrind-qt-app: qt-app + ./scripts/valgrind.sh $(QT_OUTPUT_PATH)/$(BUILDTYPE)/qmapboxgl --test -platform offscreen + #### Linux targets ##################################################### LINUX_OUTPUT_PATH = build/linux-$(shell uname -m) @@ -270,9 +277,6 @@ test: $(LINUX_MAKEFILE) run-glfw-app: glfw-app cd $(LINUX_OUTPUT_PATH)/$(BUILDTYPE) && ./mapbox-glfw -run-valgrind-glfw-app: glfw-app - cd $(LINUX_OUTPUT_PATH)/$(BUILDTYPE) && valgrind --leak-check=full --suppressions=../../../scripts/valgrind.sup ./mapbox-glfw - ifneq (,$(shell which gdb)) GDB = gdb -batch -return-child-result -ex 'set print thread-events off' -ex 'run' -ex 'thread apply all bt' --args endif @@ -123,6 +123,7 @@ print_flags webp static_libs cflags ldflags print_flags jni.hpp cflags print_flags earcut cflags print_flags benchmark static_libs cflags ldflags +print_flags valgrind CONFIG+=" } } diff --git a/include/mbgl/util/image.hpp b/include/mbgl/util/image.hpp index fbb0686a7b..124cdca7cd 100644 --- a/include/mbgl/util/image.hpp +++ b/include/mbgl/util/image.hpp @@ -1,5 +1,7 @@ #pragma once +#include <mbgl/util/noncopyable.hpp> + #include <string> #include <memory> #include <algorithm> @@ -12,7 +14,7 @@ enum ImageAlphaMode { }; template <ImageAlphaMode Mode> -class Image { +class Image : private util::noncopyable { public: Image() = default; @@ -26,6 +28,18 @@ public: height(h), data(std::move(data_)) {} + Image(Image&& o) + : width(o.width), + height(o.height), + data(std::move(o.data)) {} + + Image& operator=(Image&& o) { + width = o.width; + height = o.height; + data = std::move(o.data); + return *this; + } + bool operator==(const Image& rhs) const { return width == rhs.width && height == rhs.height && std::equal(data.get(), data.get() + size(), rhs.data.get(), diff --git a/package.json b/package.json index b8d31d9cd7..0b7f7e2d1b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mapbox-gl-native", - "version": "3.2.0", + "version": "3.2.1", "description": "Renders map tiles with Mapbox GL", "keywords": [ "mapbox", @@ -13,15 +13,15 @@ }, "license": "BSD-2-Clause", "dependencies": { - "nan": "^2.3.3", - "node-pre-gyp": "^0.6.26" + "nan": "^2.3.5", + "node-pre-gyp": "^0.6.28" }, "devDependencies": { "aws-sdk": "^2.3.5", "csscolorparser": "^1.0.2", "ejs": "^2.4.1", "express": "^4.11.1", - "mapbox-gl-shaders": "mapbox/mapbox-gl-shaders#e032997109c0ead5394ff64f2f0ea6b0f8efdc3f", + "mapbox-gl-shaders": "mapbox/mapbox-gl-shaders#8dfd9653e6659cb2837a4b98f3e8e1d559b29a09", "mapbox-gl-style-spec": "^8.5.1", "mapbox-gl-test-suite": "mapbox/mapbox-gl-test-suite#f45fd7aba98650c7f3bf778c9cbbfd3b548a4ee8", "node-gyp": "^3.3.1", diff --git a/platform/default/webp_reader.cpp b/platform/default/webp_reader.cpp index 37d23da110..5b0eaf4741 100644 --- a/platform/default/webp_reader.cpp +++ b/platform/default/webp_reader.cpp @@ -15,8 +15,11 @@ PremultipliedImage decodeWebP(const uint8_t* data, size_t size) { throw std::runtime_error("failed to retrieve WebP basic header information"); } - std::unique_ptr<uint8_t[]> webp(WebPDecodeRGBA(data, size, &width, &height)); - if (!webp) { + int stride = width * 4; + size_t webpSize = stride * height; + auto webp = std::make_unique<uint8_t[]>(webpSize); + + if (!WebPDecodeRGBAInto(data, size, webp.get(), webpSize, stride)) { throw std::runtime_error("failed to decode WebP data"); } diff --git a/platform/ios/CHANGELOG.md b/platform/ios/CHANGELOG.md index 8c2eeca35f..2b34571d1c 100644 --- a/platform/ios/CHANGELOG.md +++ b/platform/ios/CHANGELOG.md @@ -21,19 +21,24 @@ Mapbox welcomes participation and contributions from everyone. Please read [CON - Fixed an issue preventing KVO change notifications from being generated on MGLMapView’s `userTrackingMode` key path when `-setUserTrackingMode:animated:` is called. ([#4724](https://github.com/mapbox/mapbox-gl-native/pull/4724)) - Rendering now occurs on the main thread, fixing a hang when calling `-[MGLMapView styleURL]` before the map view has fully loaded or while the application is in the background. ([#2909](https://github.com/mapbox/mapbox-gl-native/pull/2909)) - Improved responsiveness when zooming in then immediately panning around. ([#4595](https://github.com/mapbox/mapbox-gl-native/pull/4595)) +- Fixed a crash setting MGLMapView’s `userLocationVerticalAlignment` property before a user location update has occurred. ([#5274](https://github.com/mapbox/mapbox-gl-native/issues/5274)) - Added category methods on NSValue for converting to and from the structure types defined in MGLGeometry.h. ([#4802](https://github.com/mapbox/mapbox-gl-native/pull/4802)) - Added NSFormatter subclasses for converting geographic coordinates and directions into display strings. ([#4802](https://github.com/mapbox/mapbox-gl-native/pull/4802)) - Added a new method, `-[MGLMapView cameraThatFitsCoordinateBounds:]`, to get a camera that you can pass into `-setCamera:` that fits the given coordinate bounds. ([#4790](https://github.com/mapbox/mapbox-gl-native/pull/4790)) - Added a `-reloadStyle:` action to MGLMapView to force a reload of the current style. ([#4728](https://github.com/mapbox/mapbox-gl-native/pull/4728)) - A more specific user agent string is now sent with style and tile requests. ([#4012](https://github.com/mapbox/mapbox-gl-native/pull/4012)) - Mapbox Telemetry is automatically disabled while the host application is running in the iOS Simulator. ([#4726](https://github.com/mapbox/mapbox-gl-native/pull/4726)) -- Fixed an issue preventing `-[MGLMapViewDelegate mapViewDidFinishLoadingMap:]` from being called when returning to the view controller containing the map view from another view controller. ([#5164](https://github.com/mapbox/mapbox-gl-native/pull/5164)) - Suppressed “Unable to make space for entry” console spew. ([#4708](https://github.com/mapbox/mapbox-gl-native/pull/4708)) - Removed unused SVG files from the SDK’s resource bundle. ([#4641](https://github.com/mapbox/mapbox-gl-native/pull/4641)) - Deprecated `-[MGLMapView emptyMemoryCache]`. ([#4725](https://github.com/mapbox/mapbox-gl-native/pull/4725)) - Added `MGLCoordinateInCoordinateBounds()`, a function that tests whether or not a coordinate is in a given bounds. ([#5053](https://github.com/mapbox/mapbox-gl-native/pull/5053)) - Added a new option to `MGLMapDebugMaskOptions`, `MGLMapDebugWireframesMask`, that shows wireframes instead of the usual rendered output. ([#4359](https://github.com/mapbox/mapbox-gl-native/pull/4359)) +## 3.2.3 + +- Fixed an issue preventing `-[MGLMapViewDelegate mapViewDidFinishLoadingMap:]` from being called when returning to the view controller containing the map view from another view controller. ([#5164](https://github.com/mapbox/mapbox-gl-native/pull/5164)) +- Declarations in the API documentation are shown in both Objective-C and Swift. ([realm/jazzy#530](https://github.com/realm/jazzy/pull/530)) + ## 3.2.2 - Existing MGLStyle class methods that return default style URLs have been deprecated in favor of new methods that require an explicit style version parameter. The deprecated, unversioned methods continue to return version 8 of the respective styles and will not be updated as new versions of the styles are released. ([#4759](https://github.com/mapbox/mapbox-gl-native/pull/4759)) diff --git a/platform/ios/DEVELOPING.md b/platform/ios/DEVELOPING.md index 67481d4f8a..d771f7541b 100644 --- a/platform/ios/DEVELOPING.md +++ b/platform/ios/DEVELOPING.md @@ -8,30 +8,42 @@ The Mapbox iOS SDK and iosapp demo application build against the iOS 7.0 SDK. Th ## Building the SDK -[Install core dependencies](../../INSTALL.md), then run +Make sure that you have the [core dependencies](../../INSTALL.md) installed. + +Create and open an Xcode workspace that includes both the SDK source and some Objective-C test applications by running: ```bash make iproj ``` -which will create and open an Xcode workspace that includes both the SDK source and some Objective-C test applications. Before building, use the scheme picker button in the toolbar to change the scheme to “iosapp” and the destination to one of the simulators or connected devices listed in the menu. +### Xcode schemes + +Before building, use the scheme picker button in the toolbar to change the scheme to **iosapp** and the destination to one of the simulators or connected devices listed in the menu. -“iosapp” is only one of several shared schemes. Others include: +**iosapp** is only one of several shared schemes. Others include: -* “CI” builds exactly what gets built for continuous integration on Bitrise. -* “bench” is a simple benchmarking application. For more consistent results between runs, run platform/ios/benchmark/assets/{glyphs,tiles}/download.sh to download any necessary resources. -* “dynamic” builds the SDK as a dynamic framework. -* “static” builds the SDK as a static library and separate resource bundle. -* “dynamic+static” is a combination of the “dynamic” and “static” schemes. +* **CI** builds exactly what gets built for continuous integration on Bitrise. +* **bench** is a simple benchmarking application. For more consistent results between runs, run `platform/ios/benchmark/assets/{glyphs,tiles}/download.sh` to download any necessary resources. +* **dynamic** builds the SDK as a dynamic framework. +* **static** builds the SDK as a static library and separate resource bundle. +* **dynamic+static** is a combination of the **dynamic** and **static** schemes. If you don’t have an Apple Developer account, change the destination to a simulator such as “iPhone 6s” before you run and build the app. -`iproj` is only one of several available `make` rules. Others include: +### Packaging builds + +Install [jazzy](https://github.com/realm/jazzy) for generating API documentation: + +```bash +[sudo] gem install jazzy +``` + +Build and package the SDK by using one of the following commands: -* `make ipackage` builds both dynamic and static frameworks in the Release configuration for devices and the iOS Simulator. +* `make ipackage` builds both dynamic and static frameworks in the Debug configuration for devices and the iOS Simulator. +* `make iframework` builds a dynamic framework in the Debug configuration for devices and the iOS Simulator. The CocoaPods pod downloads the output of this target. * `make ipackage-sim` builds a dynamic framework in the Debug configuration for the iOS simulator. This is the fastest target. -* `make ipackage-strip` builds both dynamic and static frameworks in the Release configuration, stripped of debug symbols, for devices and the iOS Simulator. -* `make iframework` builds a dynamic framework in the Release configuration for devices and the iOS Simulator. The CocoaPods pod downloads the output of this target. +* `make ipackage-strip` builds both dynamic and static frameworks in the Debug configuration, stripped of debug symbols, for devices and the iOS Simulator. * `make ifabric` builds a special static framework for compatibility with the Fabric Mac application. You can customize the build output by passing the following arguments into the `make` invocation: @@ -41,12 +53,14 @@ You can customize the build output by passing the following arguments into the ` * `FORMAT=dynamic` builds only a dynamic framework. `FORMAT=static` builds only a static framework, for compatibility with iOS 7.x. * `SYMBOLS=NO` strips the build output of any debug symbols, yielding much smaller binaries. -These targets require that you install [jazzy](https://github.com/realm/jazzy) for generating API documentation: +An example command that creates a dynamic framework suitable for eventual App Store distribution: ```bash -[sudo] gem install jazzy +make iframework BUILDTYPE=Release SYMBOLS=NO ``` +The products of these build commands can be found in the `build/ios/pkg` folder at the base of the repository. + ## Contributing ### Adding a source code file diff --git a/platform/ios/Mapbox-iOS-SDK-symbols.podspec b/platform/ios/Mapbox-iOS-SDK-symbols.podspec index a29e9467ea..3642d8b7de 100644 --- a/platform/ios/Mapbox-iOS-SDK-symbols.podspec +++ b/platform/ios/Mapbox-iOS-SDK-symbols.podspec @@ -10,7 +10,7 @@ Pod::Spec.new do |m| m.author = { 'Mapbox' => 'mobile@mapbox.com' } m.screenshot = 'https://raw.githubusercontent.com/mapbox/mapbox-gl-native/master/platform/ios/screenshot.png' m.social_media_url = 'https://twitter.com/mapbox' - m.documentation_url = 'https://www.mapbox.com/ios-sdk/' + m.documentation_url = 'https://www.mapbox.com/ios-sdk/api/' m.source = { :http => "https://mapbox.s3.amazonaws.com/mapbox-gl-native/ios/builds/mapbox-ios-sdk-#{m.version.to_s}-dynamic.zip", diff --git a/platform/ios/Mapbox-iOS-SDK.podspec b/platform/ios/Mapbox-iOS-SDK.podspec index 33b83e5370..da9213dba2 100644 --- a/platform/ios/Mapbox-iOS-SDK.podspec +++ b/platform/ios/Mapbox-iOS-SDK.podspec @@ -10,7 +10,7 @@ Pod::Spec.new do |m| m.author = { 'Mapbox' => 'mobile@mapbox.com' } m.screenshot = 'https://raw.githubusercontent.com/mapbox/mapbox-gl-native/master/platform/ios/screenshot.png' m.social_media_url = 'https://twitter.com/mapbox' - m.documentation_url = 'https://www.mapbox.com/ios-sdk/' + m.documentation_url = 'https://www.mapbox.com/ios-sdk/api/' m.source = { :http => "https://mapbox.s3.amazonaws.com/mapbox-gl-native/ios/builds/mapbox-ios-sdk-#{m.version.to_s}-dynamic.zip", diff --git a/platform/ios/app/MBXViewController.m b/platform/ios/app/MBXViewController.m index 70f777422d..8317aee1d9 100644 --- a/platform/ios/app/MBXViewController.m +++ b/platform/ios/app/MBXViewController.m @@ -575,7 +575,7 @@ static NSString * const MBXViewControllerAnnotationViewReuseIdentifer = @"MBXVie - (MGLAnnotationView *)mapView:(MGLMapView *)mapView viewForAnnotation:(id<MGLAnnotation>)annotation { // Use GL backed pins for dropped pin annotations - if ([annotation isMemberOfClass:[MBXDroppedPinAnnotation class]]) + if ([annotation isKindOfClass:[MBXDroppedPinAnnotation class]]) { return nil; } @@ -603,82 +603,6 @@ static NSString * const MBXViewControllerAnnotationViewReuseIdentifer = @"MBXVie return annotationView; } -- (MGLAnnotationImage *)mapView:(MGLMapView * __nonnull)mapView imageForAnnotation:(id <MGLAnnotation> __nonnull)annotation -{ - if ([annotation isKindOfClass:[MBXDroppedPinAnnotation class]] - || [annotation isKindOfClass:[MBXCustomCalloutAnnotation class]]) - { - return nil; // use default marker - } - - NSString *title = [(MGLPointAnnotation *)annotation title]; - if (!title.length) return nil; - NSString *lastTwoCharacters = [title substringFromIndex:title.length - 2]; - - MGLAnnotationImage *annotationImage = [mapView dequeueReusableAnnotationImageWithIdentifier:lastTwoCharacters]; - - if ( ! annotationImage) - { - UIColor *color; - - // make every tenth annotation blue - if ([lastTwoCharacters hasSuffix:@"0"]) { - color = [UIColor blueColor]; - } else { - color = [UIColor redColor]; - } - - UIImage *image = [self imageWithText:lastTwoCharacters backgroundColor:color]; - annotationImage = [MGLAnnotationImage annotationImageWithImage:image reuseIdentifier:lastTwoCharacters]; - - // don't allow touches on blue annotations - if ([color isEqual:[UIColor blueColor]]) annotationImage.enabled = NO; - } - - return annotationImage; -} - -- (UIImage *)imageWithText:(NSString *)text backgroundColor:(UIColor *)color -{ - CGRect rect = CGRectMake(0, 0, 20, 15); - - UIGraphicsBeginImageContextWithOptions(rect.size, NO, [[UIScreen mainScreen] scale]); - - CGContextRef ctx = UIGraphicsGetCurrentContext(); - - CGContextSetFillColorWithColor(ctx, [[color colorWithAlphaComponent:0.75] CGColor]); - CGContextFillRect(ctx, rect); - - CGContextSetStrokeColorWithColor(ctx, [[UIColor blackColor] CGColor]); - CGContextStrokeRectWithWidth(ctx, rect, 2); - - NSAttributedString *drawString = [[NSAttributedString alloc] initWithString:text attributes:@{ - NSFontAttributeName: [UIFont fontWithName:@"Arial-BoldMT" size:12], - NSForegroundColorAttributeName: [UIColor whiteColor], - }]; - CGSize stringSize = drawString.size; - CGRect stringRect = CGRectMake((rect.size.width - stringSize.width) / 2, - (rect.size.height - stringSize.height) / 2, - stringSize.width, - stringSize.height); - [drawString drawInRect:stringRect]; - - UIImage *image = UIGraphicsGetImageFromCurrentImageContext(); - UIGraphicsEndImageContext(); - return image; -} - -- (void)mapView:(MGLMapView *)mapView didDeselectAnnotation:(id<MGLAnnotation>)annotation { - NSString *title = [(MGLPointAnnotation *)annotation title]; - if ( ! title.length) - { - return; - } - NSString *lastTwoCharacters = [title substringFromIndex:title.length - 2]; - MGLAnnotationImage *annotationImage = [mapView dequeueReusableAnnotationImageWithIdentifier:lastTwoCharacters]; - annotationImage.image = annotationImage.image ? nil : [self imageWithText:lastTwoCharacters backgroundColor:[UIColor grayColor]]; -} - - (BOOL)mapView:(__unused MGLMapView *)mapView annotationCanShowCallout:(__unused id <MGLAnnotation>)annotation { return YES; diff --git a/platform/ios/src/MGLMapView.h b/platform/ios/src/MGLMapView.h index d02c3a93a9..63d799bda9 100644 --- a/platform/ios/src/MGLMapView.h +++ b/platform/ios/src/MGLMapView.h @@ -882,9 +882,10 @@ IB_DESIGNABLE Adds an annotation to the map view. @note `MGLMultiPolyline`, `MGLMultiPolygon`, and `MGLShapeCollection` objects - cannot be added to the map view at this time. Any multipolyline, - multipolygon, or shape collection object that is passed into this method is - silently ignored. + cannot be added to the map view at this time. Nor can `MGLMultiPoint` + objects that are not instances of `MGLPolyline` or `MGLPolygon`. Any + multipoint, multipolyline, multipolygon, or shape collection object that is + specified is silently ignored. @param annotation The annotation object to add to the receiver. This object must conform to the `MGLAnnotation` protocol. The map view retains the @@ -895,9 +896,10 @@ IB_DESIGNABLE Adds an array of annotations to the map view. @note `MGLMultiPolyline`, `MGLMultiPolygon`, and `MGLShapeCollection` objects - cannot be added to the map view at this time. Any multipolyline, - multipolygon, or shape collection objects that are passed in are silently - ignored. + cannot be added to the map view at this time. Nor can `MGLMultiPoint` + objects that are not instances of `MGLPolyline` or `MGLPolygon`. Any + multipoint, multipolyline, multipolygon, or shape collection objects that + are specified are silently ignored. @param annotations An array of annotation objects. Each object in the array must conform to the `MGLAnnotation` protocol. The map view retains each diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm index 1ad758dc77..5d1bcb1109 100644 --- a/platform/ios/src/MGLMapView.mm +++ b/platform/ios/src/MGLMapView.mm @@ -1779,8 +1779,16 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) if (annotation == [self annotationWithTag:annotationTag]) { const mbgl::Point<double> point = MGLPointFromLocationCoordinate2D(annotation.coordinate); - MGLAnnotationImage *annotationImage = [self imageOfAnnotationWithTag:annotationTag]; - _mbglMap->updateAnnotation(annotationTag, mbgl::SymbolAnnotation { point, annotationImage.styleIconIdentifier.UTF8String ?: "" }); + + MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); + NSString *symbolName; + if (!annotationContext.annotationView) + { + MGLAnnotationImage *annotationImage = [self imageOfAnnotationWithTag:annotationTag]; + symbolName = annotationImage.styleIconIdentifier; + } + + _mbglMap->updateAnnotation(annotationTag, mbgl::SymbolAnnotation { point, symbolName.UTF8String ?: "" }); if (annotationTag == _selectedAnnotationTag) { [self deselectAnnotation:annotation animated:YES]; @@ -2095,8 +2103,7 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) } std::sort(visibleAnnotations.begin(), visibleAnnotations.end()); - auto foundElement = std::find(visibleAnnotations.begin(), visibleAnnotations.end(), - ((MGLAnnotationAccessibilityElement *)element).tag); + auto foundElement = std::find(visibleAnnotations.begin(), visibleAnnotations.end(), tag); if (foundElement == visibleAnnotations.end()) { return NSNotFound; @@ -2835,7 +2842,14 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) if ([annotation isKindOfClass:[MGLMultiPoint class]]) { - // The multipoint knows how to style itself (with the map view’s help). + // Actual multipoints aren’t supported as annotations. + if ([annotation isMemberOfClass:[MGLMultiPoint class]] + || [annotation isMemberOfClass:[MGLMultiPointFeature class]]) + { + continue; + } + + // The polyline or polygon knows how to style itself (with the map view’s help). MGLMultiPoint *multiPoint = (MGLMultiPoint *)annotation; if (!multiPoint.pointCount) { continue; @@ -2846,13 +2860,9 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) context.annotation = annotation; _annotationContextsByAnnotationTag[annotationTag] = context; } - else if ([annotation isKindOfClass:[MGLMultiPolyline class]] - || [annotation isKindOfClass:[MGLMultiPolygon class]] - || [annotation isKindOfClass:[MGLShapeCollection class]]) - { - continue; - } - else + else if ( ! [annotation isKindOfClass:[MGLMultiPolyline class]] + || ![annotation isKindOfClass:[MGLMultiPolygon class]] + || ![annotation isKindOfClass:[MGLShapeCollection class]]) { MGLAnnotationView *annotationView; NSString *symbolName; @@ -3384,10 +3394,13 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) { MGLAnnotationContext &annotationContext = _annotationContextsByAnnotationTag.at(annotationTag); - if (annotationContext.annotationView) + MGLAnnotationView *annotationView = annotationContext.annotationView; + if (annotationView) { // Annotations represented by views use the view frame as the positioning rect. - positioningRect = annotationContext.annotationView.frame; + positioningRect = annotationView.frame; + + [annotationView.superview bringSubviewToFront:annotationView]; } } @@ -3832,9 +3845,10 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) [self.locationManager stopUpdatingHeading]; - if (self.userLocationAnnotationView) + CLLocation *location = self.userLocation.location; + if (location && self.userLocationAnnotationView) { - [self locationManager:self.locationManager didUpdateLocations:@[self.userLocation.location] animated:animated]; + [self locationManager:self.locationManager didUpdateLocations:@[location] animated:animated]; } break; @@ -3882,7 +3896,11 @@ mbgl::Duration MGLDurationInSeconds(NSTimeInterval duration) _userLocationVerticalAlignment = alignment; if (self.userTrackingMode != MGLUserTrackingModeNone) { - [self locationManager:self.locationManager didUpdateLocations:@[self.userLocation.location] animated:animated]; + CLLocation *location = self.userLocation.location; + if (location) + { + [self locationManager:self.locationManager didUpdateLocations:@[location] animated:animated]; + } } } diff --git a/platform/node/CHANGELOG.md b/platform/node/CHANGELOG.md index a8f1d5be72..e1bff6cc28 100644 --- a/platform/node/CHANGELOG.md +++ b/platform/node/CHANGELOG.md @@ -1,3 +1,7 @@ +# 3.2.1 + +- Fixes a memory leak in raster image data ([#5269](https://github.com/mapbox/mapbox-gl-native/pull/5269)) + # 3.2.0 - Switches to [earcut.hpp](https://github.com/mapbox/earcut.hpp) for tessellation ([#2444](https://github.com/mapbox/mapbox-gl-native/pull/2444)) diff --git a/platform/osx/INSTALL.md b/platform/osx/INSTALL.md index 8a4323aa8e..63b8947227 100644 --- a/platform/osx/INSTALL.md +++ b/platform/osx/INSTALL.md @@ -21,7 +21,15 @@ Grab a [prebuilt release](https://github.com/mapbox/mapbox-gl-native/releases/) ## Usage -In a storyboard or XIB, add a view to your view controller. (Drag Custom View from the Object library to the View Controller scene on the Interface Builder canvas.) In the Identity inspector, set the view’s custom class to `MGLMapView`. If you need to manipulate the map view programmatically: +In a storyboard or XIB: + +1. Add a view to your view controller or window. (Drag Custom View from the Object library to the View Controller scene on the Interface Builder canvas. In a XIB, drag it instead to the window on the canvas.) +2. In the Identity inspector, set the view’s custom class to `MGLMapView`. +3. MGLMapView needs to be layer-backed: + * You can make the window layer-backed by selecting the window and checking Full Size Content View in the Attributes inspector. This allows the map view to underlap the title bar and toolbar. + * Alternatively, if you don’t want the entire window to be layer-backed, you can make just the map view layer-backed by selecting it and checking its entry under the View Effects inspector’s Core Animation Layer section. + +If you need to manipulate the map view programmatically: 1. Switch to the Assistant Editor. 1. Import the `Mapbox` module. diff --git a/platform/osx/app/MapDocument.m b/platform/osx/app/MapDocument.m index 4274126747..e9f3b99592 100644 --- a/platform/osx/app/MapDocument.m +++ b/platform/osx/app/MapDocument.m @@ -18,7 +18,18 @@ NS_ARRAY_OF(id <MGLAnnotation>) *MBXFlattenedShapes(NS_ARRAY_OF(id <MGLAnnotatio NSMutableArray *flattenedShapes = [NSMutableArray arrayWithCapacity:shapes.count]; for (id <MGLAnnotation> shape in shapes) { NSArray *subshapes; - if ([shape isKindOfClass:[MGLMultiPolyline class]]) { + // Flatten multipoints but not polylines or polygons. + if ([shape isMemberOfClass:[MGLMultiPoint class]]) { + NSUInteger pointCount = [(MGLMultiPoint *)shape pointCount]; + CLLocationCoordinate2D *coordinates = [(MGLMultiPoint *)shape coordinates]; + NSMutableArray *pointAnnotations = [NSMutableArray arrayWithCapacity:pointCount]; + for (NSUInteger i = 0; i < pointCount; i++) { + MGLPointAnnotation *pointAnnotation = [[MGLPointAnnotation alloc] init]; + pointAnnotation.coordinate = coordinates[i]; + [pointAnnotations addObject:pointAnnotation]; + } + subshapes = pointAnnotations; + } else if ([shape isKindOfClass:[MGLMultiPolyline class]]) { subshapes = [(MGLMultiPolyline *)shape polylines]; } else if ([shape isKindOfClass:[MGLMultiPolygon class]]) { subshapes = [(MGLMultiPolygon *)shape polygons]; diff --git a/platform/osx/src/MGLMapView.h b/platform/osx/src/MGLMapView.h index 5c7e75135b..7b3efd293b 100644 --- a/platform/osx/src/MGLMapView.h +++ b/platform/osx/src/MGLMapView.h @@ -535,9 +535,10 @@ IB_DESIGNABLE Adds an annotation to the map view. @note `MGLMultiPolyline`, `MGLMultiPolygon`, and `MGLShapeCollection` objects - cannot be added to the map view at this time. Any multipolyline, - multipolygon, or shape collection object that is passed into this method is - silently ignored. + cannot be added to the map view at this time. Nor can `MGLMultiPoint` + objects that are not instances of `MGLPolyline` or `MGLPolygon`. Any + multipoint, multipolyline, multipolygon, or shape collection object that is + specified is silently ignored. @param annotation The annotation object to add to the receiver. This object must conform to the `MGLAnnotation` protocol. The map view retains the @@ -549,9 +550,10 @@ IB_DESIGNABLE Adds an array of annotations to the map view. @note `MGLMultiPolyline`, `MGLMultiPolygon`, and `MGLShapeCollection` objects - cannot be added to the map view at this time. Any multipolyline, - multipolygon, or shape collection objects that are passed in are silently - ignored. + cannot be added to the map view at this time. Nor can `MGLMultiPoint` + objects that are not instances of `MGLPolyline` or `MGLPolygon`. Any + multipoint, multipolyline, multipolygon, or shape collection objects that + are specified are silently ignored. @param annotations An array of annotation objects. Each object in the array must conform to the `MGLAnnotation` protocol. The map view retains each diff --git a/platform/osx/src/MGLMapView.mm b/platform/osx/src/MGLMapView.mm index 03b94c25ba..ac4eae9d34 100644 --- a/platform/osx/src/MGLMapView.mm +++ b/platform/osx/src/MGLMapView.mm @@ -1608,6 +1608,12 @@ public: NSAssert([annotation conformsToProtocol:@protocol(MGLAnnotation)], @"Annotation does not conform to MGLAnnotation"); if ([annotation isKindOfClass:[MGLMultiPoint class]]) { + // Actual multipoints aren’t supported as annotations. + if ([annotation isMemberOfClass:[MGLMultiPoint class]] + || [annotation isMemberOfClass:[MGLMultiPointFeature class]]) { + continue; + } + // The multipoint knows how to style itself (with the map view’s help). MGLMultiPoint *multiPoint = (MGLMultiPoint *)annotation; if (!multiPoint.pointCount) { @@ -1618,11 +1624,9 @@ public: MGLAnnotationContext context; context.annotation = annotation; _annotationContextsByAnnotationTag[annotationTag] = context; - } else if ([annotation isKindOfClass:[MGLMultiPolyline class]] - || [annotation isKindOfClass:[MGLMultiPolygon class]] - || [annotation isKindOfClass:[MGLShapeCollection class]]) { - continue; - } else { + } else if (![annotation isKindOfClass:[MGLMultiPolyline class]] + || ![annotation isKindOfClass:[MGLMultiPolygon class]] + || ![annotation isKindOfClass:[MGLShapeCollection class]]) { MGLAnnotationImage *annotationImage = nil; if (delegateHasImagesForAnnotations) { annotationImage = [self.delegate mapView:self imageForAnnotation:annotation]; diff --git a/platform/qt/scripts/configure.sh b/platform/qt/scripts/configure.sh index 4b855f7c58..5f31c853d2 100644 --- a/platform/qt/scripts/configure.sh +++ b/platform/qt/scripts/configure.sh @@ -28,6 +28,8 @@ if [ "$MASON_PLATFORM" == "osx" ]; then CONFIG+=" 'opengl_ldflags%': ['-framework OpenGL', '-framework CoreFoundation'],"$LN } else + VALGRIND_VERSION=latest + function print_opengl_flags { CONFIG+=" 'opengl_cflags%': $(quote_flags $(pkg-config gl x11 --cflags)),"$LN CONFIG+=" 'opengl_ldflags%': $(quote_flags $(pkg-config gl x11 --libs)),"$LN diff --git a/scripts/build-shaders.py b/scripts/build-shaders.py index 60e8552123..3b02cdc886 100755 --- a/scripts/build-shaders.py +++ b/scripts/build-shaders.py @@ -20,28 +20,21 @@ with open(input_file, "r") as f: # Replace uniform pragmas -pragma_mapbox_regex = re.compile("(\s*)\#pragma \mapbox\: (initialize|define) (.*) (lowp|mediump|highp)") -color_regex = re.compile("(.*)color") +pragma_mapbox_regex = re.compile("(\s*)\#pragma \mapbox\: (define|initialize) (low|medium|high)p (float|vec(2|3|4)) (.*)") def replace_uniform_pragmas(line): - # FIXME We should obtain these from the source code. if pragma_mapbox_regex.match(line): params = line.split() - u_method = params[2] - u_name = "color" if color_regex.match(params[3]) else params[3] - u_precision = params[4] - u_type = "vec4" if color_regex.match(u_name) else "float" - if u_method == "define": - return """uniform {precision} {type_} u_{name};""".format( - precision = u_precision, - type_ = u_type, - name = u_name) + if params[2] == "define": + return """uniform {u_precision} {u_type} u_{u_name};""".format( + u_precision = params[3], + u_type = params[4], + u_name = params[5]) else: - return """ {precision} {type_} {glsl_name} = u_{name};""".format( - precision = u_precision, - type_ = u_type, - glsl_name = params[3], - name = u_name) + return """ {u_precision} {u_type} {u_name} = u_{u_name};""".format( + u_precision = params[3], + u_type = params[4], + u_name = params[5]) else: return line diff --git a/scripts/generate-style-code.js b/scripts/generate-style-code.js index 0f2dcdc42c..6379598446 100644 --- a/scripts/generate-style-code.js +++ b/scripts/generate-style-code.js @@ -46,6 +46,11 @@ global.propertyType = function (property) { } global.defaultValue = function (property) { + // https://github.com/mapbox/mapbox-gl-native/issues/5258 + if (property.name === 'line-round-limit') { + return 1; + } + switch (property.type) { case 'number': return property.default; diff --git a/scripts/travis_setup.sh b/scripts/travis_setup.sh index e9fa87811f..dfd045224a 100755 --- a/scripts/travis_setup.sh +++ b/scripts/travis_setup.sh @@ -45,8 +45,8 @@ git submodule update --init .mason # Install and set up to load a more recent version of mesa mapbox_time "install_mesa" \ -mason install mesa 10.4.3 -export LD_LIBRARY_PATH="`mason prefix mesa 10.4.3`/lib:${LD_LIBRARY_PATH:-}" +mason install mesa 11.2.2 +export LD_LIBRARY_PATH="`mason prefix mesa 11.2.2`/lib:${LD_LIBRARY_PATH:-}" # Install and set up to load awscli pip install --user awscli diff --git a/scripts/valgrind.sh b/scripts/valgrind.sh new file mode 100755 index 0000000000..009e18c432 --- /dev/null +++ b/scripts/valgrind.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +set -e +set -o pipefail + +PARAMS="\ + --leak-check=full \ + --show-leak-kinds=definite \ + --errors-for-leak-kinds=definite \ + --error-exitcode=1 \ + --gen-suppressions=all \ + --suppressions=scripts/valgrind.sup" + +export VALGRIND_LIB=$(.mason/mason prefix valgrind latest)/lib/valgrind + +$(.mason/mason prefix valgrind latest)/bin/valgrind $PARAMS $@ diff --git a/scripts/valgrind.sup b/scripts/valgrind.sup index fcde049d9c..f21f54be54 100644 --- a/scripts/valgrind.sup +++ b/scripts/valgrind.sup @@ -1,91 +1,182 @@ { - Valgrind gets confused with buffers managed by the graphics driver + Graphics driver buffers Memcheck:Leak ... - obj:/usr/*/dri/r600_dri.so + obj:*/r600_dri.* + obj:*/r600_dri.* + obj:*/r600_dri.* ... } { - GLFW _glfwCreateCursor() + Graphics driver buffers Memcheck:Leak - match-leak-kinds: reachable ... - obj:/usr/*/libX11.so.6.3.0 + obj:*/i965_dri.* + obj:*/i965_dri.* + obj:*/i965_dri.* ... - fun:_glfwCreateCursor +} +{ + Graphics driver buffers + Memcheck:Leak + ... + obj:*/nouveau_dri.* + obj:*/nouveau_dri.* + obj:*/nouveau_dri.* ... } { - GLFW _glfwPlatform*() + Ubuntu 16.04 - GLFW Memcheck:Leak - match-leak-kinds: reachable ... - fun:_glfwPlatform* + fun:glfwInit ... } { - Buffer managed by OpenGL? + Ubuntu 16.04 - GLFW Memcheck:Cond ... - fun:_ZN4mbgl*Uniform* + fun:glfwInit ... - fun:_ZN8GLFWView10invalidateEv +} +{ + Ubuntu 16.04 - Qt + glib + Memcheck:Cond + fun:g_utf8_offset_to_pointer ... } { - Buffer managed by OpenGL? + Ubuntu 16.04 - Qt + glib Memcheck:Cond - fun:_ZNSt7__equalILb0EE5equalIPKfS3_EEbT_S4_T0_ ... - fun:_ZN4mbgl7Painter11renderLayerERKNS_10StyleLayerEPKNS_4Tile2IDEPKSt5arrayIfLm16EE + fun:g_signal_emit_valist + ... + fun:*QApplicationPrivate* + ... +} +{ + Ubuntu 16.04 - Qt + X11 + Memcheck:Param + writev(vector[...]) + ... + obj:*/libxcb.* + fun:xcb_flush + ... } { - Valgrind doesn't like our make_unique (C++14 polyfill) + Ubuntu 16.04 - Qt + X11 Memcheck:Leak - match-leak-kinds: reachable - fun:malloc ... - fun:_ZN4mbgl4util11make_unique* + fun:_XmbTextListToTextProperty + ... + fun:*QWidget*setVisible* ... } { - dlopen doing its magic + Ubuntu 16.04 - Qt + Fontconfig Memcheck:Leak - match-leak-kinds: reachable ... - fun:_dl_open + obj:*/libfontconfig.* + obj:*/libfontconfig.* + ... + fun:FcFontRenderPrepare ... } { - X11 false positive + Ubuntu 16.04 - Qt + Fontconfig Memcheck:Leak - match-leak-kinds: reachable ... - obj:/usr/*/libX11.so.6.3.0 + obj:*/libfontconfig.* + obj:*/libfontconfig.* ... - fun:_XrmInitParseInfo + fun:FcPatternAddInteger ... } { - OpenSSL false positive + Ubuntu 16.04 - Qt + Fontconfig Memcheck:Leak ... - fun:CRYPTO_malloc + obj:*/libfontconfig.* + obj:*/libfontconfig.* + ... + fun:FcConfigParseAndLoad ... } { - OpenSSL false positive + Ubuntu 16.04 - Qt + Dbus Memcheck:Leak ... - fun:CRYPTO_realloc + fun:dbus_connection_send_with_reply ... + fun:*QWidget* } { - Libcurl false positive + Ubuntu 16.04 - Qt + Dbus + Memcheck:Leak + ... + fun:px_proxy_factory_get_proxies + obj:*/libQt5Network.* +} +{ + Ubuntu 16.04 - Qt Memcheck:Leak - fun:malloc ... - obj:/usr/*/libcurl.so.4.3.0 - fun:curl_multi_socket_action + fun:*QGuiApplicationPrivate*createEventDispatcher* + fun:*QCoreApplication*init* + ... +} +{ + Ubuntu 16.04 - Qt + Memcheck:Leak + ... + fun:*QWidget*setWindowTitle* + ... +} +{ + Ubuntu 16.04 - Qt + Memcheck:Leak + ... + fun:*QNetworkConfigurationManagerPrivate*updateConfigurations* + ... +} +{ + Ubuntu 16.04 - Mysterious leak when running utests + Memcheck:Leak + fun:calloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* +} +{ + Ubuntu 16.04 - Mysterious leak when running utests + Memcheck:Leak + fun:malloc + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* + obj:* +} +{ + Ubuntu 14.04 - Travis CI bot using swrast + Memcheck:Cond + fun:do_stencil_test + fun:_swrast_stencil_and_ztest_span + fun:_swrast_write_rgba_span + fun:draw_wide_line ... } diff --git a/src/mbgl/geometry/buffer.hpp b/src/mbgl/geometry/buffer.hpp index 1712ca9887..6465b6f8e3 100644 --- a/src/mbgl/geometry/buffer.hpp +++ b/src/mbgl/geometry/buffer.hpp @@ -88,19 +88,6 @@ protected: return reinterpret_cast<char *>(array) + (pos - itemSize); } - // Get a pointer to the item at a given index. - inline void *getElement(size_t i) { - if (array == nullptr) { - throw std::runtime_error("Buffer was already deleted or doesn't contain elements"); - } - - if (i * itemSize >= pos) { - throw std::runtime_error("Can't get element after array bounds"); - } else { - return reinterpret_cast<char *>(array) + (i * itemSize); - } - } - public: static const size_t itemSize = item_size; diff --git a/src/mbgl/geometry/glyph_atlas.hpp b/src/mbgl/geometry/glyph_atlas.hpp index 3f9a87809f..d3e2a62199 100644 --- a/src/mbgl/geometry/glyph_atlas.hpp +++ b/src/mbgl/geometry/glyph_atlas.hpp @@ -53,7 +53,7 @@ private: BinPack<uint16_t> bin; std::unordered_map<FontStack, std::map<uint32_t, GlyphValue>, FontStackHash> index; const std::unique_ptr<uint8_t[]> data; - std::atomic<bool> dirty; + util::Atomic<bool> dirty; mbgl::optional<gl::UniqueTexture> texture; }; diff --git a/src/mbgl/gl/object_store.hpp b/src/mbgl/gl/object_store.hpp index e430214013..3ae1b6e2fe 100644 --- a/src/mbgl/gl/object_store.hpp +++ b/src/mbgl/gl/object_store.hpp @@ -20,27 +20,27 @@ class ObjectStore; struct ProgramDeleter { ObjectStore* store; - void operator()(GLuint id) const; + void operator()(GLuint) const; }; struct ShaderDeleter { ObjectStore* store; - void operator()(GLuint id) const; + void operator()(GLuint) const; }; struct BufferDeleter { ObjectStore* store; - void operator()(GLuint id) const; + void operator()(GLuint) const; }; struct TextureDeleter { ObjectStore* store; - void operator()(GLuint id) const; + void operator()(GLuint) const; }; struct VAODeleter { ObjectStore* store; - void operator()(GLuint id) const; + void operator()(GLuint) const; }; using ObjectPool = std::array<GLuint, TextureMax>; @@ -62,36 +62,36 @@ public: ~ObjectStore(); UniqueProgram createProgram() { - return UniqueProgram(MBGL_CHECK_ERROR(glCreateProgram()), { this }); + return UniqueProgram { MBGL_CHECK_ERROR(glCreateProgram()), { this } }; } UniqueShader createShader(GLenum type) { - return UniqueShader(MBGL_CHECK_ERROR(glCreateShader(type)), { this }); + return UniqueShader { MBGL_CHECK_ERROR(glCreateShader(type)), { this } }; } UniqueBuffer createBuffer() { GLuint id = 0; MBGL_CHECK_ERROR(glGenBuffers(1, &id)); - return UniqueBuffer(std::move(id), { this }); + return UniqueBuffer { std::move(id), { this } }; } UniqueTexture createTexture() { GLuint id = 0; MBGL_CHECK_ERROR(glGenTextures(1, &id)); - return UniqueTexture(std::move(id), { this }); + return UniqueTexture { std::move(id), { this } }; } UniqueVAO createVAO() { GLuint id = 0; MBGL_CHECK_ERROR(gl::GenVertexArrays(1, &id)); - return UniqueVAO(std::move(id), { this }); + return UniqueVAO { std::move(id), { this } }; } UniqueTexturePool createTexturePool() { ObjectPool ids; MBGL_CHECK_ERROR(glGenTextures(TextureMax, ids.data())); assert(ids.size() == size_t(TextureMax)); - return UniqueTexturePool(std::move(ids), { this }); + return UniqueTexturePool { std::move(ids), { this } }; } // Actually remove the objects we marked as abandoned with the above methods. diff --git a/src/mbgl/gl/texture_pool.cpp b/src/mbgl/gl/texture_pool.cpp index 8818cbef0a..c4984b0186 100644 --- a/src/mbgl/gl/texture_pool.cpp +++ b/src/mbgl/gl/texture_pool.cpp @@ -7,35 +7,68 @@ namespace mbgl { namespace gl { -GLuint TexturePool::getTextureID(gl::ObjectStore& store) { - auto nextAvailableID = [](auto& pool_) { - auto it = pool_.availableIDs.begin(); - GLuint id = *it; - pool_.availableIDs.erase(it); - return id; +class TexturePool::Impl : private util::noncopyable { +public: + class Pool : private util::noncopyable { + public: + Pool(gl::ObjectStore& store) : pool(store.createTexturePool()), availableIDs(gl::TextureMax) { + std::copy(pool.get().begin(), pool.get().end(), availableIDs.begin()); + } + + Pool(Pool&& o) : pool(std::move(o.pool)), availableIDs(std::move(o.availableIDs)) {} + Pool& operator=(Pool&& o) { pool = std::move(o.pool); availableIDs = std::move(o.availableIDs); return *this; } + + gl::UniqueTexturePool pool; + std::vector<GLuint> availableIDs; }; - for (auto& pool : pools) { - if (pool.availableIDs.empty()) continue; - return nextAvailableID(pool); - } + GLuint acquireTexture(gl::ObjectStore& store) { + auto nextAvailableID = [](auto& pool_) { + auto it = pool_.availableIDs.begin(); + GLuint id = *it; + pool_.availableIDs.erase(it); + return id; + }; - // All texture IDs are in use. - pools.emplace_back(Impl { store }); - return nextAvailableID(pools.back()); -} + for (auto& pool : pools) { + if (pool.availableIDs.empty()) continue; + return nextAvailableID(pool); + } + + // All texture IDs are in use. + pools.emplace_back(Pool { store }); + return nextAvailableID(pools.back()); + } -void TexturePool::releaseTextureID(GLuint& id) { - for (auto it = pools.begin(); it != pools.end(); ++it) { - if (std::find(it->pool.get().begin(), it->pool.get().end(), id) != it->pool.get().end()) { - it->availableIDs.push_back(id); - id = 0; - if (GLsizei(it->availableIDs.size()) == gl::TextureMax) { - pools.erase(it); + void releaseTexture(GLuint id) { + for (auto it = pools.begin(); it != pools.end(); ++it) { + if (std::find(it->pool.get().begin(), it->pool.get().end(), id) != it->pool.get().end()) { + it->availableIDs.push_back(id); + if (GLsizei(it->availableIDs.size()) == gl::TextureMax) { + pools.erase(it); + } + return; } - return; } } + +private: + std::vector<Pool> pools; +}; + +void TextureReleaser::operator()(GLuint id) const { + assert(pool); + pool->impl->releaseTexture(id); +} + +TexturePool::TexturePool() : impl(std::make_unique<Impl>()) { +} + +TexturePool::~TexturePool() { +} + +PooledTexture TexturePool::acquireTexture(gl::ObjectStore& store) { + return PooledTexture { impl->acquireTexture(store) , { this } }; } } // namespace gl diff --git a/src/mbgl/gl/texture_pool.hpp b/src/mbgl/gl/texture_pool.hpp index a32ec76746..1cdcdf220c 100644 --- a/src/mbgl/gl/texture_pool.hpp +++ b/src/mbgl/gl/texture_pool.hpp @@ -4,33 +4,34 @@ #include <mbgl/gl/gl.hpp> #include <mbgl/gl/object_store.hpp> -#include <algorithm> +#include <unique_resource.hpp> + #include <memory> -#include <vector> namespace mbgl { namespace gl { +class TexturePool; + +struct TextureReleaser { + TexturePool* pool; + void operator()(GLuint) const; +}; + +using PooledTexture = std_experimental::unique_resource<GLuint, TextureReleaser>; + class TexturePool : private util::noncopyable { public: - GLuint getTextureID(gl::ObjectStore&); - void releaseTextureID(GLuint&); - -private: - class Impl : private util::noncopyable { - public: - Impl(gl::ObjectStore& store) : pool(store.createTexturePool()), availableIDs(gl::TextureMax) { - std::copy(pool.get().begin(), pool.get().end(), availableIDs.begin()); - } + TexturePool(); + ~TexturePool(); - Impl(Impl&& o) : pool(std::move(o.pool)), availableIDs(std::move(o.availableIDs)) {} - Impl& operator=(Impl&& o) { pool = std::move(o.pool); availableIDs = std::move(o.availableIDs); return *this; } + PooledTexture acquireTexture(gl::ObjectStore&); - gl::UniqueTexturePool pool; - std::vector<GLuint> availableIDs; - }; +private: + friend TextureReleaser; - std::vector<Impl> pools; + class Impl; + const std::unique_ptr<Impl> impl; }; } // namespace gl diff --git a/src/mbgl/renderer/painter_fill.cpp b/src/mbgl/renderer/painter_fill.cpp index b89686c815..4c6ad1ba0f 100644 --- a/src/mbgl/renderer/painter_fill.cpp +++ b/src/mbgl/renderer/painter_fill.cpp @@ -55,7 +55,7 @@ void Painter::renderFill(FillBucket& bucket, outlineShader->u_matrix = vtxMatrix; config.lineWidth = 2.0f; // This is always fixed and does not depend on the pixelRatio! - outlineShader->u_color = stroke_color; + outlineShader->u_outline_color = stroke_color; outlineShader->u_opacity = opacity; // Draw the entire line @@ -184,7 +184,7 @@ void Painter::renderFill(FillBucket& bucket, outlineShader->u_matrix = vtxMatrix; config.lineWidth = 2.0f; // This is always fixed and does not depend on the pixelRatio! - outlineShader->u_color = fill_color; + outlineShader->u_outline_color = fill_color; outlineShader->u_opacity = opacity; // Draw the entire line diff --git a/src/mbgl/renderer/renderable.hpp b/src/mbgl/renderer/renderable.hpp deleted file mode 100644 index 27acfc4d0d..0000000000 --- a/src/mbgl/renderer/renderable.hpp +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#include <mbgl/util/clip_id.hpp> - -namespace mbgl { - -struct Renderable { - ClipID& clip; -}; - -} // namespace mbgl diff --git a/src/mbgl/shader/outline_shader.hpp b/src/mbgl/shader/outline_shader.hpp index 22b9013afd..25c873b4fe 100644 --- a/src/mbgl/shader/outline_shader.hpp +++ b/src/mbgl/shader/outline_shader.hpp @@ -11,10 +11,10 @@ public: void bind(GLbyte *offset) final; - UniformMatrix<4> u_matrix = {"u_matrix", *this}; - Uniform<std::array<GLfloat, 4>> u_color = {"u_color", *this}; - Uniform<GLfloat> u_opacity = {"u_opacity", *this}; - Uniform<std::array<GLfloat, 2>> u_world = {"u_world", *this}; + UniformMatrix<4> u_matrix = {"u_matrix", *this}; + Uniform<std::array<GLfloat, 4>> u_outline_color = {"u_outline_color", *this}; + Uniform<GLfloat> u_opacity = {"u_opacity", *this}; + Uniform<std::array<GLfloat, 2>> u_world = {"u_world", *this}; }; } // namespace mbgl diff --git a/src/mbgl/style/layers/line_layer_properties.hpp b/src/mbgl/style/layers/line_layer_properties.hpp index e0c63b516b..01a8534222 100644 --- a/src/mbgl/style/layers/line_layer_properties.hpp +++ b/src/mbgl/style/layers/line_layer_properties.hpp @@ -20,7 +20,7 @@ public: LayoutProperty<LineCapType> lineCap { LineCapType::Butt }; LayoutProperty<LineJoinType> lineJoin { LineJoinType::Miter }; LayoutProperty<float> lineMiterLimit { 2 }; - LayoutProperty<float> lineRoundLimit { 1.05 }; + LayoutProperty<float> lineRoundLimit { 1 }; }; class LinePaintProperties { diff --git a/src/mbgl/style/source.hpp b/src/mbgl/style/source.hpp index cadc45c25c..07e1aeaf58 100644 --- a/src/mbgl/style/source.hpp +++ b/src/mbgl/style/source.hpp @@ -4,7 +4,6 @@ #include <mbgl/tile/tile_data.hpp> #include <mbgl/tile/tile_cache.hpp> #include <mbgl/style/types.hpp> -#include <mbgl/renderer/renderable.hpp> #include <mbgl/util/mat4.hpp> #include <mbgl/util/rapidjson.hpp> diff --git a/src/mbgl/util/assert.hpp b/src/mbgl/util/assert.hpp deleted file mode 100644 index 47fc13f94e..0000000000 --- a/src/mbgl/util/assert.hpp +++ /dev/null @@ -1,10 +0,0 @@ -#pragma once - -#include <cassert> - -// Based on FreeBSD's src/include/assert.h -// Licensed under the original BSD license -#define assert_always(expr) \ - ((void)((expr) ? ((void)0) : ((void)fprintf(stderr, "%s:%u: failed assertion `%s'\n", \ - __FILE__, __LINE__, #expr), \ - abort()))) diff --git a/src/mbgl/util/channel.hpp b/src/mbgl/util/channel.hpp deleted file mode 100644 index 319205b444..0000000000 --- a/src/mbgl/util/channel.hpp +++ /dev/null @@ -1,36 +0,0 @@ -#pragma once - -#include <mbgl/util/noncopyable.hpp> - -#include <mutex> -#include <condition_variable> -#include <queue> - -namespace mbgl { - -template <class T> -class Channel : public mbgl::util::noncopyable { -public: - void send(const T& t) { - std::unique_lock<std::mutex> lock(mutex); - queue.push(t); - condition.notify_one(); - } - - T receive() { - std::unique_lock<std::mutex> lock(mutex); - condition.wait(lock, [&](){ return !queue.empty(); }); - - T t = queue.front(); - queue.pop(); - - return t; - } - -private: - std::mutex mutex; - std::condition_variable condition; - std::queue<T> queue; -}; - -} diff --git a/src/mbgl/util/raster.cpp b/src/mbgl/util/raster.cpp index 70fdf7b02a..3146a00513 100644 --- a/src/mbgl/util/raster.cpp +++ b/src/mbgl/util/raster.cpp @@ -13,12 +13,6 @@ Raster::Raster(gl::TexturePool& texturePool_) : texturePool(texturePool_) {} -Raster::~Raster() { - if (textured) { - texturePool.releaseTextureID(textureID); - } -} - bool Raster::isLoaded() const { std::lock_guard<std::mutex> lock(mtx); return loaded; @@ -42,10 +36,10 @@ void Raster::bind(bool linear, gl::ObjectStore& store) { return; } - if (img.data && !textured) { + if (img.data && !texture) { upload(store); - } else if (textured) { - MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, textureID)); + } else if (texture) { + MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, *texture)); } GLint new_filter = linear ? GL_LINEAR : GL_NEAREST; @@ -57,15 +51,15 @@ void Raster::bind(bool linear, gl::ObjectStore& store) { } void Raster::upload(gl::ObjectStore& store) { - if (img.data && !textured) { - textureID = texturePool.getTextureID(store); - MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, textureID)); + if (img.data && !texture) { + texture = texturePool.acquireTexture(store); + MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, *texture)); #ifndef GL_ES_VERSION_2_0 MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0)); #endif MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE)); MBGL_CHECK_ERROR(glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE)); - MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, img.data.release())); - textured = true; + MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, img.data.get())); + img.data.reset(); } } diff --git a/src/mbgl/util/raster.hpp b/src/mbgl/util/raster.hpp index ff16112d90..b539032d81 100644 --- a/src/mbgl/util/raster.hpp +++ b/src/mbgl/util/raster.hpp @@ -5,6 +5,7 @@ #include <mbgl/util/image.hpp> #include <mbgl/util/ptr.hpp> #include <mbgl/util/chrono.hpp> +#include <mbgl/util/optional.hpp> #include <mutex> @@ -14,7 +15,6 @@ class Raster : public std::enable_shared_from_this<Raster> { public: Raster(gl::TexturePool&); - ~Raster(); // load image data void load(PremultipliedImage); @@ -33,11 +33,8 @@ public: GLsizei width = 0; GLsizei height = 0; - // has been uploaded to texture - bool textured = false; - - // the uploaded texture - GLuint textureID = 0; + // GL buffer object handle. + mbgl::optional<gl::PooledTexture> texture; // texture opacity double opacity = 0; diff --git a/test/gl/object.cpp b/test/gl/object.cpp index 4511fc4d1c..f2b21ec9f4 100644 --- a/test/gl/object.cpp +++ b/test/gl/object.cpp @@ -130,29 +130,31 @@ TEST(GLObject, TexturePool) { mbgl::gl::TexturePool pool; + std::vector<mbgl::gl::PooledTexture> ids; + // Fill an entire texture pool. for (auto i = 0; i != mbgl::gl::TextureMax; ++i) { - EXPECT_EQ(pool.getTextureID(store), GLuint(i + 1)); + ids.push_back(pool.acquireTexture(store)); + EXPECT_EQ(ids.back().get(), GLuint(i + 1)); EXPECT_TRUE(store.empty()); } // Reuse texture ids from the same pool. for (auto i = 0; i != mbgl::gl::TextureMax; ++i) { - GLuint id = i + 1; - pool.releaseTextureID(id); - EXPECT_EQ(id, 0); - EXPECT_EQ(pool.getTextureID(store), GLuint(i + 1)); + ids[i].reset(); + ids.push_back(pool.acquireTexture(store)); + EXPECT_EQ(ids.back().get(), GLuint(i + 1)); EXPECT_TRUE(store.empty()); } // Trigger a new texture pool creation. { - GLuint id = pool.getTextureID(store); + mbgl::gl::PooledTexture id = pool.acquireTexture(store); EXPECT_EQ(id, mbgl::gl::TextureMax + 1); EXPECT_TRUE(store.empty()); - pool.releaseTextureID(id); - EXPECT_EQ(id, 0); + id.reset(); + // Last used texture from pool triggers pool recycling. EXPECT_FALSE(store.empty()); @@ -161,35 +163,26 @@ TEST(GLObject, TexturePool) { } // First pool is still full, thus creating a new pool. - GLuint id1 = pool.getTextureID(store); - EXPECT_GT(id1, mbgl::gl::TextureMax); + mbgl::gl::PooledTexture id1 = pool.acquireTexture(store); + EXPECT_GT(id1.get(), mbgl::gl::TextureMax); EXPECT_TRUE(store.empty()); // Release all textures from the first pool. - for (auto i = 0; i != mbgl::gl::TextureMax; ++i) { - GLuint id = i + 1; - pool.releaseTextureID(id); - if (i == mbgl::gl::TextureMax - 1) { - // Last texture from pool triggers pool recycling. - EXPECT_FALSE(store.empty()); - } else { - EXPECT_TRUE(store.empty()); - } - } + ids.clear(); + EXPECT_FALSE(store.empty()); store.performCleanup(); EXPECT_TRUE(store.empty()); // The first pool is now gone, the next pool is now in use. - GLuint id2 = pool.getTextureID(store); - EXPECT_GT(id2, id1); - pool.releaseTextureID(id2); - EXPECT_EQ(id2, 0); + mbgl::gl::PooledTexture id2 = pool.acquireTexture(store); + EXPECT_GT(id2.get(), id1.get()); + + id2.reset(); EXPECT_TRUE(store.empty()); // Last used texture from the pool triggers pool recycling. - pool.releaseTextureID(id1); - EXPECT_EQ(id1, 0); + id1.reset(); EXPECT_FALSE(store.empty()); store.performCleanup(); diff --git a/test/storage/asset_file_source.cpp b/test/storage/asset_file_source.cpp index 64f7254807..ab214ff793 100644 --- a/test/storage/asset_file_source.cpp +++ b/test/storage/asset_file_source.cpp @@ -21,7 +21,7 @@ std::string getFileSourceRoot() { using namespace mbgl; -TEST(AssetFileSource, Stress) { +TEST(AssetFileSource, Load) { util::RunLoop loop; AssetFileSource fs(getFileSourceRoot()); diff --git a/test/test.gypi b/test/test.gypi index f9a25e4e86..b7e7b5c32b 100644 --- a/test/test.gypi +++ b/test/test.gypi @@ -18,7 +18,6 @@ ], 'sources': [ - 'util/assert.cpp', 'util/async_task.cpp', 'util/geo.cpp', 'util/image.cpp', diff --git a/test/util/assert.cpp b/test/util/assert.cpp deleted file mode 100644 index cf7baf8ca9..0000000000 --- a/test/util/assert.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include <mbgl/test/util.hpp> - -#include <mbgl/util/assert.hpp> - -using namespace mbgl; - -TEST(Assert, Always) { - EXPECT_DEATH_IF_SUPPORTED(assert_always(true == false), "failed assertion `true == false'"); -} |