Merge pull request tangrams#434 from tangrams/fix-bbox

Fix bbox (Final round!)
matteblair · Dec 14, 2015 · abdb310 · abdb310
2 parents f17cd22 + c853993
commit abdb310
Show file tree

Hide file tree

Showing 14 changed files with 59 additions and 52 deletions.
diff --git a/core/resources/scene.yaml b/core/resources/scene.yaml
@@ -97,7 +97,6 @@ layers:
             polygons:
                 order: 0
                 color: '#f0ebeb'
-
     landuse:
         data: { source: osm }
         filter:
@@ -327,7 +326,6 @@ layers:
                 polygons:
                     style: polygons
                     color: '#bddec5'
-
     poi_labels:
         data: { source: osm, layer: pois }
         filter: { name: true, not: { kind: [peak, viewpoint, bicycle_rental, car_sharing] }, $zoom: { min: 15 } }

diff --git a/core/src/labels/label.cpp b/core/src/labels/label.cpp
@@ -8,15 +8,13 @@
 namespace Tangram {
 
 Label::Label(Label::Transform _transform, glm::vec2 _size, Type _type, LabelMesh& _mesh,
-             Range _vertexRange, Options _options, size_t _hash) :
-    m_options(_options),
-    m_hash(_hash),
+             Range _vertexRange, Options _options) :
     m_type(_type),
     m_transform(_transform),
     m_dim(_size),
     m_mesh(_mesh),
