[core] Cleanup in response to review comments.

16 files changed, 229 insertions, 211 deletions

diff --git a/include/mbgl/tile/tile_id.hpp b/include/mbgl/tile/tile_id.hpp
index 0457dd3a07..11fb5ce537 100644
--- a/include/mbgl/tile/tile_id.hpp
+++ b/include/mbgl/tile/tile_id.hpp
@@ -30,9 +30,9 @@ public:
     CanonicalTileID scaledTo(uint8_t z) const;
     std::array<CanonicalTileID, 4> children() const;
 
-    const uint8_t z;
-    const uint32_t x;
-    const uint32_t y;
+    uint8_t z;
+    uint32_t x;
+    uint32_t y;
 };
 
 ::std::ostream& operator<<(::std::ostream& os, const CanonicalTileID& rhs);
diff --git a/src/mbgl/geometry/feature_index.hpp b/src/mbgl/geometry/feature_index.hpp
index 07dc1b1558..e95bb94da6 100644
--- a/src/mbgl/geometry/feature_index.hpp
+++ b/src/mbgl/geometry/feature_index.hpp
@@ -25,21 +25,17 @@ public:
         , sourceLayerName(std::move(sourceLayerName_))
         , bucketName(std::move(bucketName_))
         , sortIndex(sortIndex_)
-        , z(0)
-        , x(0)
-        , y(0)
+        , tileID(0, 0, 0)
     {}
     
     IndexedSubfeature(std::size_t index_, std::string sourceLayerName_, std::string bucketName_, size_t sortIndex_,
-                      std::string sourceID_, uint8_t z_, uint32_t x_, uint32_t y_)
+                      std::string sourceID_, CanonicalTileID tileID_)
         : index(index_)
         , sourceLayerName(std::move(sourceLayerName_))
         , bucketName(std::move(bucketName_))
         , sortIndex(std::move(sortIndex_))
         , sourceID(std::move(sourceID_))
-        , z(z_)
-        , x(x_)
-        , y(y_)
+        , tileID(std::move(tileID_))
     {}
     
     size_t index;
@@ -49,9 +45,7 @@ public:
 
     // Only used for symbol features
     std::string sourceID;
