[core] Cleanup SymbolLayout

Reuse reverse rotation matrix from CollisionTile when populating the
collision boxes.

3 files changed, 36 insertions, 34 deletions

diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index cc8db3ab98..07ba2bf4a3 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -8,12 +8,14 @@
 #include <mbgl/text/get_anchors.hpp>
 #include <mbgl/text/glyph_atlas.hpp>
 #include <mbgl/text/collision_tile.hpp>
+#include <mbgl/util/constants.hpp>
 #include <mbgl/util/utf.hpp>
 #include <mbgl/util/token.hpp>
 #include <mbgl/util/math.hpp>
 #include <mbgl/util/std.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/string.hpp>
+#include <mbgl/math/clamp.hpp>
 #include <mbgl/math/minmax.hpp>
 #include <mbgl/platform/platform.hpp>
 #include <mbgl/platform/log.hpp>
@@ -53,8 +55,8 @@ SymbolLayout::SymbolLayout(std::string bucketName_,
     auto layerName = layer.getName();
 
     // Determine and load glyph ranges
-    const size_t featureCount = static_cast<size_t>(layer.featureCount());
-    for (size_t i = 0; i < featureCount; i++) {
+    const size_t featureCount = layer.featureCount();
+    for (size_t i = 0; i < featureCount; ++i) {
         auto feature = layer.getFeature(i);
         if (!filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); }))
             continue;