-    m_vertexRange(_vertexRange)
-{
+    m_vertexRange(_vertexRange),
+    m_options(_options) {
     if (!m_options.collide || m_type == Type::debug){
         enterState(State::visible, 1.0);
     } else {
@@ -228,7 +226,6 @@ void Label::resetState() {
     m_updateMeshVisibility = true;
     m_dirty = true;
     m_proxy = false;
-    m_skipTransitions = false;
     enterState(State::wait_occ, 0.0);
 }
 

diff --git a/core/src/labels/label.h b/core/src/labels/label.h
@@ -81,10 +81,14 @@ class Label {
         Transition selectTransition;
         Transition hideTransition;
         Transition showTransition;
+        float buffer = 0.f;
+
+        // the label hash based on its styling parameters
+        size_t paramHash;
     };
 
     Label(Transform _transform, glm::vec2 _size, Type _type, LabelMesh& _mesh, Range _vertexRange,
-            Options _options, size_t _hash);
+            Options _options);
 
     virtual ~Label();
 
@@ -124,7 +128,7 @@ class Label {
 
     /* Whether the label belongs to a proxy tile */
     bool isProxy() const { return m_proxy; }
-    size_t hash() const { return m_hash; }
+    size_t hash() const { return m_options.paramHash; }
     const glm::vec2& dimension() const { return m_dim; }
     /* Gets for label options: color and offset */
     const Options& options() const { return m_options; }
@@ -156,12 +160,8 @@ class Label {
     bool m_occlusionSolved;
     // whether or not we need to update the mesh visibilit (alpha channel)
     bool m_updateMeshVisibility;
-    // label options
-    Options m_options;
     // whether this label should skip transitions to move to first visible state
     bool m_skipTransitions;
-    // the label hash based on its styling parameters
-    size_t m_hash;
 
 protected:
 
@@ -184,6 +184,8 @@ class Label {
     LabelMesh& m_mesh;
     // first vertex and count in m_mesh vertices
     Range m_vertexRange;
+    // label options
+    Options m_options;
 };
 
 }
@@ -208,4 +210,3 @@ namespace std {
         }
     };
 }
-
diff --git a/core/src/labels/labels.cpp b/core/src/labels/labels.cpp
@@ -260,7 +260,7 @@ void Labels::drawDebug(const View& _view) {
                     Primitives::setColor(0x00ff00);
                     break;
                 case Label::State::visible:
-                    Primitives::setColor(0xffffff);
+                    Primitives::setColor(0x000000);
                     break;
                 case Label::State::wait_occ:
                     Primitives::setColor(0x0000ff);

diff --git a/core/src/labels/spriteLabel.cpp b/core/src/labels/spriteLabel.cpp
@@ -6,8 +6,8 @@ namespace Tangram {
 using namespace LabelProperty;
 
 SpriteLabel::SpriteLabel(Label::Transform _transform, glm::vec2 _size, LabelMesh& _mesh, int _vertexOffset,
-        Label::Options _options, float _extrudeScale, LabelProperty::Anchor _anchor, size_t _hash) :
-    Label(_transform, _size, Label::Type::point, _mesh, {_vertexOffset, 4}, _options, _hash),
+        Label::Options _options, float _extrudeScale, LabelProperty::Anchor _anchor) :
+    Label(_transform, _size, Label::Type::point, _mesh, {_vertexOffset, 4}, _options),
     m_extrudeScale(_extrudeScale)
 {
     switch(_anchor) {

diff --git a/core/src/labels/spriteLabel.h b/core/src/labels/spriteLabel.h
@@ -9,7 +9,7 @@ class SpriteLabel : public Label {
 public:
 
     SpriteLabel(Label::Transform _transform, glm::vec2 _size, LabelMesh& _mesh, int _vertexOffset,
-                Label::Options _options, float _extrudeScale, LabelProperty::Anchor _anchor, size_t _hash);
+                Label::Options _options, float _extrudeScale, LabelProperty::Anchor _anchor);
 
     void updateBBoxes(float _zoomFract) override;
     void align(glm::vec2& _screenPosition, const glm::vec2& _ap1, const glm::vec2& _ap2) override;

diff --git a/core/src/labels/textLabel.cpp b/core/src/labels/textLabel.cpp
@@ -5,11 +5,12 @@ namespace Tangram {
 using namespace LabelProperty;
 
 TextLabel::TextLabel(Label::Transform _transform, Type _type, glm::vec2 _dim, TextBuffer& _mesh, Range _vertexRange,
-    Label::Options _options, FontContext::FontMetrics _metrics, int _nLines, Anchor _anchor, size_t _hash) :
-    Label(_transform, _dim, _type, static_cast<LabelMesh&>(_mesh), _vertexRange, _options, _hash),
+    Label::Options _options, FontContext::FontMetrics _metrics, int _nLines, Anchor _anchor, glm::vec2 _quadsLocalOrigin) :
+    Label(_transform, _dim, _type, static_cast<LabelMesh&>(_mesh), _vertexRange, _options),
     m_metrics(_metrics),
-    m_nLines(_nLines)
-{
+    m_nLines(_nLines),
+    m_quadLocalOrigin(_quadsLocalOrigin) {
+
     if (m_type == Type::point) {
         glm::vec2 halfDim = m_dim * 0.5f;
         switch(_anchor) {
@@ -42,11 +43,11 @@ void TextLabel::updateBBoxes(float _zoomFract) {
     obbCenter += t * m_dim.x * 0.5f;
     // move down on the perpendicular to estimated font baseline
     obbCenter -= m_perpAxis * m_dim.y * 0.5f;
-    // ajdust with baseline
-    obbCenter += m_perpAxis * m_metrics.lineHeight * (float) (m_nLines - 1);
-    obbCenter -= m_perpAxis * m_metrics.descender;
+    // ajdust with local origin of the quads
+    obbCenter += m_perpAxis * m_quadLocalOrigin.y + m_perpAxis * m_dim.y;
+    obbCenter += t * m_quadLocalOrigin.x;
 
-    m_obb = OBB(obbCenter.x, obbCenter.y, m_transform.state.rotation, m_dim.x, m_dim.y);
+    m_obb = OBB(obbCenter.x, obbCenter.y, m_transform.state.rotation, m_dim.x + m_options.buffer, m_dim.y + m_options.buffer);
     m_aabb = m_obb.getExtent();
 }
 
@@ -56,7 +57,13 @@ void TextLabel::align(glm::vec2& _screenPosition, const glm::vec2& _ap1, const g
         case Type::debug:
         case Type::point:
             // modify position set by updateScreenTransform()
-            _screenPosition.x -= m_dim.x * 0.5f;
+            _screenPosition.x -= m_dim.x * 0.5f + m_quadLocalOrigin.x;
+            _screenPosition.y -= m_metrics.descender;
+
+            if (m_nLines > 1) {
+                _screenPosition.y -= m_dim.y * 0.5f;
+                _screenPosition.y += (m_metrics.lineHeight + m_metrics.descender);
+            }
             _screenPosition += m_anchor;
             break;
         case Type::line: {
@@ -66,8 +73,7 @@ void TextLabel::align(glm::vec2& _screenPosition, const glm::vec2& _ap1, const g
             // move back by half the length (so that text will be drawn centered)
             glm::vec2 direction = glm::normalize(_ap1 - _ap2);
             _screenPosition += direction * m_dim.x * 0.5f;
-
-            _screenPosition += m_perpAxis * (m_dim.y * 0.5f  + m_metrics.descender);
+            _screenPosition += m_perpAxis * (m_dim.y * 0.5f + 2.f * m_metrics.descender);
             break;
         }
     }

diff --git a/core/src/labels/textLabel.h b/core/src/labels/textLabel.h
@@ -11,8 +11,8 @@ class TextLabel : public Label {
 
 public:
     TextLabel(Label::Transform _transform, Type _type, glm::vec2 _dim, TextBuffer& _mesh, Range _vertexRange,
-              Label::Options _options, FontContext::FontMetrics _metrics, int _nLines, LabelProperty::Anchor _anchor,
-              size_t _hash);
+              Label::Options _options, FontContext::FontMetrics _metrics, int _nLines, LabelProperty::Anchor _anchor, 
+              glm::vec2 _quadsLocalOrigin);
 
     void updateBBoxes(float _zoomFract) override;
 
@@ -26,6 +26,7 @@ class TextLabel : public Label {
 
     glm::vec2 m_anchor;
     glm::vec2 m_perpAxis;
+    glm::vec2 m_quadLocalOrigin;
 
 };
 

diff --git a/core/src/style/pointStyle.cpp b/core/src/style/pointStyle.cpp
@@ -65,11 +65,6 @@ bool PointStyle::checkRule(const DrawRule& _rule) const {
     return true;
 }
 
-size_t PointStyle::hashParams(const Parameters& _params) const {
-    std::hash<Parameters> hash;
-    return hash(_params);
-}
-
 PointStyle::Parameters PointStyle::applyRule(const DrawRule& _rule, const Properties& _props, float _zoom) const {
 
     Parameters p;
@@ -111,6 +106,9 @@ PointStyle::Parameters PointStyle::applyRule(const DrawRule& _rule, const Proper
         p.labelOptions.properties = std::make_shared<Properties>(_props);
     }
 
+    std::hash<Parameters> hash;
+    p.labelOptions.paramHash = hash(p);
+
     return p;
 }
 
@@ -167,7 +165,7 @@ void PointStyle::buildPoint(const Point& _point, const DrawRule& _rule, const Pr
     Label::Transform transform = { glm::vec2(_point) };
 
     mesh.addLabel(std::make_unique<SpriteLabel>(transform, p.size, mesh, _mesh.numVertices(),
-                p.labelOptions, p.extrudeScale, p.anchor, hashParams(p)));
+                p.labelOptions, p.extrudeScale, p.anchor));
 
     std::vector<Label::Vertex> vertices;
 
@@ -195,7 +193,7 @@ void PointStyle::buildLine(const Line& _line, const DrawRule& _rule, const Prope
         Label::Transform transform = { glm::vec2(_line[i]) };
 
         mesh.addLabel(std::make_unique<SpriteLabel>(transform, p.size, mesh, _mesh.numVertices(),
-                    p.labelOptions, p.extrudeScale, p.anchor, hashParams(p)));
+                    p.labelOptions, p.extrudeScale, p.anchor));
         pushQuad(vertices, p.size, {uvsQuad.x, uvsQuad.y}, {uvsQuad.z, uvsQuad.w},
                 p.color, p.extrudeScale);
     }
@@ -227,7 +225,7 @@ void PointStyle::buildPolygon(const Polygon& _polygon, const DrawRule& _rule, co
                 Label::Transform transform = { glm::vec2(point) };
 
                 mesh.addLabel(std::make_unique<SpriteLabel>(transform, p.size, mesh, _mesh.numVertices(),
-                            p.labelOptions, p.extrudeScale, p.anchor, hashParams(p)));
+                            p.labelOptions, p.extrudeScale, p.anchor));
                 pushQuad(vertices, p.size, {uvsQuad.x, uvsQuad.y}, {uvsQuad.z, uvsQuad.w},
                         p.color, p.extrudeScale);
             }
@@ -238,7 +236,7 @@ void PointStyle::buildPolygon(const Polygon& _polygon, const DrawRule& _rule, co
         Label::Transform transform = { c };
 
         mesh.addLabel(std::make_unique<SpriteLabel>(transform, p.size, mesh, _mesh.numVertices(),
-                    p.labelOptions, p.extrudeScale, p.anchor, hashParams(p)));
+                    p.labelOptions, p.extrudeScale, p.anchor));
         pushQuad(vertices, p.size,
                 {uvsQuad.x, uvsQuad.y}, {uvsQuad.z, uvsQuad.w}, p.color, p.extrudeScale);
     }

diff --git a/core/src/style/pointStyle.h b/core/src/style/pointStyle.h
@@ -53,8 +53,6 @@ class PointStyle : public Style {
     std::shared_ptr<SpriteAtlas> m_spriteAtlas;
     std::shared_ptr<Texture> m_texture;
 
-    inline size_t hashParams(const Parameters& _params) const;
-
 };
 
 }

diff --git a/core/src/style/textStyle.cpp b/core/src/style/textStyle.cpp
@@ -145,6 +145,12 @@ auto TextStyle::applyRule(const DrawRule& _rule, const Properties& _props) const
     float emSize = p.fontSize / 16.f;
     p.blurSpread = m_sdf ? emSize * 5.0f : 0.0f;
 
+    float boundingBoxBuffer = -p.fontSize / 2.f;
+    p.labelOptions.buffer = boundingBoxBuffer;
+
+    std::hash<Parameters> hash;
+    p.labelOptions.paramHash = hash(p);
+
     return p;
 }
 

