Merge pull request #76 from mapbox/tile-cancels

fixes #67: cancel unnecessary tile fetches

include/llmr/map/tile.hpp

@@ -5,6 +5,7 @@
 #include <llmr/util/noncopyable.hpp>
 #include <llmr/geometry/debug_font_buffer.hpp>
 #include <llmr/geometry/vao.hpp>
+#include <llmr/platform/platform.hpp>
 
 #include <cstdint>
 #include <forward_list>
@@ -43,7 +44,8 @@ class Tile : public std::enable_shared_from_this<Tile>,
     enum state {
         initial,
         loading,
-        ready,
+        loaded,
+        parsed,
         obsolete
     };
 
@@ -92,8 +94,8 @@ class Tile : public std::enable_shared_from_this<Tile>,
 private:
     // Source data
     std::string data;
-
     const Style& style;
+    platform::Request req;
 };
 
 }

include/llmr/platform/platform.hpp

@@ -10,20 +10,26 @@
 
 namespace llmr {
 
-class Tile;
-
 namespace platform {
 
 // Restarts painting. This could for example trigger the event loop of the controlling application.
 void restart();
 
+struct Request {
+    int16_t identifier = -1;
+    std::string original_url;
+};
+
 struct Response {
     int16_t code = -1;
     std::string body;
 };
 
-// Makes an HTTP request of a URL on a background thread, calls a function with the results on the same thread, and finally calls a callback function on the main thread.
-void request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> foreground_callback);
+// Makes an HTTP request of a URL on a background thread, calls a function with the results on the same thread, and finally calls a callback function on the main thread. Returns a cancellable request.
+Request request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> foreground_callback);
+
+// Cancels an HTTP request.
+void cancel_request_http(Request request);
 
 // Returns a relative timestamp in seconds. This value must be monotonic.
 double time();

ios/MBXViewController.mm

@@ -16,14 +16,14 @@
 
 #include <llmr/llmr.hpp>
 #include <llmr/platform/platform.hpp>
+#include <llmr/map/tile.hpp>
 
 NSString *const MBXNeedsRenderNotification = @"MBXNeedsRenderNotification";
 NSString *const MBXUpdateActivityNotification = @"MBXUpdateActivityNotification";
 
 @interface MBXViewController () <UIGestureRecognizerDelegate>
 
 @property (nonatomic) EAGLContext *context;
-@property (nonatomic) NSUInteger activityCount;
 @property (nonatomic) CGPoint center;
 @property (nonatomic) CGFloat scale;
 @property (nonatomic) CGFloat angle;
@@ -153,11 +153,10 @@ - (void)updateRender:(NSNotification *)notification
 
 - (void)updateNetworkActivity:(NSNotification *)notification
 {
-    NSInteger input = [[notification userInfo][@"count"] integerValue];
-
-    self.activityCount += input;
-
-    [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:(self.activityCount > 0)];
+    [[NSURLSession sharedSession] getTasksWithCompletionHandler:^(NSArray *dataTasks, NSArray *uploadTasks, NSArray *downloadTasks)
+    {
+        [[UIApplication sharedApplication] setNetworkActivityIndicatorVisible:([downloadTasks count] > 0)];
+    }];
 }
 
 - (void)updateRender
@@ -391,7 +390,6 @@ - (BOOL)gestureRecognizer:(UIGestureRecognizer *)gestureRecognizer shouldRecogni
 
 MBXMapView *mapView;
 CADisplayLink *displayLink;
-NSOperationQueue *queue;
 
 namespace llmr
 {
@@ -402,22 +400,15 @@ void restart()
             [[NSNotificationCenter defaultCenter] postNotificationName:MBXNeedsRenderNotification object:nil];
         }
 
