[core, ios] Improve performance of queryRenderedFeatures by reducing # of temp GeometryCollection

platform/ios/CHANGELOG.md

@@ -8,6 +8,9 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 
 * Fixed flickering on style change for the same tile set ([#15127](https://github.com/mapbox/mapbox-gl-native/pull/15127))
 
+### Other changes
+
+* Improved feature querying performance (focussed on roads). ([#15183](https://github.com/mapbox/mapbox-gl-native/pull/15183))
 
 ## 5.2.0
 

platform/ios/app/MBXViewController.m

@@ -17,6 +17,8 @@
 #import "../src/MGLMapView_Experimental.h"
 
 #import <objc/runtime.h>
+#import <os/log.h>
+#import <os/signpost.h>
 
 static const CLLocationCoordinate2D WorldTourDestinations[] = {
     { .latitude = 38.8999418, .longitude = -77.033996 },
@@ -63,6 +65,7 @@ typedef NS_ENUM(NSInteger, MBXSettingsAnnotationsRows) {
     MBXSettingsAnnotationsTestShapes,
     MBXSettingsAnnotationsCustomCallout,
     MBXSettingsAnnotationsQueryAnnotations,
+    MBXSettingsAnnotationsQueryRoadsAroundDC,
     MBXSettingsAnnotationsCustomUserDot,
     MBXSettingsAnnotationsRemoveAnnotations,
     MBXSettingsAnnotationSelectRandomOffscreenPointAnnotation,
@@ -215,10 +218,36 @@ @interface MBXViewController () <UITableViewDelegate,
 @property (nonatomic) NSMutableArray<UIWindow *> *helperWindows;
 @property (nonatomic) NSMutableArray<UIView *> *contentInsetsOverlays;
 
+@property (nonatomic) os_log_t log;
+@property (nonatomic) os_signpost_id_t signpost;
+@property (nonatomic) NSMutableArray<dispatch_block_t> *pendingIdleBlocks;
+
 @end
 
+#define OS_SIGNPOST_BEGIN_WITH_SELF(self, name) \
+    if (@available(iOS 12.0, *)) { os_signpost_interval_begin(self.log, self.signpost, name); }
+
+#define OS_SIGNPOST_END_WITH_SELF(self, name) \
+    if (@available(iOS 12.0, *)) { os_signpost_interval_end(self.log, self.signpost, name); }
+
+#define OS_SIGNPOST_BEGIN(name) \
+    OS_SIGNPOST_BEGIN_WITH_SELF(self, name)
+
+#define OS_SIGNPOST_END(name) \
+    OS_SIGNPOST_END_WITH_SELF(self, name)
+
+#define OS_SIGNPOST_EVENT(name, ...) \
+    if (@available(iOS 12.0, *)) { os_signpost_event_emit(self.log, self.signpost, name, ##__VA_ARGS__); }
+
+// Expose properties for testing
 @interface MGLMapView (MBXViewController)
 @property (nonatomic) NSDictionary *annotationViewReuseQueueByIdentifier;
+- (void)setNeedsRerender;
+- (UIEdgeInsets)defaultEdgeInsetsForShowAnnotations;
+@end
+
+@interface MGLStyle (MBXViewController)
+@property (nonatomic, readonly, copy) NSArray<MGLVectorStyleLayer *> *roadStyleLayers;
 @end
 
 @implementation MBXViewController
@@ -234,6 +263,12 @@ - (void)viewDidLoad
 {
     [super viewDidLoad];
 
+    self.pendingIdleBlocks = [NSMutableArray array];
+    self.log = os_log_create("com.mapbox.iosapp", "MBXViewController");
+    if (@available(iOS 12.0, *)) {
+        self.signpost = os_signpost_id_generate(self.log);
+    }
+
     // Keep track of current map state and debug preferences,
     // saving and restoring when the application's state changes.
     self.currentState =  [MBXStateManager sharedManager].currentState;
@@ -390,6 +425,7 @@ - (void)dismissSettings:(__unused id)sender
                 @"Add Test Shapes",
                 @"Add Point With Custom Callout",
                 @"Query Annotations",
+                @"Query Roads around DC",
                 [NSString stringWithFormat:@"%@ Custom User Dot", (_customUserLocationAnnnotationEnabled ? @"Disable" : @"Enable")],
                 @"Remove Annotations",
                 @"Select an offscreen point annotation",
@@ -520,22 +556,22 @@ - (void)performActionForSettingAtIndexPath:(NSIndexPath *)indexPath
             switch (indexPath.row)
             {
                 case MBXSettingsAnnotations100Views:
-                    [self parseFeaturesAddingCount:100 usingViews:YES];
+                    [self parseFeaturesAddingCount:100 usingViews:YES completionHandler:NULL];
                     break;
                 case MBXSettingsAnnotations1000Views:
-                    [self parseFeaturesAddingCount:1000 usingViews:YES];
+                    [self parseFeaturesAddingCount:1000 usingViews:YES completionHandler:NULL];
                     break;
                 case MBXSettingsAnnotations10000Views:
-                    [self parseFeaturesAddingCount:10000 usingViews:YES];
+                    [self parseFeaturesAddingCount:10000 usingViews:YES completionHandler:NULL];
                     break;
                 case MBXSettingsAnnotations100Sprites:
-                    [self parseFeaturesAddingCount:100 usingViews:NO];
+                    [self parseFeaturesAddingCount:100 usingViews:NO completionHandler:NULL];
                     break;
                 case MBXSettingsAnnotations1000Sprites:
-                    [self parseFeaturesAddingCount:1000 usingViews:NO];
+                    [self parseFeaturesAddingCount:1000 usingViews:NO completionHandler:NULL];
                     break;
                 case MBXSettingsAnnotations10000Sprites:
-                    [self parseFeaturesAddingCount:10000 usingViews:NO];
+                    [self parseFeaturesAddingCount:10000 usingViews:NO completionHandler:NULL];
                     break;
                 case MBXSettingsAnnotationAnimation:
                     [self animateAnnotationView];
@@ -549,6 +585,25 @@ - (void)performActionForSettingAtIndexPath:(NSIndexPath *)indexPath
                 case MBXSettingsAnnotationsQueryAnnotations:
                     [self testQueryPointAnnotations];
                     break;
+
+                case MBXSettingsAnnotationsQueryRoadsAroundDC:
+                {
+                    __weak __typeof__(self) weakSelf = self;
+                    [self parseFeaturesAddingCount:100 usingViews:YES completionHandler:^{
+                        [weakSelf.mapView setNeedsRerender];
+                        [weakSelf.pendingIdleBlocks addObject:^{
+                            __typeof__(self) strongSelf = weakSelf;
+                            NSLog(@"BEGIN: query-roads-batch");
+                            OS_SIGNPOST_BEGIN_WITH_SELF(strongSelf, "query-roads-batch");
+                            for (int i = 0; i < 10; i++) {
+                                [strongSelf queryRoads];
+                            }
+                            OS_SIGNPOST_END_WITH_SELF(strongSelf, "query-roads-batch");
+                            NSLog(@"END: query-roads-batch");
+                        }];
+                    }];
+                }
+                    break;
                 case MBXSettingsAnnotationsCustomUserDot:
                     [self toggleCustomUserDot];
                     break;
@@ -815,7 +870,7 @@ - (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath
 
 #pragma mark - Debugging Actions
 
-- (void)parseFeaturesAddingCount:(NSUInteger)featuresCount usingViews:(BOOL)useViews
+- (void)parseFeaturesAddingCount:(NSUInteger)featuresCount usingViews:(BOOL)useViews completionHandler:(nullable dispatch_block_t)completion
 {
     [self.mapView removeAnnotations:self.mapView.annotations];
 
@@ -850,7 +905,11 @@ - (void)parseFeaturesAddingCount:(NSUInteger)featuresCount usingViews:(BOOL)useV
             dispatch_async(dispatch_get_main_queue(), ^
             {
                 [self.mapView addAnnotations:annotations];
-                [self.mapView showAnnotations:annotations animated:YES];
+                UIEdgeInsets insets = [self.mapView defaultEdgeInsetsForShowAnnotations];
+                [self.mapView showAnnotations:annotations
+                                  edgePadding:insets
+                                     animated:YES
+                            completionHandler:completion];
             });
         }
     });
@@ -1765,6 +1824,21 @@ - (NSString *)telemetryDebugLogFilePath
     return filePath;
 }
 
+#pragma mark - Query Rendered Features
+
+- (void)queryRoads
+{
+    OS_SIGNPOST_BEGIN("query-roads");
+
+    NSArray *roadStyleLayerIdentifiers = [self.mapView.style.roadStyleLayers valueForKey:@"identifier"];
+    NSArray *visibleRoadFeatures = [self.mapView visibleFeaturesInRect:self.mapView.bounds inStyleLayersWithIdentifiers:[NSSet setWithArray:roadStyleLayerIdentifiers]];
+
+    OS_SIGNPOST_END("query-roads");
+    OS_SIGNPOST_EVENT("query-roads-count", "%lu", (unsigned long)visibleRoadFeatures.count);
+
+    NSLog(@"Roads & labels feature count: %lu", (unsigned long)visibleRoadFeatures.count);
+}
+
 #pragma mark - Random World Tour
 
 - (void)addAnnotations:(NSInteger)numAnnotations aroundCoordinate:(CLLocationCoordinate2D)coordinate radius:(CLLocationDistance)radius {
@@ -1989,6 +2063,7 @@ - (IBAction)cycleStyles:(__unused id)sender
                 }
             }
         }
+        free(methods);
         NSAssert(numStyleURLMethods == styleNames.count,
                  @"MGLStyle provides %u default styles but iosapp only knows about %lu of them.",
                  numStyleURLMethods, (unsigned long)styleNames.count);
@@ -2047,6 +2122,16 @@ - (void)viewWillTransitionToSize:(CGSize)size withTransitionCoordinator:(id<UIVi
 
 #pragma mark - MGLMapViewDelegate
 
+- (void)mapViewDidBecomeIdle:(MGLMapView *)mapView
+{
+    NSArray *blocks = [self.pendingIdleBlocks copy];
+    [self.pendingIdleBlocks removeAllObjects];
+
+    for (dispatch_block_t block in blocks) {
+        block();
+    }
+}
+
 - (MGLAnnotationView *)mapView:(MGLMapView *)mapView viewForAnnotation:(id<MGLAnnotation>)annotation
 {
     if (annotation == mapView.userLocation)

platform/ios/ios.xcodeproj/xcshareddata/xcschemes/iosapp.xcscheme

@@ -65,7 +65,7 @@
       </AdditionalOptions>
    </LaunchAction>
    <ProfileAction
-      buildConfiguration = "Release"
+      buildConfiguration = "RelWithDebInfo"
       shouldUseLaunchSchemeArgsEnv = "YES"
       savedToolIdentifier = ""
       useCustomWorkingDirectory = "NO"

platform/ios/src/MGLMapView.mm

@@ -5050,14 +5050,19 @@ - (void)calloutViewWillAppear:(UIView <MGLCalloutView> *)calloutView
                      completion:NULL];
 }
 
-- (void)showAnnotations:(NSArray<id <MGLAnnotation>> *)annotations animated:(BOOL)animated
+- (UIEdgeInsets)defaultEdgeInsetsForShowAnnotations
 {
     CGFloat maximumPadding = 100;
     CGFloat yPadding = (self.frame.size.height / 5 <= maximumPadding) ? (self.frame.size.height / 5) : maximumPadding;
     CGFloat xPadding = (self.frame.size.width / 5 <= maximumPadding) ? (self.frame.size.width / 5) : maximumPadding;
-
+    
     UIEdgeInsets edgeInsets = UIEdgeInsetsMake(yPadding, xPadding, yPadding, xPadding);
+    return edgeInsets;
+}
 
+- (void)showAnnotations:(NSArray<id <MGLAnnotation>> *)annotations animated:(BOOL)animated
+{
+    UIEdgeInsets edgeInsets = [self defaultEdgeInsetsForShowAnnotations];
     [self showAnnotations:annotations edgePadding:edgeInsets animated:animated completionHandler:nil];
 }
 

platform/macos/CHANGELOG.md

@@ -18,13 +18,11 @@
 
 * The `-[MGLMapView setCamera:withDuration:animationTimingFunction:edgePadding:completionHandler:]` method now adds the current value of the `MGLMapView.contentInsets` property to the `edgePadding` parameter. ([#14813](https://github.com/mapbox/mapbox-gl-native/pull/14813))
 * Updated "map ID" to the more accurate term "tileset ID" in documentation; updated "style's Map ID" to the more accurate term "style URL". ([#15116](https://github.com/mapbox/mapbox-gl-native/pull/15116))
-
-### Other changes
-
 * Added variants of multiple animated `MGLMapView` methods that accept completion handlers ([#14381](https://github.com/mapbox/mapbox-gl-native/pull/14381)):
   * `-[MGLMapView setVisibleCoordinateBounds:edgePadding:animated:completionHandler:]`
   * `-[MGLMapView setContentInsets:animated:completionHandler:]`
   * `-[MGLMapView showAnnotations:edgePadding:animated:completionHandler:]`
+* Improved feature querying performance (focussed on roads). ([#15183](https://github.com/mapbox/mapbox-gl-native/pull/15183))  
 
 ## 0.14.0 - May 22, 2018
 

src/mbgl/geometry/feature_index.cpp

@@ -149,17 +149,19 @@ void FeatureIndex::addFeature(
             assert(geometryTileFeature);
         }
 
+        const GeometryCollection& geometries = geometryTileFeature->getGeometries();
+
         bool needsCrossTileIndex = renderLayer->baseImpl->getTypeInfo()->crossTileIndex == style::LayerTypeInfo::CrossTileIndex::Required;
         if (!needsCrossTileIndex &&
-             !renderLayer->queryIntersectsFeature(queryGeometry, *geometryTileFeature, tileID.z, transformState, pixelsToTileUnits, posMatrix)) {
+             !renderLayer->queryIntersectsFeature(queryGeometry, *geometryTileFeature, geometries, tileID.z, transformState, pixelsToTileUnits, posMatrix)) {
             continue;
         }
 
         if (options.filter && !(*options.filter)(style::expression::EvaluationContext { static_cast<float>(tileID.z), geometryTileFeature.get() })) {
             continue;
         }
 
-        result[layerID].push_back(convertFeature(*geometryTileFeature, tileID));
+        result[layerID].emplace_back(convertFeature(*geometryTileFeature, geometries, tileID));
     }
 }
 

src/mbgl/renderer/layers/render_circle_layer.cpp

@@ -139,6 +139,7 @@ GeometryCoordinates projectQueryGeometry(const GeometryCoordinates& queryGeometr
 bool RenderCircleLayer::queryIntersectsFeature(
         const GeometryCoordinates& queryGeometry,
         const GeometryTileFeature& feature,
+        const GeometryCollection&, 
         const float zoom,
         const TransformState& transformState,
         const float pixelsToTileUnits,

src/mbgl/renderer/layers/render_circle_layer.hpp

@@ -21,6 +21,7 @@ class RenderCircleLayer final : public RenderLayer {
     bool queryIntersectsFeature(
             const GeometryCoordinates&,
             const GeometryTileFeature&,
+            const GeometryCollection&, 
             const float,
             const TransformState&,
             const float,

src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp

@@ -222,6 +222,7 @@ void RenderFillExtrusionLayer::render(PaintParameters& parameters) {
 bool RenderFillExtrusionLayer::queryIntersectsFeature(
         const GeometryCoordinates& queryGeometry,
         const GeometryTileFeature& feature,
+        const GeometryCollection&,
         const float,
         const TransformState& transformState,
         const float pixelsToTileUnits,

src/mbgl/renderer/layers/render_fill_extrusion_layer.hpp

@@ -23,6 +23,7 @@ class RenderFillExtrusionLayer final : public RenderLayer {
     bool queryIntersectsFeature(
         const GeometryCoordinates&,
         const GeometryTileFeature&,
+        const GeometryCollection&,
         const float,
         const TransformState&,
         const float,

src/mbgl/renderer/layers/render_fill_layer.cpp

@@ -252,6 +252,7 @@ void RenderFillLayer::render(PaintParameters& parameters) {
 bool RenderFillLayer::queryIntersectsFeature(
         const GeometryCoordinates& queryGeometry,
         const GeometryTileFeature& feature,
+        const GeometryCollection&,
         const float,
         const TransformState& transformState,
         const float pixelsToTileUnits,

src/mbgl/renderer/layers/render_fill_layer.hpp

@@ -24,6 +24,7 @@ class RenderFillLayer final : public RenderLayer {
     bool queryIntersectsFeature(
             const GeometryCoordinates&,
             const GeometryTileFeature&,
+            const GeometryCollection&,
             const float,
             const TransformState&,
             const float,

src/mbgl/renderer/layers/render_heatmap_layer.cpp

@@ -217,6 +217,7 @@ void RenderHeatmapLayer::updateColorRamp() {
 bool RenderHeatmapLayer::queryIntersectsFeature(
         const GeometryCoordinates& queryGeometry,
         const GeometryTileFeature& feature,
+        const GeometryCollection&,
         const float zoom,
         const TransformState&,
         const float pixelsToTileUnits,

src/mbgl/renderer/layers/render_heatmap_layer.hpp

@@ -25,6 +25,7 @@ class RenderHeatmapLayer final : public RenderLayer {
     bool queryIntersectsFeature(
             const GeometryCoordinates&,
             const GeometryTileFeature&,
+            const GeometryCollection&,
             const float,
             const TransformState&,
             const float,

src/mbgl/renderer/layers/render_line_layer.cpp

@@ -214,14 +214,16 @@ void RenderLineLayer::render(PaintParameters& parameters) {
     }
 }
 
-optional<GeometryCollection> offsetLine(const GeometryCollection& rings, const double offset) {
-    if (offset == 0) return {};
+std::unique_ptr<GeometryCollection> offsetLine(const GeometryCollection& rings, const double offset) {
+
+    if (offset == 0) return nullptr;
+
+    std::unique_ptr<GeometryCollection> newRings = std::make_unique<GeometryCollection>();
 
-    GeometryCollection newRings;
     Point<double> zero(0, 0);
     for (const auto& ring : rings) {
-        newRings.emplace_back();
-        auto& newRing = newRings.back();
+        newRings->emplace_back();
+        auto& newRing = newRings->back();
 
         for (auto i = ring.begin(); i != ring.end(); i++) {
             auto& p = *i;
@@ -237,7 +239,7 @@ optional<GeometryCollection> offsetLine(const GeometryCollection& rings, const d
             const double cosHalfAngle = extrude.x * bToC.x + extrude.y * bToC.y;
             extrude *= (1.0 / cosHalfAngle);
 
-            newRing.push_back(convertPoint<int16_t>(extrude * offset) + p);
+            newRing.emplace_back(convertPoint<int16_t>(extrude * offset) + p);
         }
     }
 
@@ -247,6 +249,7 @@ optional<GeometryCollection> offsetLine(const GeometryCollection& rings, const d
 bool RenderLineLayer::queryIntersectsFeature(
         const GeometryCoordinates& queryGeometry,
         const GeometryTileFeature& feature,
+        const GeometryCollection& geometries,
         const float zoom,
         const TransformState& transformState,
         const float pixelsToTileUnits,
@@ -265,13 +268,14 @@ bool RenderLineLayer::queryIntersectsFeature(
                           .evaluate(feature, zoom, style::LineOffset::defaultValue()) * pixelsToTileUnits;
 
     // Apply offset to geometry
-    auto offsetGeometry = offsetLine(feature.getGeometries(), offset);
+    auto offsetGeometry = offsetLine(geometries, offset);
 
     // Test intersection
     const float halfWidth = getLineWidth(feature, zoom) / 2.0 * pixelsToTileUnits;
+
     return util::polygonIntersectsBufferedMultiLine(
             translatedQueryGeometry.value_or(queryGeometry),
-            offsetGeometry.value_or(feature.getGeometries()),
+            offsetGeometry != nullptr ? *offsetGeometry : geometries,
             halfWidth);
 }