diff --git a/.mason b/.mason index ec5ff7cc3..0d0689065 160000 --- a/.mason +++ b/.mason @@ -1 +1 @@ -Subproject commit ec5ff7cc35d1dec325a00f4d8feff7fa3f6da2ab +Subproject commit 0d06890655b5f2995682ea5b73aeb94010f8c898 diff --git a/.travis.yml b/.travis.yml index eb779e245..ddc0c3ee5 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 d0f063840..b16dcafb4 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. diff --git a/Makefile b/Makefile index ad1ebba79..3f8332938 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/configure b/configure index 69acbde01..67148de71 100755 --- a/configure +++ b/configure @@ -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 fbb0686a7..124cdca7c 100644 --- a/include/mbgl/util/image.hpp +++ b/include/mbgl/util/image.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -12,7 +14,7 @@ enum ImageAlphaMode { }; template -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 b8d31d9cd..0b7f7e2d1 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 37d23da11..5b0eaf474 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 webp(WebPDecodeRGBA(data, size, &width, &height)); - if (!webp) { + int stride = width * 4; + size_t webpSize = stride * height; + auto webp = std::make_unique(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 8c2eeca35..2b34571d1 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 67481d4f8..d771f7541 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 -“iosapp” is only one of several shared schemes. Others include: +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. -* “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. +**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. 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 -* `make ipackage` builds both dynamic and static frameworks in the Release configuration for devices and the iOS Simulator. +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 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 a29e9467e..3642d8b7d 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 33b83e537..da9213dba 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 70f777422..8317aee1d 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)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 __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)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 )annotation { return YES; diff --git a/platform/ios/src/MGLMapView.h b/platform/ios/src/MGLMapView.h index d02c3a93a..63d799bda 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 1ad758dc7..5d1bcb110 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 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 a8f1d5be7..e1bff6cc2 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 8a4323aa8..63b894722 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 427412674..e9f3b9959 100644 --- a/platform/osx/app/MapDocument.m +++ b/platform/osx/app/MapDocument.m @@ -18,7 +18,18 @@ NS_ARRAY_OF(id ) *MBXFlattenedShapes(NS_ARRAY_OF(id 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 5c7e75135..7b3efd293 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 03b94c25b..ac4eae9d3 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 4b855f7c5..5f31c853d 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 60e855212..3b02cdc88 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 0f2dcdc42..637959844 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 e9fa87811..dfd045224 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 000000000..009e18c43 --- /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 fcde049d9..f21f54be5 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 - ... - fun:_glfwCreateCursor + obj:*/i965_dri.* + obj:*/i965_dri.* + obj:*/i965_dri.* ... } { - GLFW _glfwPlatform*() + Graphics driver buffers Memcheck:Leak - match-leak-kinds: reachable ... - fun:_glfwPlatform* + obj:*/nouveau_dri.* + obj:*/nouveau_dri.* + obj:*/nouveau_dri.* ... } { - Buffer managed by OpenGL? + Ubuntu 16.04 - GLFW + Memcheck:Leak + ... + fun:glfwInit + ... +} +{ + Ubuntu 16.04 - GLFW Memcheck:Cond ... - fun:_ZN4mbgl*Uniform* - ... - fun:_ZN8GLFWView10invalidateEv + fun:glfwInit ... } { - Buffer managed by OpenGL? + Ubuntu 16.04 - Qt + glib 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* + fun:g_utf8_offset_to_pointer ... } { - dlopen doing its magic - Memcheck:Leak - match-leak-kinds: reachable + Ubuntu 16.04 - Qt + glib + Memcheck:Cond ... - fun:_dl_open + fun:g_signal_emit_valist + ... + fun:*QApplicationPrivate* ... } { - X11 false positive - Memcheck:Leak - match-leak-kinds: reachable + Ubuntu 16.04 - Qt + X11 + Memcheck:Param + writev(vector[...]) ... - obj:/usr/*/libX11.so.6.3.0 - ... - fun:_XrmInitParseInfo + obj:*/libxcb.* + fun:xcb_flush ... } { - OpenSSL false positive + Ubuntu 16.04 - Qt + X11 Memcheck:Leak ... - fun:CRYPTO_malloc + fun:_XmbTextListToTextProperty + ... + fun:*QWidget*setVisible* ... } { - OpenSSL false positive + Ubuntu 16.04 - Qt + Fontconfig Memcheck:Leak ... - fun:CRYPTO_realloc + obj:*/libfontconfig.* + obj:*/libfontconfig.* + ... + fun:FcFontRenderPrepare ... } { - Libcurl false positive + Ubuntu 16.04 - Qt + Fontconfig + Memcheck:Leak + ... + obj:*/libfontconfig.* + obj:*/libfontconfig.* + ... + fun:FcPatternAddInteger + ... +} +{ + Ubuntu 16.04 - Qt + Fontconfig + Memcheck:Leak + ... + obj:*/libfontconfig.* + obj:*/libfontconfig.* + ... + fun:FcConfigParseAndLoad + ... +} +{ + Ubuntu 16.04 - Qt + Dbus + Memcheck:Leak + ... + fun:dbus_connection_send_with_reply + ... + fun:*QWidget* +} +{ + Ubuntu 16.04 - Qt + Dbus + Memcheck:Leak + ... + fun:px_proxy_factory_get_proxies + obj:*/libQt5Network.* +} +{ + Ubuntu 16.04 - Qt + Memcheck:Leak + ... + 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:/usr/*/libcurl.so.4.3.0 - fun:curl_multi_socket_action + 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 1712ca988..6465b6f8e 100644 --- a/src/mbgl/geometry/buffer.hpp +++ b/src/mbgl/geometry/buffer.hpp @@ -88,19 +88,6 @@ protected: return reinterpret_cast(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(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 3f9a87809..d3e2a6219 100644 --- a/src/mbgl/geometry/glyph_atlas.hpp +++ b/src/mbgl/geometry/glyph_atlas.hpp @@ -53,7 +53,7 @@ private: BinPack bin; std::unordered_map, FontStackHash> index; const std::unique_ptr data; - std::atomic dirty; + util::Atomic dirty; mbgl::optional texture; }; diff --git a/src/mbgl/gl/object_store.hpp b/src/mbgl/gl/object_store.hpp index e43021401..3ae1b6e2f 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; @@ -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 8818cbef0..c4984b018 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 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; + }; + + 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()); } - // All texture IDs are in use. - pools.emplace_back(Impl { 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 pools; +}; + +void TextureReleaser::operator()(GLuint id) const { + assert(pool); + pool->impl->releaseTexture(id); +} + +TexturePool::TexturePool() : impl(std::make_unique()) { +} + +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 a32ec7674..1cdcdf220 100644 --- a/src/mbgl/gl/texture_pool.hpp +++ b/src/mbgl/gl/texture_pool.hpp @@ -4,33 +4,34 @@ #include #include -#include +#include + #include -#include namespace mbgl { namespace gl { +class TexturePool; + +struct TextureReleaser { + TexturePool* pool; + void operator()(GLuint) const; +}; + +using PooledTexture = std_experimental::unique_resource; + class TexturePool : private util::noncopyable { public: - GLuint getTextureID(gl::ObjectStore&); - void releaseTextureID(GLuint&); + TexturePool(); + ~TexturePool(); + + PooledTexture acquireTexture(gl::ObjectStore&); 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()); - } + friend TextureReleaser; - 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; } - - gl::UniqueTexturePool pool; - std::vector availableIDs; - }; - - std::vector pools; + class Impl; + const std::unique_ptr impl; }; } // namespace gl diff --git a/src/mbgl/renderer/painter_fill.cpp b/src/mbgl/renderer/painter_fill.cpp index b89686c81..4c6ad1ba0 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 27acfc4d0..000000000 --- a/src/mbgl/renderer/renderable.hpp +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#include - -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 22b9013af..25c873b4f 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> u_color = {"u_color", *this}; - Uniform u_opacity = {"u_opacity", *this}; - Uniform> u_world = {"u_world", *this}; + UniformMatrix<4> u_matrix = {"u_matrix", *this}; + Uniform> u_outline_color = {"u_outline_color", *this}; + Uniform u_opacity = {"u_opacity", *this}; + Uniform> 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 e0c63b516..01a853422 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 lineCap { LineCapType::Butt }; LayoutProperty lineJoin { LineJoinType::Miter }; LayoutProperty lineMiterLimit { 2 }; - LayoutProperty lineRoundLimit { 1.05 }; + LayoutProperty lineRoundLimit { 1 }; }; class LinePaintProperties { diff --git a/src/mbgl/style/source.hpp b/src/mbgl/style/source.hpp index cadc45c25..07e1aeaf5 100644 --- a/src/mbgl/style/source.hpp +++ b/src/mbgl/style/source.hpp @@ -4,7 +4,6 @@ #include #include #include -#include #include #include diff --git a/src/mbgl/util/assert.hpp b/src/mbgl/util/assert.hpp deleted file mode 100644 index 47fc13f94..000000000 --- a/src/mbgl/util/assert.hpp +++ /dev/null @@ -1,10 +0,0 @@ -#pragma once - -#include - -// 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 319205b44..000000000 --- a/src/mbgl/util/channel.hpp +++ /dev/null @@ -1,36 +0,0 @@ -#pragma once - -#include - -#include -#include -#include - -namespace mbgl { - -template -class Channel : public mbgl::util::noncopyable { -public: - void send(const T& t) { - std::unique_lock lock(mutex); - queue.push(t); - condition.notify_one(); - } - - T receive() { - std::unique_lock 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 queue; -}; - -} diff --git a/src/mbgl/util/raster.cpp b/src/mbgl/util/raster.cpp index 70fdf7b02..3146a0051 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 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 ff16112d9..b539032d8 100644 --- a/src/mbgl/util/raster.hpp +++ b/src/mbgl/util/raster.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -14,7 +15,6 @@ class Raster : public std::enable_shared_from_this { 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 texture; // texture opacity double opacity = 0; diff --git a/test/gl/object.cpp b/test/gl/object.cpp index 4511fc4d1..f2b21ec9f 100644 --- a/test/gl/object.cpp +++ b/test/gl/object.cpp @@ -130,29 +130,31 @@ TEST(GLObject, TexturePool) { mbgl::gl::TexturePool pool; + std::vector 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 64f725480..ab214ff79 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 f9a25e4e8..b7e7b5c32 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 cf7baf8ca..000000000 --- a/test/util/assert.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include - -#include - -using namespace mbgl; - -TEST(Assert, Always) { - EXPECT_DEATH_IF_SUPPORTED(assert_always(true == false), "failed assertion `true == false'"); -}