@@ -487,18 +489,20 @@ void SymbolLayout::addSymbols(Buffer &buffer, const SymbolQuads &symbols, float
 
 void SymbolLayout::addToDebugBuffers(CollisionTile& collisionTile, SymbolBucket& bucket) {
 
+    if (!hasSymbolInstances()) {
+        return;
+    }
+
     const float yStretch = collisionTile.yStretch;
-    const float angle = collisionTile.config.angle;
-    float angle_sin = std::sin(-angle);
-    float angle_cos = std::cos(-angle);
-    std::array<float, 4> matrix = {{angle_cos, -angle_sin, angle_sin, angle_cos}};
 
-    for (const SymbolInstance &symbolInstance : symbolInstances) {
-        for (int i = 0; i < 2; i++) {
-            auto& feature = i == 0 ?
-                symbolInstance.textCollisionFeature :
-                symbolInstance.iconCollisionFeature;
+    auto& collisionBox = bucket.collisionBox;
+    if (collisionBox.groups.empty()) {
+        // Move to a new group because the old one can't hold the geometry.
+        collisionBox.groups.emplace_back();
+    }
 
+    for (const SymbolInstance &symbolInstance : symbolInstances) {
+        auto populateCollisionBox = [&](const auto& feature) {
             for (const CollisionBox &box : feature.boxes) {
                 auto& anchor = box.anchor;
 
@@ -506,19 +510,13 @@ void SymbolLayout::addToDebugBuffers(CollisionTile& collisionTile, SymbolBucket&
                 Point<float> tr{box.x2, box.y1 * yStretch};
                 Point<float> bl{box.x1, box.y2 * yStretch};
                 Point<float> br{box.x2, box.y2 * yStretch};
-                tl = util::matrixMultiply(matrix, tl);
-                tr = util::matrixMultiply(matrix, tr);
-                bl = util::matrixMultiply(matrix, bl);
-                br = util::matrixMultiply(matrix, br);
-
-                const float maxZoom = util::max(0.0f, util::min(25.0f, static_cast<float>(zoom + log(box.maxScale) / log(2))));
-                const float placementZoom= util::max(0.0f, util::min(25.0f, static_cast<float>(zoom + log(box.placementScale) / log(2))));
-
-                auto& collisionBox = bucket.collisionBox;
-                if (collisionBox.groups.empty()) {
-                    // Move to a new group because the old one can't hold the geometry.
-                    collisionBox.groups.emplace_back();
-                }
+                tl = util::matrixMultiply(collisionTile.reverseRotationMatrix, tl);
+                tr = util::matrixMultiply(collisionTile.reverseRotationMatrix, tr);
+                bl = util::matrixMultiply(collisionTile.reverseRotationMatrix, bl);
+                br = util::matrixMultiply(collisionTile.reverseRotationMatrix, br);
+
+                const float maxZoom = util::clamp(zoom + log(box.maxScale) / log(2), util::MIN_ZOOM, util::MAX_ZOOM);
+                const float placementZoom = util::clamp(zoom + log(box.placementScale) / log(2), util::MIN_ZOOM, util::MAX_ZOOM);
 
                 collisionBox.vertices.emplace_back(anchor.x, anchor.y, tl.x, tl.y, maxZoom, placementZoom);
                 collisionBox.vertices.emplace_back(anchor.x, anchor.y, tr.x, tr.y, maxZoom, placementZoom);
@@ -529,10 +527,12 @@ void SymbolLayout::addToDebugBuffers(CollisionTile& collisionTile, SymbolBucket&
                 collisionBox.vertices.emplace_back(anchor.x, anchor.y, bl.x, bl.y, maxZoom, placementZoom);
                 collisionBox.vertices.emplace_back(anchor.x, anchor.y, tl.x, tl.y, maxZoom, placementZoom);
 
-                auto& group= collisionBox.groups.back();
+                auto& group = collisionBox.groups.back();
                 group.vertexLength += 8;
             }
-        }
+        };
+        populateCollisionBox(symbolInstance.textCollisionFeature);
+        populateCollisionBox(symbolInstance.iconCollisionFeature);
     }
 }
 
diff --git a/src/mbgl/text/collision_tile.cpp b/src/mbgl/text/collision_tile.cpp
index ec92782317..d6fc1a6ada 100644
--- a/src/mbgl/text/collision_tile.cpp
+++ b/src/mbgl/text/collision_tile.cpp
@@ -2,6 +2,7 @@
 #include <mbgl/geometry/feature_index.hpp>
 #include <mbgl/util/constants.hpp>
 #include <mbgl/util/math.hpp>
+#include <mbgl/math/minmax.hpp>
 
 #include <mapbox/geometry/envelope.hpp>
 #include <mapbox/geometry/multi_point.hpp>
@@ -53,7 +54,7 @@ float CollisionTile::findPlacementScale(float minPlacementScale, const Point<flo
     if (std::isnan(s1) || std::isnan(s2)) s1 = s2 = 1;
     if (std::isnan(s3) || std::isnan(s4)) s3 = s4 = 1;
 
-    float collisionFreeScale = ::fmin(::fmax(s1, s2), ::fmax(s3, s4));
+    float collisionFreeScale = util::min(util::max(s1, s2), util::max(s3, s4));
 
     if (collisionFreeScale > blocking.maxScale) {
         // After a box's maxScale the label has shrunk enough that the box is no longer needed to cover it,
@@ -106,10 +107,10 @@ float CollisionTile::placeFeature(const CollisionFeature& feature, const bool al
             const Point<float> rbl = util::matrixMultiply(reverseRotationMatrix, bl);
             const Point<float> rbr = util::matrixMultiply(reverseRotationMatrix, br);
             CollisionBox rotatedBox(box.anchor,
-                    ::fmin(::fmin(rtl.x, rtr.x), ::fmin(rbl.x, rbr.x)),
-                    ::fmin(::fmin(rtl.y, rtr.y), ::fmin(rbl.y, rbr.y)),
-                    ::fmax(::fmax(rtl.x, rtr.x), ::fmax(rbl.x, rbr.x)),
-                    ::fmax(::fmax(rtl.y, rtr.y), ::fmax(rbl.y, rbr.y)),
+                    util::min(rtl.x, rtr.x, rbl.x, rbr.x),
+                    util::min(rtl.y, rtr.y, rbl.y, rbr.y),
+                    util::max(rtl.x, rtr.x, rbl.x, rbr.x),
+                    util::max(rtl.y, rtr.y, rbl.y, rbr.y),
                     box.maxScale);
 
             for (auto& blocking : edges) {
diff --git a/src/mbgl/text/collision_tile.hpp b/src/mbgl/text/collision_tile.hpp
index 8d660eb6e6..dbc23b2a79 100644
--- a/src/mbgl/text/collision_tile.hpp
+++ b/src/mbgl/text/collision_tile.hpp
@@ -50,6 +50,10 @@ public:
     const float maxScale = 2.0f;
     float yStretch;
 
+    std::array<float, 4> rotationMatrix;
+    std::array<float, 4> reverseRotationMatrix;
+    std::array<CollisionBox, 4> edges;
+
 private:
     float findPlacementScale(float minPlacementScale,
             const Point<float>& anchor, const CollisionBox& box,
@@ -58,9 +62,6 @@ private:
 
     Tree tree;
     Tree ignoredTree;
-    std::array<float, 4> rotationMatrix;
-    std::array<float, 4> reverseRotationMatrix;
-    std::array<CollisionBox, 4> edges;
 };
 
 } // namespace mbgl