-        void request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> foreground_callback)
+        Request request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> foreground_callback)
         {
-            [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil userInfo:[NSDictionary dictionaryWithObject:@1 forKey:@"count"]];
-
-            if (!queue)
-                queue = [NSOperationQueue new];
-
-            NSMutableURLRequest *urlRequest = [NSMutableURLRequest requestWithURL:[NSURL URLWithString:@(url.c_str())]];
-
-            [NSURLConnection sendAsynchronousRequest:urlRequest
-                                               queue:queue
-                                   completionHandler:^(NSURLResponse *response, NSData *data, NSError *error)
+            NSURLSessionDataTask *task = [[NSURLSession sharedSession] dataTaskWithURL:[NSURL URLWithString:@(url.c_str())] completionHandler:^(NSData *data, NSURLResponse *response, NSError *error)
             {
+                [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil];
+
                 Response res;
 
-                if (error == nil)
+                if ( ! error && [response isKindOfClass:[NSHTTPURLResponse class]])
                 {
                     res.code = [(NSHTTPURLResponse *)response statusCode];
                     res.body = { (const char *)[data bytes], [data length] };
@@ -427,11 +418,29 @@ void request_http(std::string url, std::function<void(Response&)> background_fun
 
                 dispatch_async(dispatch_get_main_queue(), ^(void)
                 {
-                   foreground_callback();
+                    foreground_callback();
                 });
+            }];
 
-                [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil userInfo:[NSDictionary dictionaryWithObject:@(-1) forKey:@"count"]];
-                [[NSNotificationCenter defaultCenter] postNotificationName:MBXNeedsRenderNotification object:nil];
+            [task resume];
+
+            [[NSNotificationCenter defaultCenter] postNotificationName:MBXUpdateActivityNotification object:nil];
+
+            Request req;
+
+            req.identifier = task.taskIdentifier;
+            req.original_url = url;
+
+            return req;
+        }
+
+        void cancel_request_http(Request request)
+        {
+            [[NSURLSession sharedSession] getTasksWithCompletionHandler:^(NSArray *dataTasks, NSArray *uploadTasks, NSArray *downloadTasks)
+            {
+                for (NSURLSessionDownloadTask *task in downloadTasks)
+                    if (task.taskIdentifier == request.identifier)
+                        return [task cancel];
             }];
         }
 

ios/llmr-app.gyp

@@ -37,7 +37,7 @@
           'INFOPLIST_FILE': 'Info.plist',
           'CLANG_CXX_LIBRARY': 'libc++',
           'CLANG_CXX_LANGUAGE_STANDARD':'c++11',
-          'IPHONEOS_DEPLOYMENT_TARGET':'6.0',
+          'IPHONEOS_DEPLOYMENT_TARGET':'7.0',
           'TARGETED_DEVICE_FAMILY': '1,2',
           'GCC_VERSION': 'com.apple.compilers.llvm.clang.1_0',
           'CLANG_ENABLE_OBJC_ARC': 'YES'

llmr.gyp

@@ -51,7 +51,7 @@
       'xcode_settings': {
         'SDKROOT': 'macosx',
         'SUPPORTED_PLATFORMS':['macosx'],
-        'MACOSX_DEPLOYMENT_TARGET':'10.8',
+        'MACOSX_DEPLOYMENT_TARGET':'10.9',
         'PUBLIC_HEADERS_FOLDER_PATH': 'include',
         'OTHER_CPLUSPLUSFLAGS':[
             '<@(png_cflags)'
@@ -140,7 +140,7 @@
         'ARCHS': [ "armv7", "armv7s", "arm64", "i386" ],
         'TARGETED_DEVICE_FAMILY': '1,2',
         'CODE_SIGN_IDENTITY': 'iPhone Developer',
-        'IPHONEOS_DEPLOYMENT_TARGET': '5.0',
+        'IPHONEOS_DEPLOYMENT_TARGET': '7.0',
         'PUBLIC_HEADERS_FOLDER_PATH': 'include',
         'GCC_INPUT_FILETYPE':'sourcecode.cpp.cpp',
         'OTHER_CPLUSPLUSFLAGS':[

macosx/llmr-app.gyp

@@ -41,7 +41,7 @@
           'INFOPLIST_FILE': 'Info.plist',
           'CLANG_CXX_LIBRARY': 'libc++',
           'CLANG_CXX_LANGUAGE_STANDARD':'c++11',
-          'MACOSX_DEPLOYMENT_TARGET':'10.8',
+          'MACOSX_DEPLOYMENT_TARGET':'10.9',
           'GCC_VERSION': 'com.apple.compilers.llvm.clang.1_0',
           'CLANG_ENABLE_OBJC_ARC': 'YES'
         },

macosx/main.mm

@@ -3,6 +3,7 @@
 #import <Foundation/Foundation.h>
 #import <AppKit/AppKit.h>
 #include <llmr/platform/platform.hpp>
+#include <llmr/map/tile.hpp>
 #include "settings.hpp"
 
 #include <cstdio>
@@ -239,33 +240,45 @@ void restart() {
     [[NSApplication sharedApplication] postEvent: [NSEvent eventWithCGEvent:event] atStart:NO];
 }
 
-void request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> foreground_callback) {
-    if (!queue) {
-        queue = [NSOperationQueue new];
-    }
-
-    NSMutableURLRequest *urlRequest = [NSMutableURLRequest
-                                       requestWithURL:[NSURL
-                                               URLWithString:@(url.c_str())]];
-
-    [NSURLConnection
-     sendAsynchronousRequest:urlRequest
-     queue:[NSOperationQueue mainQueue]
-     completionHandler: ^(NSURLResponse * response, NSData * data, NSError * error) {
+Request request_http(std::string url, std::function<void(Response&)> background_function, std::function<void()> foreground_callback)
+{
+    NSURLSessionDataTask *task = [[NSURLSession sharedSession] dataTaskWithURL:[NSURL URLWithString:@(url.c_str())] completionHandler:^(NSData *data, NSURLResponse *response, NSError *error)
+    {
         Response res;
-        if (error == nil) {
-            NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)response;
-            res.code = [httpResponse statusCode];
+
+        if ( ! error && [response isKindOfClass:[NSHTTPURLResponse class]])
+        {
+            res.code = [(NSHTTPURLResponse *)response statusCode];
             res.body = { (const char *)[data bytes], [data length] };
         }
 
         background_function(res);
-        dispatch_async(dispatch_get_main_queue(), ^(void) {
+
+        dispatch_async(dispatch_get_main_queue(), ^(void)
+        {
             foreground_callback();
         });
     }];
+
+    [task resume];
+
+    Request req;
+
+    req.identifier = task.taskIdentifier;
+    req.original_url = url;
+
+    return req;
 }
 
+void cancel_request_http(Request request)
+{
+    [[NSURLSession sharedSession] getTasksWithCompletionHandler:^(NSArray *dataTasks, NSArray *uploadTasks, NSArray *downloadTasks)
+    {
+        for (NSURLSessionDownloadTask *task in downloadTasks)
+            if (task.taskIdentifier == request.identifier)
+                return [task cancel];
+    }];
+}
 
 double time() {
     return glfwGetTime();

src/map/map.cpp

@@ -278,7 +278,7 @@ bool Map::findLoadedChildren(const Tile::ID& id, int32_t maxCoveringZoom, std::f
     auto ids = Tile::children(id, z + 1);
     for (const Tile::ID& child_id : ids) {
         const Tile::Ptr& tile = hasTile(child_id);
-        if (tile && tile->state == Tile::ready) {
+        if (tile && tile->state == Tile::parsed) {
             assert(tile);
             retain.emplace_front(tile->id);
         } else {
@@ -305,7 +305,7 @@ bool Map::findLoadedParent(const Tile::ID& id, int32_t minCoveringZoom, std::for
     for (int32_t z = id.z - 1; z >= minCoveringZoom; --z) {
         const Tile::ID parent_id = Tile::parent(id, z);
         const Tile::Ptr tile = hasTile(parent_id);
-        if (tile && tile->state == Tile::ready) {
+        if (tile && tile->state == Tile::parsed) {
             assert(tile);
             retain.emplace_front(tile->id);
             return true;
@@ -361,7 +361,7 @@ bool Map::updateTiles() {
         Tile::Ptr tile = addTile(id);
         assert(tile);
 
-        if (tile->state != Tile::ready) {
+        if (tile->state != Tile::parsed) {
             // The tile we require is not yet loaded. Try to find a parent or
             // child tile that we already have.
 
@@ -416,7 +416,7 @@ bool Map::render() {
 
     for (Tile::Ptr& tile : tiles) {
         assert(tile);
-        if (tile->state == Tile::ready) {
+        if (tile->state == Tile::parsed) {
             painter.changeMatrix(tile->id);
             painter.render(tile);
         }

src/map/tile.cpp

@@ -9,7 +9,6 @@
 #include <llmr/renderer/fill_bucket.hpp>
 #include <llmr/renderer/line_bucket.hpp>
 #include <llmr/renderer/point_bucket.hpp>
-#include <llmr/platform/platform.hpp>
 #include <llmr/util/pbf.hpp>
 #include <llmr/util/string.hpp>
 
@@ -75,22 +74,25 @@ void Tile::request() {
 
     // Note: Somehow this feels slower than the change to request_http()
     std::shared_ptr<Tile> tile = shared_from_this();
-    platform::request_http(url, [=](platform::Response& res) {
-        if (res.code == 200) {
+    platform::Request request = platform::request_http(url, [=](platform::Response& res) {
+        if (res.code == 200 && tile->state != obsolete) {
+            tile->state = Tile::loaded;
             tile->data.swap(res.body);
             tile->parse();
-        } else {
+        } else if (tile->state != obsolete) {
             fprintf(stderr, "tile loading failed\n");
         }
     }, []() {
         platform::restart();
     });
+    req = request;
 }
 
 void Tile::cancel() {
     // TODO: thread safety
     if (state != obsolete) {
         state = obsolete;
+        platform::cancel_request_http(req);
     } else {
         assert((!"logic error? multiple cancelleations"));
     }
@@ -99,7 +101,7 @@ void Tile::cancel() {
 bool Tile::parse() {
     // std::lock_guard<std::mutex> lock(mtx);
 
-    if (state == obsolete) {
+    if (state != loaded) {
         return false;
     }
 
@@ -115,7 +117,7 @@ bool Tile::parse() {
     if (state == obsolete) {
         return false;
     } else {
-        state = ready;
+        state = parsed;
     }
 
     return true;

src/renderer/painter.cpp

@@ -132,7 +132,7 @@ void Painter::clear() {
 }
 
 void Painter::render(const Tile::Ptr& tile) {
-    if (tile->state != Tile::ready) {
+    if (tile->state != Tile::parsed) {
         return;
     }