diff --git a/core/src/text/textBuffer.cpp b/core/src/text/textBuffer.cpp
@@ -219,6 +219,8 @@ bool TextBuffer::addLabel(const TextStyle::Parameters& _params, Label::Transform
 
     /// Generate the quads
     float yMin = std::numeric_limits<float>::max();
+    float xMin = std::numeric_limits<float>::max();
+
     for (int i = 0; i < int(quads.size()); ++i) {
         if (wordBreaks.size() > 0) {
             bool skip = false;
@@ -242,21 +244,19 @@ bool TextBuffer::addLabel(const TextStyle::Parameters& _params, Label::Transform
         vertices.push_back({{q.x1, q.y1}, {q.s1, q.t1}, { 1.f,  1.f, 0.f}, _params.fill, stroke});
 
         yMin = std::min(yMin, q.y0);
+        xMin = std::min(xMin, q.x0);
+
         bbox.x = std::max(bbox.x, q.x1);
         bbox.y = std::max(bbox.y, std::abs(yMin - q.y1));
     }
 
-    // Adjust the bounding boxes
-    bbox.y += metrics.descender * nLine;
-    // Approximate the first left side bearing
-    bbox.x += quads[0].x0;
+    bbox.x -= xMin;
+    glm::vec2 quadsLocalOrigin(xMin, quads[0].y0);
 
     _fontContext.unlock();
 
-
-    std::hash<TextStyle::Parameters> hash;
     m_labels.emplace_back(new TextLabel(_transform, _type, bbox, *this, { vertexOffset, numVertices },
-                                        _params.labelOptions, metrics, nLine, _params.anchor, hash(_params)));
+                                        _params.labelOptions, metrics, nLine, _params.anchor, quadsLocalOrigin));
 
     // TODO: change this in TypeMesh::adVertices()
     m_nVertices = vertices.size();

diff --git a/tests/unit/labelTests.cpp b/tests/unit/labelTests.cpp
@@ -15,7 +15,7 @@ TextBuffer dummy(nullptr);
 TextLabel makeLabel(Label::Transform _transform, Label::Type _type) {
     Label::Options options;
     options.offset = {0.0f, 0.0f};
-    return TextLabel(_transform, _type, {0, 0}, dummy, {0, 0}, options, {}, 1, LabelProperty::Anchor::center, 0);
+    return TextLabel(_transform, _type, {0, 0}, dummy, {0, 0}, options, {}, 1, LabelProperty::Anchor::center, {});
 }
 
 TEST_CASE( "Ensure the transition from wait -> sleep when occlusion happens", "[Core][Label]" ) {

diff --git a/tests/unit/labelsTests.cpp b/tests/unit/labelsTests.cpp
@@ -22,7 +22,9 @@ std::unique_ptr<TextLabel> makeLabel(Label::Transform _transform, Label::Type _t
     options.properties->add("id", id);
     options.interactive = true;
 
-    return std::unique_ptr<TextLabel>(new TextLabel(_transform, _type, {10, 10},dummy, {0, 0}, options, {}, 1, LabelProperty::Anchor::center, 0));
+    return std::unique_ptr<TextLabel>(new TextLabel(_transform, _type, {10, 10},dummy,
+                                                    {0, 0}, options, {}, 1, LabelProperty::Anchor::center,
+                                                    _transform.modelPosition1 - glm::vec2{5,5}));
 }
 
 TEST_CASE("Test getFeaturesAtPoint", "[Labels][FeaturePicking]") {
-Original file line number
+Diff line change
@@ Expand Up / @@ -53,8 +53,6 @@ class PointStyle : public Style { @@
         std::shared_ptr<SpriteAtlas> m_spriteAtlas;
         std::shared_ptr<Texture> m_texture;
-        inline size_t hashParams(const Parameters& _params) const;
     };
     }
@@ Expand Down @@