From a1ed9f879408a12e462196d961075c2ae3d7486e Mon Sep 17 00:00:00 2001
From: Bruno de Oliveira Abinader <bruno@mapbox.com>
Date: Wed, 5 Jul 2017 14:37:49 +0300
Subject: [core] Don't upload empty buckets

---
 src/mbgl/renderer/bucket.hpp                | 5 ++++-
 src/mbgl/renderer/buckets/symbol_bucket.cpp | 3 +--
 test/gl/bucket.test.cpp                     | 6 ++++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/mbgl/renderer/bucket.hpp b/src/mbgl/renderer/bucket.hpp
index 6c391cf014..a411daf3d4 100644
--- a/src/mbgl/renderer/bucket.hpp
+++ b/src/mbgl/renderer/bucket.hpp
@@ -28,6 +28,9 @@ public:
     Bucket() = default;
     virtual ~Bucket() = default;
 
+    // Feature geometries are also used to populate the feature index.
+    // Obtaining these is a costly operation, so we do it only once, and
+    // pass-by-const-ref the geometries as a second parameter.
     virtual void addFeature(const GeometryTileFeature&,
                             const GeometryCollection&) {};
 
@@ -46,7 +49,7 @@ public:
     };
 
     bool needsUpload() const {
-        return !uploaded;
+        return hasData() && !uploaded;
     }
 
 protected:
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index cbddade899..28e6a47250 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -69,8 +69,7 @@ void SymbolBucket::render(Painter& painter,
 }
 
 bool SymbolBucket::hasData() const {
-    assert(false); // Should be calling SymbolLayout::has{Text,Icon,CollisonBox}Data() instead.
-    return false;
+    return hasTextData() || hasIconData() || hasCollisionBoxData();
 }
 
 bool SymbolBucket::hasTextData() const {
diff --git a/test/gl/bucket.test.cpp b/test/gl/bucket.test.cpp
index ee9ea54414..feca82998f 100644
--- a/test/gl/bucket.test.cpp
+++ b/test/gl/bucket.test.cpp
@@ -16,16 +16,19 @@ using namespace mbgl;
 TEST(Buckets, CircleBucket) {
     CircleBucket bucket { { {0, 0, 0}, MapMode::Still, 1.0 }, {} };
     ASSERT_FALSE(bucket.hasData());
+    ASSERT_FALSE(bucket.needsUpload());
 }
 
 TEST(Buckets, FillBucket) {
     FillBucket bucket { { {0, 0, 0}, MapMode::Still, 1.0 }, {} };
     ASSERT_FALSE(bucket.hasData());
+    ASSERT_FALSE(bucket.needsUpload());
 }
 
 TEST(Buckets, LineBucket) {
     LineBucket bucket { { {0, 0, 0}, MapMode::Still, 1.0 }, {}, {} };
     ASSERT_FALSE(bucket.hasData());
+    ASSERT_FALSE(bucket.needsUpload());
 }
 
 TEST(Buckets, SymbolBucket) {
@@ -37,6 +40,8 @@ TEST(Buckets, SymbolBucket) {
     ASSERT_FALSE(bucket.hasIconData());
     ASSERT_FALSE(bucket.hasTextData());
     ASSERT_FALSE(bucket.hasCollisionBoxData());
+    ASSERT_FALSE(bucket.hasData());
+    ASSERT_FALSE(bucket.needsUpload());
 }
 
 TEST(Buckets, RasterBucket) {
@@ -44,6 +49,7 @@ TEST(Buckets, RasterBucket) {
     UnassociatedImage rgba({ 1, 1 });
 
     RasterBucket bucket = { std::move(rgba) };
+    ASSERT_TRUE(bucket.hasData());
     ASSERT_TRUE(bucket.needsUpload());
 
     bucket.upload(context);
-- 
cgit v1.2.1