-    uint8_t z;
-    uint32_t x;
-    uint32_t y;
+    CanonicalTileID tileID;
 };
 
 class FeatureIndex {
diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp
index 0ef6bc1a3a..5d57093b36 100644
--- a/src/mbgl/gl/context.hpp
+++ b/src/mbgl/gl/context.hpp
@@ -61,7 +61,7 @@ public:
     optional<std::pair<BinaryProgramFormat, std::string>> getBinaryProgram(ProgramID) const;
 
     template <class Vertex, class DrawMode>
-    VertexBuffer<Vertex, DrawMode> createVertexBuffer(VertexVector<Vertex, DrawMode>&& v, const BufferUsage usage=BufferUsage::StaticDraw) {
+    VertexBuffer<Vertex, DrawMode> createVertexBuffer(VertexVector<Vertex, DrawMode>&& v, const BufferUsage usage = BufferUsage::StaticDraw) {
         return VertexBuffer<Vertex, DrawMode> {
             v.vertexSize(),
             createVertexBuffer(v.data(), v.byteSize(), usage)
@@ -75,7 +75,7 @@ public:
     }
 
     template <class DrawMode>
-    IndexBuffer<DrawMode> createIndexBuffer(IndexVector<DrawMode>&& v, const BufferUsage usage=BufferUsage::StaticDraw) {
+    IndexBuffer<DrawMode> createIndexBuffer(IndexVector<DrawMode>&& v, const BufferUsage usage = BufferUsage::StaticDraw) {
         return IndexBuffer<DrawMode> {
             v.indexSize(),
             createIndexBuffer(v.data(), v.byteSize(), usage)
diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp
index d0398fcd30..6e152349ca 100644
--- a/src/mbgl/layout/symbol_instance.cpp
+++ b/src/mbgl/layout/symbol_instance.cpp
@@ -32,7 +32,7 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
 
     // Create the collision features that will be used to check whether this symbol instance can be placed
     textCollisionFeature(line_, anchor, shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature, overscaling),
-    iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, SymbolPlacementType::Point, indexedFeature),
+    iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, indexedFeature),
     featureIndex(featureIndex_),
     textOffset(textOffset_),
     iconOffset(iconOffset_),
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index d84b74d42a..fc1ede56ff 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -293,7 +293,7 @@ void SymbolLayout::addFeature(const std::size_t index,
 
     const float textRepeatDistance = symbolSpacing / 2;
     IndexedSubfeature indexedFeature(feature.index, sourceLayer->getName(), bucketName, symbolInstances.size(),
-                                     sourceID, tileID.canonical.z, tileID.canonical.x, tileID.canonical.y);
+                                     sourceID, tileID.canonical);
 
     auto addSymbolInstance = [&] (const GeometryCoordinates& line, Anchor& anchor) {
         // https://github.com/mapbox/vector-tile-spec/tree/master/2.1#41-layers
@@ -320,7 +320,7 @@ void SymbolLayout::addFeature(const std::size_t index,
                     symbolInstances.size(),
                     textBoxScale, textPadding, textPlacement, textOffset,
                     iconBoxScale, iconPadding, iconOffset,
-                    glyphPositionMap, indexedFeature, index, feature.text ? *feature.text : std::u16string{}, overscaling);
+                    glyphPositionMap, indexedFeature, index, feature.text.value_or(std::u16string()), overscaling);
         }
     };
     
diff --git a/src/mbgl/layout/symbol_projection.cpp b/src/mbgl/layout/symbol_projection.cpp
index b8c399d857..ee6385c93c 100644
--- a/src/mbgl/layout/symbol_projection.cpp
+++ b/src/mbgl/layout/symbol_projection.cpp
@@ -294,7 +294,7 @@ namespace mbgl {
         std::vector<PlacedGlyph> placedGlyphs;
         if (symbol.glyphOffsets.size() > 1) {
 
-            const optional<std::pair<PlacedGlyph,PlacedGlyph>> firstAndLastGlyph =
+            const optional<std::pair<PlacedGlyph, PlacedGlyph>> firstAndLastGlyph =
                 placeFirstAndLastGlyph(fontScale, lineOffsetX, lineOffsetY, flip, projectedAnchorPoint, symbol.anchorPoint, symbol, labelPlaneMatrix, false);
             if (!firstAndLastGlyph) {
                 return PlacementResult::NotEnoughRoom;
diff --git a/src/mbgl/programs/collision_box_program.hpp b/src/mbgl/programs/collision_box_program.hpp
index 46f65a905a..8d712a3df3 100644
--- a/src/mbgl/programs/collision_box_program.hpp
+++ b/src/mbgl/programs/collision_box_program.hpp
@@ -56,51 +56,50 @@ public:
     }
 
 	template <class DrawMode>
-		void draw(gl::Context& context,
-				DrawMode drawMode,
-				gl::DepthMode depthMode,
-				gl::StencilMode stencilMode,
-				gl::ColorMode colorMode,
-				const UniformValues& uniformValues,
-				const gl::VertexBuffer<CollisionBoxLayoutAttributes::Vertex>& layoutVertexBuffer,
-                const gl::VertexBuffer<CollisionBoxDynamicAttributes::Vertex>& dynamicVertexBuffer,
-				const gl::IndexBuffer<DrawMode>& indexBuffer,
-				const SegmentVector<Attributes>& segments,
-				const PaintPropertyBinders& paintPropertyBinders,
-				const typename PaintProperties::PossiblyEvaluated& currentProperties,
-				float currentZoom,
-				const std::string& layerID) {
-			typename AllUniforms::Values allUniformValues = uniformValues
-				.concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
-
-			typename Attributes::Bindings allAttributeBindings = CollisionBoxLayoutAttributes::bindings(layoutVertexBuffer)
-                .concat(CollisionBoxDynamicAttributes::bindings(dynamicVertexBuffer))
-				.concat(paintPropertyBinders.attributeBindings(currentProperties));
-
-            assert(layoutVertexBuffer.vertexCount == dynamicVertexBuffer.vertexCount);
-
-			for (auto& segment : segments) {
-				auto vertexArrayIt = segment.vertexArrays.find(layerID);
-
-				if (vertexArrayIt == segment.vertexArrays.end()) {
-					vertexArrayIt = segment.vertexArrays.emplace(layerID, context.createVertexArray()).first;
-				}
-
-				program.draw(
-						context,
-						std::move(drawMode),
-						std::move(depthMode),
-						std::move(stencilMode),
-						std::move(colorMode),
-						allUniformValues,
-						vertexArrayIt->second,
-						Attributes::offsetBindings(allAttributeBindings, segment.vertexOffset),
-						indexBuffer,
-						segment.indexOffset,
-						segment.indexLength);
-			}
-		}
+    void draw(gl::Context& context,
+              DrawMode drawMode,
+              gl::DepthMode depthMode,
+              gl::StencilMode stencilMode,
+              gl::ColorMode colorMode,
+              const UniformValues& uniformValues,
+              const gl::VertexBuffer<CollisionBoxLayoutAttributes::Vertex>& layoutVertexBuffer,
+              const gl::VertexBuffer<CollisionBoxDynamicAttributes::Vertex>& dynamicVertexBuffer,
+              const gl::IndexBuffer<DrawMode>& indexBuffer,
+              const SegmentVector<Attributes>& segments,
+              const PaintPropertyBinders& paintPropertyBinders,
+              const typename PaintProperties::PossiblyEvaluated& currentProperties,
+              float currentZoom,
+              const std::string& layerID) {
+        typename AllUniforms::Values allUniformValues = uniformValues
+            .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
+
+        typename Attributes::Bindings allAttributeBindings = CollisionBoxLayoutAttributes::bindings(layoutVertexBuffer)
+            .concat(CollisionBoxDynamicAttributes::bindings(dynamicVertexBuffer))
+            .concat(paintPropertyBinders.attributeBindings(currentProperties));
+
+        assert(layoutVertexBuffer.vertexCount == dynamicVertexBuffer.vertexCount);
+
+        for (auto& segment : segments) {
+            auto vertexArrayIt = segment.vertexArrays.find(layerID);
+
+            if (vertexArrayIt == segment.vertexArrays.end()) {
+                vertexArrayIt = segment.vertexArrays.emplace(layerID, context.createVertexArray()).first;
+            }
 
+            program.draw(
+                    context,
+                    std::move(drawMode),
+                    std::move(depthMode),
+                    std::move(stencilMode),
+                    std::move(colorMode),
+                    allUniformValues,
+                    vertexArrayIt->second,
+                    Attributes::offsetBindings(allAttributeBindings, segment.vertexOffset),
+                    indexBuffer,
+                    segment.indexOffset,
+                    segment.indexLength);
+        }
+    }
 };
 
 
@@ -135,49 +134,48 @@ public:
     }
 
     template <class DrawMode>
-        void draw(gl::Context& context,
-                DrawMode drawMode,
-                gl::DepthMode depthMode,
-                gl::StencilMode stencilMode,
-                gl::ColorMode colorMode,
-                const UniformValues& uniformValues,
-                const gl::VertexBuffer<CollisionBoxLayoutAttributes::Vertex>& layoutVertexBuffer,
-                const gl::VertexBuffer<CollisionBoxDynamicAttributes::Vertex>& dynamicVertexBuffer,
-                const gl::IndexBuffer<DrawMode>& indexBuffer,
-                const SegmentVector<Attributes>& segments,
-                const PaintPropertyBinders& paintPropertyBinders,
-                const typename PaintProperties::PossiblyEvaluated& currentProperties,
-                float currentZoom,
-                const std::string& layerID) {
-            typename AllUniforms::Values allUniformValues = uniformValues
-                .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
-
-            typename Attributes::Bindings allAttributeBindings = CollisionBoxLayoutAttributes::bindings(layoutVertexBuffer)
-                .concat(CollisionBoxDynamicAttributes::bindings(dynamicVertexBuffer))
-                .concat(paintPropertyBinders.attributeBindings(currentProperties));
-
-            for (auto& segment : segments) {
-                auto vertexArrayIt = segment.vertexArrays.find(layerID);
-
-                if (vertexArrayIt == segment.vertexArrays.end()) {
-                    vertexArrayIt = segment.vertexArrays.emplace(layerID, context.createVertexArray()).first;
-                }
-
-                program.draw(
-                        context,
-                        std::move(drawMode),
-                        std::move(depthMode),
-                        std::move(stencilMode),
-                        std::move(colorMode),
-                        allUniformValues,
-                        vertexArrayIt->second,
-                        Attributes::offsetBindings(allAttributeBindings, segment.vertexOffset),
-                        indexBuffer,
-                        segment.indexOffset,
-                        segment.indexLength);
+    void draw(gl::Context& context,
+              DrawMode drawMode,
+              gl::DepthMode depthMode,
+              gl::StencilMode stencilMode,
+              gl::ColorMode colorMode,
+              const UniformValues& uniformValues,
+              const gl::VertexBuffer<CollisionBoxLayoutAttributes::Vertex>& layoutVertexBuffer,
+              const gl::VertexBuffer<CollisionBoxDynamicAttributes::Vertex>& dynamicVertexBuffer,
+              const gl::IndexBuffer<DrawMode>& indexBuffer,
+              const SegmentVector<Attributes>& segments,
+              const PaintPropertyBinders& paintPropertyBinders,
+              const typename PaintProperties::PossiblyEvaluated& currentProperties,
+              float currentZoom,
+              const std::string& layerID) {
+        typename AllUniforms::Values allUniformValues = uniformValues
+            .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
+
+        typename Attributes::Bindings allAttributeBindings = CollisionBoxLayoutAttributes::bindings(layoutVertexBuffer)
+            .concat(CollisionBoxDynamicAttributes::bindings(dynamicVertexBuffer))
+            .concat(paintPropertyBinders.attributeBindings(currentProperties));
+
+        for (auto& segment : segments) {
+            auto vertexArrayIt = segment.vertexArrays.find(layerID);
+
+            if (vertexArrayIt == segment.vertexArrays.end()) {
+                vertexArrayIt = segment.vertexArrays.emplace(layerID, context.createVertexArray()).first;
             }
-        }
 
+            program.draw(
+                    context,
+                    std::move(drawMode),
+                    std::move(depthMode),
+                    std::move(stencilMode),
+                    std::move(colorMode),
+                    allUniformValues,
+                    vertexArrayIt->second,
+                    Attributes::offsetBindings(allAttributeBindings, segment.vertexOffset),
+                    indexBuffer,
+                    segment.indexOffset,
+                    segment.indexLength);
+        }
+    }
 };
 
 using CollisionBoxVertex = CollisionBoxProgram::LayoutVertex;
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.cpp b/src/mbgl/renderer/buckets/symbol_bucket.cpp
index ebcf93f2b1..60e8a0b504 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.cpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.cpp
@@ -179,6 +179,7 @@ void SymbolBucket::sortFeatures(const float angle) {
 
     // To avoid sorting the actual symbolInstance array we sort an array of indexes.
     std::vector<size_t> symbolInstanceIndexes;
+    symbolInstanceIndexes.reserve(symbolInstances.size());
     for (size_t i = 0; i < symbolInstances.size(); i++) {
         symbolInstanceIndexes.push_back(i);
     }
diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp
index aa5777a186..55831cb72c 100644
--- a/src/mbgl/renderer/render_layer.hpp
+++ b/src/mbgl/renderer/render_layer.hpp
@@ -81,10 +81,14 @@ public:
 
     friend std::string layoutKey(const RenderLayer&);
 
+protected:
+    // renderTiles are exposed directly to CrossTileSymbolIndex and Placement so they
+    // can update opacities in the symbol buckets immediately before rendering
+    friend class CrossTileSymbolIndex;
+    friend class Placement;
     // Stores current set of tiles to be rendered for this layer.
     std::vector<std::reference_wrapper<RenderTile>> renderTiles;
 
-protected:
     // Stores what render passes this layer is currently enabled for. This depends on the
     // evaluated StyleProperties object and is updated accordingly.
     RenderPass passes = RenderPass::None;
diff --git a/src/mbgl/renderer/renderer_impl.cpp b/src/mbgl/renderer/renderer_impl.cpp
index c73cef8bb8..032b4558ce 100644
--- a/src/mbgl/renderer/renderer_impl.cpp
+++ b/src/mbgl/renderer/renderer_impl.cpp
@@ -23,7 +23,6 @@
 #include <mbgl/style/source_impl.hpp>
 #include <mbgl/style/transition_options.hpp>
 #include <mbgl/text/glyph_manager.hpp>
-#include <mbgl/text/cross_tile_symbol_index.hpp>
 #include <mbgl/tile/tile.hpp>
 #include <mbgl/util/math.hpp>
 #include <mbgl/util/string.hpp>
@@ -58,7 +57,6 @@ Renderer::Impl::Impl(RendererBackend& backend_,
     , sourceImpls(makeMutable<std::vector<Immutable<style::Source::Impl>>>())
     , layerImpls(makeMutable<std::vector<Immutable<style::Layer::Impl>>>())
     , renderLight(makeMutable<Light::Impl>())
-    , crossTileSymbolIndex(std::make_unique<CrossTileSymbolIndex>())
     , placement(std::make_unique<Placement>(TransformState{}, MapMode::Static)) {
     glyphManager->setObserver(this);
 }
@@ -368,11 +366,11 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) {
     bool symbolBucketsChanged = false;
     if (parameters.mapMode != MapMode::Continuous) {
         // TODO: Think about right way for symbol index to handle still rendering
-        crossTileSymbolIndex->reset();
+        crossTileSymbolIndex.reset();
     }
     for (auto it = order.rbegin(); it != order.rend(); ++it) {
         if (it->layer.is<RenderSymbolLayer>()) {
-            if (crossTileSymbolIndex->addLayer(*it->layer.as<RenderSymbolLayer>())) symbolBucketsChanged = true;
+            if (crossTileSymbolIndex.addLayer(*it->layer.as<RenderSymbolLayer>())) symbolBucketsChanged = true;
         }
     }
 
@@ -622,7 +620,7 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) {
 
     observer->onDidFinishRenderingFrame(
         loaded ? RendererObserver::RenderMode::Full : RendererObserver::RenderMode::Partial,
-        updateParameters.mode == MapMode::Continuous && (hasTransitions(parameters.timePoint))
+        updateParameters.mode == MapMode::Continuous && hasTransitions(parameters.timePoint)
     );
 
     if (!loaded) {
diff --git a/src/mbgl/renderer/renderer_impl.hpp b/src/mbgl/renderer/renderer_impl.hpp
index c5033bb1f9..32e2dc99f2 100644
--- a/src/mbgl/renderer/renderer_impl.hpp
+++ b/src/mbgl/renderer/renderer_impl.hpp
@@ -9,6 +9,7 @@
 #include <mbgl/style/layer.hpp>
 #include <mbgl/map/transform_state.hpp>
 #include <mbgl/map/zoom_history.hpp>
+#include <mbgl/text/cross_tile_symbol_index.hpp>
 #include <mbgl/text/glyph_manager_observer.hpp>
 #include <mbgl/text/placement.hpp>
 
@@ -108,7 +109,7 @@ private:
     std::unordered_map<std::string, std::unique_ptr<RenderLayer>> renderLayers;
     RenderLight renderLight;
 
-    std::unique_ptr<CrossTileSymbolIndex> crossTileSymbolIndex;
+    CrossTileSymbolIndex crossTileSymbolIndex;
     std::unique_ptr<Placement> placement;
 
     bool contextLost = false;
diff --git a/src/mbgl/text/collision_feature.hpp b/src/mbgl/text/collision_feature.hpp
index 4ecd25c717..df1b12819c 100644
--- a/src/mbgl/text/collision_feature.hpp
+++ b/src/mbgl/text/collision_feature.hpp
@@ -56,19 +56,26 @@ public:
         : CollisionFeature(line, anchor, shapedText.top, shapedText.bottom, shapedText.left, shapedText.right, boxScale, padding, placement, indexedFeature_, overscaling) {}
 
     // for icons
+    // Icons collision features are always SymbolPlacementType::Point, which means the collision feature
+    // will be viewport-rotation-aligned even if the icon is map-rotation-aligned (e.g. `icon-rotation-alignment: map`
+    // _or_ `symbol-placement: line`). We're relying on most icons being "close enough" to square that having
+    // incorrect rotation alignment doesn't throw off collision detection too much.
+    // See: https://github.com/mapbox/mapbox-gl-js/issues/4861
     CollisionFeature(const GeometryCoordinates& line,
                      const Anchor& anchor,
                      optional<PositionedIcon> shapedIcon,
                      const float boxScale,
                      const float padding,
-                     const style::SymbolPlacementType placement,
                      const IndexedSubfeature& indexedFeature_)
         : CollisionFeature(line, anchor,
                            (shapedIcon ? shapedIcon->top() : 0),
                            (shapedIcon ? shapedIcon->bottom() : 0),
                            (shapedIcon ? shapedIcon->left() : 0),
                            (shapedIcon ? shapedIcon->right() : 0),
-                           boxScale, padding, placement, indexedFeature_, 1) {}
+                           boxScale,
+                           padding,
+                           style::SymbolPlacementType::Point,
+                           indexedFeature_, 1) {}
 
     CollisionFeature(const GeometryCoordinates& line,
                      const Anchor&,
diff --git a/src/mbgl/text/collision_index.cpp b/src/mbgl/text/collision_index.cpp
index 72a2d1af4e..fee28b5873 100644
--- a/src/mbgl/text/collision_index.cpp
+++ b/src/mbgl/text/collision_index.cpp
@@ -241,6 +241,25 @@ void CollisionIndex::insertFeature(CollisionFeature& feature, bool ignorePlaceme
     }
 }
 
+bool polygonIntersectsBox(const LineString<float>& polygon, const GridIndex<IndexedSubfeature>::BBox& bbox) {
+    // This is just a wrapper that allows us to use the integer-based util::polygonIntersectsPolygon
+    // Conversion limits our query accuracy to single-pixel resolution
+    GeometryCoordinates integerPolygon;
+    for (const auto& point : polygon) {
+        integerPolygon.push_back(convertPoint<int16_t>(point));
+    }
+    int16_t minX1 = bbox.min.x;
+    int16_t maxY1 = bbox.max.y;
+    int16_t minY1 = bbox.min.y;
+    int16_t maxX1 = bbox.max.x;
+
+    auto bboxPoints = GeometryCoordinates {
+        { minX1, minY1 }, { maxX1, minY1 }, { maxX1, maxY1 }, { minX1, maxY1 }
+    };
+    
+    return util::polygonIntersectsPolygon(integerPolygon, bboxPoints);
+}
+
 std::vector<IndexedSubfeature> CollisionIndex::queryRenderedSymbols(const GeometryCoordinates& queryGeometry, const UnwrappedTileID& tileID, const std::string& sourceID) const {
     std::vector<IndexedSubfeature> result;
     if (queryGeometry.empty() || (collisionGrid.empty() && ignoredGrid.empty())) {
@@ -253,14 +272,10 @@ std::vector<IndexedSubfeature> CollisionIndex::queryRenderedSymbols(const Geomet
     transformState.matrixFor(posMatrix, tileID);
     matrix::multiply(posMatrix, projMatrix, posMatrix);
 
-    // Two versions of the query here just because util::polygonIntersectsPolygon requires
-    // GeometryCoordinates (based on int16_t). Otherwise, work with floats.
-    GeometryCoordinates projectedPolygon;
+    // queryGeometry is specified in integer tile units, but in projecting we switch to float pixels
     LineString<float> projectedQuery;
-
     for (const auto& point : queryGeometry) {
         auto projected = projectPoint(posMatrix, convertPoint<float>(point));
-        projectedPolygon.push_back(convertPoint<int16_t>(projected));
         projectedQuery.push_back(projected);
     }
     
@@ -273,8 +288,7 @@ std::vector<IndexedSubfeature> CollisionIndex::queryRenderedSymbols(const Geomet
 
     for (auto& queryResult : features) {
         auto& feature = queryResult.first;
-        CanonicalTileID featureTileID(feature.z, feature.x, feature.y);
-        if (feature.sourceID == sourceID && featureTileID == tileID.canonical) {
+        if (feature.sourceID == sourceID && feature.tileID == tileID.canonical) {
             // We only have to filter on the canonical ID because even if the feature is showing multiple times
             // we treat it as one feature.
             thisTileFeatures.push_back(queryResult);
@@ -284,8 +298,7 @@ std::vector<IndexedSubfeature> CollisionIndex::queryRenderedSymbols(const Geomet
     std::vector<QueryResult> ignoredFeatures = ignoredGrid.queryWithBoxes(envelope);
     for (auto& queryResult : ignoredFeatures) {
         auto& feature = queryResult.first;
-        CanonicalTileID featureTileID(feature.z, feature.x, feature.y);
-        if (feature.sourceID == sourceID && featureTileID == tileID.canonical) {
+        if (feature.sourceID == sourceID && feature.tileID == tileID.canonical) {
             thisTileFeatures.push_back(queryResult);
         }
     }
@@ -301,18 +314,10 @@ std::vector<IndexedSubfeature> CollisionIndex::queryRenderedSymbols(const Geomet
             continue;
 
         seenFeatures.insert(feature.index);
-
-        int16_t minX1 = bbox.min.x;
-        int16_t maxY1 = bbox.max.y;
-        int16_t minY1 = bbox.min.y;
-        int16_t maxX1 = bbox.max.x;
-
-        auto bboxPoints = GeometryCoordinates {
-            { minX1, minY1 }, { maxX1, minY1 }, { maxX1, maxY1 }, { minX1, maxY1 }
-        };
         
-        if (!util::polygonIntersectsPolygon(projectedPolygon, bboxPoints))
+        if (!polygonIntersectsBox(projectedQuery, bbox)) {
             continue;
+        }
 
         result.push_back(feature);
     }
diff --git a/src/mbgl/text/placement.hpp b/src/mbgl/text/placement.hpp
index 4d5fad06a1..c6d1f259a5 100644
--- a/src/mbgl/text/placement.hpp
+++ b/src/mbgl/text/placement.hpp
@@ -9,82 +9,82 @@
 
 namespace mbgl {
 
-    class RenderSymbolLayer;
-    class SymbolBucket;
-
-    class OpacityState {
-        public:
-            OpacityState(bool placed, bool offscreen);
-            OpacityState(const OpacityState& prevOpacityState, float increment, bool placed);
-            bool isHidden() const;
-            float opacity;
-            bool placed;
-    };
-
-    class JointOpacityState {
-        public:
-            JointOpacityState(bool placedIcon, bool placedText, bool offscreen);
-            JointOpacityState(const JointOpacityState& prevOpacityState, float increment, bool placedIcon, bool placedText);
-            bool isHidden() const;
-            OpacityState icon;
-            OpacityState text;
-    };
-
-    class JointPlacement {
-        public:
-            JointPlacement(bool text_, bool icon_, bool offscreen_)
-                : text(text_), icon(icon_), offscreen(offscreen_)
-            {}
-        
-            const bool text;
-            const bool icon;
-            // offscreen = outside viewport, but within CollisionIndex::viewportPadding px of the edge
-            // Because these symbols aren't onscreen yet, we can skip the "fade in" animation,
-            // and if a subsequent viewport change brings them into view, they'll be fully
-            // visible right away.
-            const bool offscreen;
-    };
-
-    class Placement {
-        public:
-            Placement(const TransformState&, MapMode mapMode);
-            void placeLayer(RenderSymbolLayer&, const mat4&, bool showCollisionBoxes);
-            bool commit(const Placement& prevPlacement, TimePoint);
-            void updateLayerOpacities(RenderSymbolLayer&);
-            JointOpacityState getOpacity(uint32_t crossTileSymbolID) const;
-            float symbolFadeChange(TimePoint now) const;
-            bool hasTransitions(TimePoint now) const;
-
-            const CollisionIndex& getCollisionIndex() const;
-        
-            bool stillRecent(TimePoint now) const;
-            void setRecent(TimePoint now);
-            void setStale();
-        private:
-
-            void placeLayerBucket(
-                    SymbolBucket&,
-                    const mat4& posMatrix,
-                    const mat4& textLabelPlaneMatrix,
-                    const mat4& iconLabelPlaneMatrix,
-                    const float scale,
-                    const float pixelRatio,
-                    const bool showCollisionBoxes,
-                    std::unordered_set<uint32_t>& seenCrossTileIDs);
-
-            void updateBucketOpacities(SymbolBucket&, std::set<uint32_t>&);
-        
-            CollisionIndex collisionIndex;
-
-            TransformState state;
-            MapMode mapMode;
-            TimePoint commitTime;
-
-            std::unordered_map<uint32_t, JointPlacement> placements;
-            std::unordered_map<uint32_t, JointOpacityState> opacities;
-        
-            TimePoint recentUntil;
-            bool stale = false;
-    };
+class RenderSymbolLayer;
+class SymbolBucket;
+
+class OpacityState {
+public:
+    OpacityState(bool placed, bool offscreen);
+    OpacityState(const OpacityState& prevOpacityState, float increment, bool placed);
+    bool isHidden() const;
+    float opacity;
+    bool placed;
+};
+
+class JointOpacityState {
+public:
+    JointOpacityState(bool placedIcon, bool placedText, bool offscreen);
+    JointOpacityState(const JointOpacityState& prevOpacityState, float increment, bool placedIcon, bool placedText);
+    bool isHidden() const;
+    OpacityState icon;
+    OpacityState text;
+};
+
+class JointPlacement {
+public:
+    JointPlacement(bool text_, bool icon_, bool offscreen_)
+        : text(text_), icon(icon_), offscreen(offscreen_)
+    {}
+
+    const bool text;
+    const bool icon;
+    // offscreen = outside viewport, but within CollisionIndex::viewportPadding px of the edge
+    // Because these symbols aren't onscreen yet, we can skip the "fade in" animation,
+    // and if a subsequent viewport change brings them into view, they'll be fully
+    // visible right away.
+    const bool offscreen;
+};
+
+class Placement {
+public:
+    Placement(const TransformState&, MapMode mapMode);
+    void placeLayer(RenderSymbolLayer&, const mat4&, bool showCollisionBoxes);
+    bool commit(const Placement& prevPlacement, TimePoint);
+    void updateLayerOpacities(RenderSymbolLayer&);
+    JointOpacityState getOpacity(uint32_t crossTileSymbolID) const;
+    float symbolFadeChange(TimePoint now) const;
+    bool hasTransitions(TimePoint now) const;
+
+    const CollisionIndex& getCollisionIndex() const;
+
+    bool stillRecent(TimePoint now) const;
+    void setRecent(TimePoint now);
+    void setStale();
+private:
+
+    void placeLayerBucket(
+            SymbolBucket&,
+            const mat4& posMatrix,
+            const mat4& textLabelPlaneMatrix,
+            const mat4& iconLabelPlaneMatrix,
+            const float scale,
+            const float pixelRatio,
+            const bool showCollisionBoxes,
+            std::unordered_set<uint32_t>& seenCrossTileIDs);
+
+    void updateBucketOpacities(SymbolBucket&, std::set<uint32_t>&);
+
+    CollisionIndex collisionIndex;
+
+    TransformState state;
+    MapMode mapMode;
+    TimePoint commitTime;
+
+    std::unordered_map<uint32_t, JointPlacement> placements;
+    std::unordered_map<uint32_t, JointOpacityState> opacities;
+
+    TimePoint recentUntil;
+    bool stale = false;
+};
 
 } // namespace mbgl
diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp
index 3f4b36984b..442d84244e 100644
--- a/src/mbgl/tile/geometry_tile.hpp
+++ b/src/mbgl/tile/geometry_tile.hpp
@@ -8,7 +8,6 @@
 #include <mbgl/util/throttler.hpp>
 #include <mbgl/actor/actor.hpp>
 #include <mbgl/geometry/feature_index.hpp>
-#include <mbgl/layout/symbol_layout.hpp>
 
 #include <atomic>
 #include <memory>
@@ -127,7 +126,6 @@ private:
     optional<PremultipliedImage> iconAtlasImage;
 
     std::unordered_map<std::string, std::shared_ptr<Bucket>> symbolBuckets;
-    std::unordered_map<std::string, std::unique_ptr<SymbolLayout>> symbolLayouts;
 
     const MapMode mode;
     
diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp
index 969b137c1f..cf74bf3647 100644
--- a/src/mbgl/tile/geometry_tile_worker.cpp
+++ b/src/mbgl/tile/geometry_tile_worker.cpp
@@ -41,6 +41,18 @@ GeometryTileWorker::GeometryTileWorker(ActorRef<GeometryTileWorker> self_,
 GeometryTileWorker::~GeometryTileWorker() = default;
 
 /*
+   NOTE: The comments below are technically correct, but currently
+   conceptually misleading. The change to foreground label placement
+   means that:
+   (1) "placement" here is a misnomer: the remaining role of
+    "attemptPlacement" is symbol buffer generation
+   (2) Once a tile has completed layout, we will only run
+    "attemptPlacement" once
+   (3) Tiles won't be rendered until "attemptPlacement" has run once
+ 
+   TODO: Simplify GeometryTileWorker to fit its new role
+    https://github.com/mapbox/mapbox-gl-native/issues/10457
+
    GeometryTileWorker is a state machine. This is its transition diagram.
    States are indicated by [state], lines are transitions triggered by
    messages, (parentheses) are actions taken on transition.