Re-enable vertical glyph support.

7 files changed, 42 insertions, 55 deletions

diff --git a/src/mbgl/layout/symbol_instance.cpp b/src/mbgl/layout/symbol_instance.cpp
index 3bc13331f8..b396c05f4e 100644
--- a/src/mbgl/layout/symbol_instance.cpp
+++ b/src/mbgl/layout/symbol_instance.cpp
@@ -32,7 +32,7 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
     hasIcon(shapedIcon),
 
     // Create the collision features that will be used to check whether this symbol instance can be placed
-    textCollisionFeature(line_, anchor, shapedTextOrientations.second ?: shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature),
+    textCollisionFeature(line_, anchor, shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature),
     iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, SymbolPlacementType::Point, indexedFeature),
     featureIndex(featureIndex_),
     textOffset(textOffset_),
@@ -45,12 +45,10 @@ SymbolInstance::SymbolInstance(Anchor& anchor_,
             iconQuad = getIconQuad(*shapedIcon, layout, layoutTextSize, shapedTextOrientations.first);
         }
         if (shapedTextOrientations.first) {
-            auto quads = getGlyphQuads(shapedTextOrientations.first, layout, textPlacement, positions);
-            glyphQuads.insert(glyphQuads.end(), quads.begin(), quads.end());
+            horizontalGlyphQuads = getGlyphQuads(shapedTextOrientations.first, layout, textPlacement, positions);
         }
         if (shapedTextOrientations.second) {
-            auto quads = getGlyphQuads(shapedTextOrientations.second, layout, textPlacement, positions);
-            glyphQuads.insert(glyphQuads.end(), quads.begin(), quads.end());
+            verticalGlyphQuads = getGlyphQuads(shapedTextOrientations.second, layout, textPlacement, positions);
         }
     }
 
diff --git a/src/mbgl/layout/symbol_instance.hpp b/src/mbgl/layout/symbol_instance.hpp
index f40f38368a..231d1ca079 100644
--- a/src/mbgl/layout/symbol_instance.hpp
+++ b/src/mbgl/layout/symbol_instance.hpp
@@ -39,7 +39,8 @@ public:
     uint32_t index;
     bool hasText;
     bool hasIcon;
-    SymbolQuads glyphQuads;
+    SymbolQuads horizontalGlyphQuads;
+    SymbolQuads verticalGlyphQuads;
     optional<SymbolQuad> iconQuad;
     CollisionFeature textCollisionFeature;
     CollisionFeature iconCollisionFeature;
diff --git a/src/mbgl/layout/symbol_layout.cpp b/src/mbgl/layout/symbol_layout.cpp
index 206ee05027..26059dcaf9 100644
--- a/src/mbgl/layout/symbol_layout.cpp
+++ b/src/mbgl/layout/symbol_layout.cpp
@@ -416,18 +416,6 @@ std::vector<float> CalculateTileDistances(const GeometryCoordinates& line, const
 std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes) {
     auto bucket = std::make_unique<SymbolBucket>(layout, layerPaintProperties, textSize, iconSize, zoom, sdfIcons, iconsNeedLinear, symbolInstances);
 
-    // Calculate which labels can be shown and when they can be shown and
-    // create the bufers used for rendering.
-
-    const SymbolPlacementType textPlacement = layout.get<TextRotationAlignment>() != AlignmentType::Map
-                                                  ? SymbolPlacementType::Point
-                                                  : layout.get<SymbolPlacement>();
-    const SymbolPlacementType iconPlacement = layout.get<IconRotationAlignment>() != AlignmentType::Map
-                                                  ? SymbolPlacementType::Point
-                                                  : layout.get<SymbolPlacement>();
-
-    const bool keepUpright = layout.get<TextKeepUpright>();
-
     // this iterates over the *bucket's* symbol instances so that it can set the placedsymbol index. TODO cleanup
     for (SymbolInstance &symbolInstance : bucket->symbolInstances) {
 
@@ -439,16 +427,27 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes)
         // Insert final placement into collision tree and add glyphs/icons to buffers
 
         if (hasText) {
-            const bool useVerticalMode = false; // TODO: Add both versions of glyphs to buckets
             const Range<float> sizeData = bucket->textSizeBinder->getVertexSizeData(feature);
             bucket->text.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
-                    symbolInstance.textOffset, useVerticalMode, symbolInstance.line, CalculateTileDistances(symbolInstance.line, symbolInstance.anchor));
+                    symbolInstance.textOffset, symbolInstance.writingModes, symbolInstance.line, CalculateTileDistances(symbolInstance.line, symbolInstance.anchor));
             symbolInstance.placedTextIndices.push_back(bucket->text.placedSymbols.size() - 1);
 
-            for (const auto& symbol : symbolInstance.glyphQuads) {
+            for (const auto& symbol : symbolInstance.horizontalGlyphQuads) {
                 addSymbol(
                     bucket->text, sizeData, symbol,
-                    keepUpright, textPlacement, symbolInstance.anchor, bucket->text.placedSymbols.back());
+                    symbolInstance.anchor, bucket->text.placedSymbols.back());
+            }
+            
+            if (symbolInstance.writingModes & WritingModeType::Vertical) {
+                bucket->text.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
+                        symbolInstance.textOffset, WritingModeType::Vertical, symbolInstance.line, CalculateTileDistances(symbolInstance.line, symbolInstance.anchor));
+                symbolInstance.placedTextIndices.push_back(bucket->text.placedSymbols.size() - 1);
+
+                for (const auto& symbol : symbolInstance.verticalGlyphQuads) {
+                    addSymbol(
+                        bucket->text, sizeData, symbol,
+                        symbolInstance.anchor, bucket->text.placedSymbols.back());
+                }
             }
         }
 
@@ -456,11 +455,11 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(const bool showCollisionBoxes)
             if (symbolInstance.iconQuad) {
                 const Range<float> sizeData = bucket->iconSizeBinder->getVertexSizeData(feature);
                 bucket->icon.placedSymbols.emplace_back(symbolInstance.anchor.point, symbolInstance.anchor.segment, sizeData.min, sizeData.max,
-                        symbolInstance.iconOffset, false, symbolInstance.line, std::vector<float>());
+                        symbolInstance.iconOffset, WritingModeType::None, symbolInstance.line, std::vector<float>());
                 symbolInstance.placedIconIndices.push_back(bucket->icon.placedSymbols.size() - 1);
                 addSymbol(
                     bucket->icon, sizeData, *symbolInstance.iconQuad,
-                    keepUpright, iconPlacement, symbolInstance.anchor, bucket->icon.placedSymbols.back());
+                 symbolInstance.anchor, bucket->icon.placedSymbols.back());
             }
         }
         
@@ -481,8 +480,6 @@ template <typename Buffer>
 void SymbolLayout::addSymbol(Buffer& buffer,
                              const Range<float> sizeData,
                              const SymbolQuad& symbol,
-                             const bool keepUpright,
-                             const style::SymbolPlacementType placement,
                              const Anchor& labelAnchor,
                              PlacedSymbol& placedSymbol) {
     constexpr const uint16_t vertexLength = 4;
@@ -493,11 +490,6 @@ void SymbolLayout::addSymbol(Buffer& buffer,
     const auto &br = symbol.br;
     const auto &tex = symbol.tex;
 
-    if (placement == style::SymbolPlacementType::Line && keepUpright) {
-        // drop incorrectly oriented glyphs
-        if ((symbol.writingMode == WritingModeType::Vertical) != placedSymbol.useVerticalMode) return;
-    }
-
     if (buffer.segments.empty() || buffer.segments.back().vertexLength + vertexLength > std::numeric_limits<uint16_t>::max()) {
         buffer.segments.emplace_back(buffer.vertices.vertexSize(), buffer.triangles.indexSize());
     }
diff --git a/src/mbgl/layout/symbol_layout.hpp b/src/mbgl/layout/symbol_layout.hpp
index 99b2fd561c..620a317add 100644
--- a/src/mbgl/layout/symbol_layout.hpp
+++ b/src/mbgl/layout/symbol_layout.hpp
@@ -63,8 +63,6 @@ private:
     void addSymbol(Buffer&,
                    const Range<float> sizeData,
                    const SymbolQuad&,
-                   const bool keepUpright,
-                   const style::SymbolPlacementType,
                    const Anchor& labelAnchor,
                    PlacedSymbol& placedSymbol);
 
diff --git a/src/mbgl/layout/symbol_projection.cpp b/src/mbgl/layout/symbol_projection.cpp
index 6b1dbf8fac..d99b7e7620 100644
--- a/src/mbgl/layout/symbol_projection.cpp
+++ b/src/mbgl/layout/symbol_projection.cpp
@@ -261,7 +261,8 @@ namespace mbgl {
                               const mat4& labelPlaneMatrix,
                               const mat4& glCoordMatrix,
                               gl::VertexVector<SymbolDynamicLayoutAttributes::Vertex>& dynamicVertexArray,
-                              const Point<float>& projectedAnchorPoint) {
+                              const Point<float>& projectedAnchorPoint,
+                              const float aspectRatio) {
         const float fontScale = fontSize / 24.0;
         const float lineOffsetX = symbol.lineOffset[0] * fontSize;
         const float lineOffsetY = symbol.lineOffset[1] * fontSize;
@@ -278,30 +279,28 @@ namespace mbgl {
             const Point<float> firstPoint = project(firstAndLastGlyph->first.point, glCoordMatrix).first;
             const Point<float> lastPoint = project(firstAndLastGlyph->second.point, glCoordMatrix).first;
 
-            if (keepUpright && !flip &&
-                (symbol.useVerticalMode ? firstPoint.y < lastPoint.y : firstPoint.x > lastPoint.x)) {
-                return PlacementResult::NeedsFlipping;
-            }
-            
             if (keepUpright && !flip) {
-                if (symbol.writingMode == WritingModeType::Horizontal) {
+                if (symbol.writingModes == (WritingModeType::Horizontal | WritingModeType::Vertical)) {
                     // On top of choosing whether to flip, choose whether to render this version of the glyphs or the alternate
                     // vertical glyphs. We can't just filter out vertical glyphs in the horizontal range because the horizontal
                     // and vertical versions can have slightly different projections which could lead to angles where both or
                     // neither showed.
-                    if (std::abs(lastPoint.y - firstPoint.y) > std::abs(lastPoint.x - firstPoint.x)) {
+                    auto rise = std::abs(lastPoint.y - firstPoint.y);
+                    auto run = std::abs(lastPoint.x - firstPoint.x) * aspectRatio;
+                    if (rise > run) {
                         return PlacementResult::UseVertical;
                     }
                 }
 
-                if (symbol.writingMode == WritingModeType::Vertical ? firstPoint.y < lastPoint.y : firstPoint.x > lastPoint.x) {
+                if ((symbol.writingModes == WritingModeType::Vertical) ?
+                    (firstPoint.y < lastPoint.y) :
+                    (firstPoint.x > lastPoint.x)) {
                     // Includes "horizontalOnly" case for labels without vertical glyphs
                     return PlacementResult::NeedsFlipping;
                 }
 
             }
 
-
             placedGlyphs.push_back(firstAndLastGlyph->first);
             for (size_t glyphIndex = 1; glyphIndex < symbol.glyphOffsets.size() - 1; glyphIndex++) {
                 const float glyphOffsetX = symbol.glyphOffsets[glyphIndex];
@@ -324,7 +323,7 @@ namespace mbgl {
                     projectedVertex.first :
                     projectTruncatedLineSegment(symbol.anchorPoint,tileSegmentEnd, a, 1, posMatrix);
                 
-                if (symbol.useVerticalMode ? b.y > a.y : b.x < a.x) {
+                if (symbol.writingModes == WritingModeType::Vertical ? b.y > a.y : b.x < a.x) {
                     return PlacementResult::NeedsFlipping;
                 }
             }
@@ -371,7 +370,7 @@ namespace mbgl {
             // Don't do calculations for vertical glyphs unless the previous symbol was horizontal
             // and we determined that vertical glyphs were necessary.
             // Also don't do calculations for symbols that are collided and fully faded out
-            if (placedSymbol.hidden || (placedSymbol.writingMode == WritingModeType::Vertical && !useVertical)) {
+            if (placedSymbol.hidden || (placedSymbol.writingModes == WritingModeType::Vertical && !useVertical)) {
                 hideGlyphs(placedSymbol.glyphOffsets.size(), dynamicVertexArray);
                 continue;
             }
@@ -397,13 +396,13 @@ namespace mbgl {
             
             const Point<float> anchorPoint = project(placedSymbol.anchorPoint, labelPlaneMatrix).first;
 
-            PlacementResult placeUnflipped = placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, false /*unflipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray, anchorPoint);
+            PlacementResult placeUnflipped = placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, false /*unflipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray, anchorPoint, state.getSize().aspectRatio());
             
             useVertical = placeUnflipped == PlacementResult::UseVertical;
 
             if (placeUnflipped == PlacementResult::NotEnoughRoom || useVertical ||
                 (placeUnflipped == PlacementResult::NeedsFlipping &&
-                 placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, true /*flipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray, anchorPoint) == PlacementResult::NotEnoughRoom)) {
+                 placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, true /*flipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray, anchorPoint, state.getSize().aspectRatio()) == PlacementResult::NotEnoughRoom)) {
                 hideGlyphs(placedSymbol.glyphOffsets.size(), dynamicVertexArray);
             }
         }
diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp
index c18f89078d..131ee72c43 100644
--- a/src/mbgl/renderer/buckets/symbol_bucket.hpp
+++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp
@@ -19,24 +19,20 @@ namespace mbgl {
 class PlacedSymbol {
 public:
     PlacedSymbol(Point<float> anchorPoint_, uint16_t segment_, float lowerSize_, float upperSize_,
-            std::array<float, 2> lineOffset_, bool useVerticalMode_, const GeometryCoordinates& line_, const std::vector<float>& tileDistances_) :
+            std::array<float, 2> lineOffset_, WritingModeType writingModes_, const GeometryCoordinates& line_, const std::vector<float>& tileDistances_) :
         anchorPoint(anchorPoint_), segment(segment_), lowerSize(lowerSize_), upperSize(upperSize_),
-        lineOffset(lineOffset_), useVerticalMode(useVerticalMode_), line(line_), tileDistances(tileDistances_)
+        lineOffset(lineOffset_), writingModes(writingModes_), line(line_), tileDistances(tileDistances_), hidden(false)
     {
-        // TODO WIP hook these up
-        writingMode = WritingModeType::None;
-        hidden = false;
     }
     Point<float> anchorPoint;
     uint16_t segment;
     float lowerSize;
     float upperSize;
     std::array<float, 2> lineOffset;
-    bool useVerticalMode;
+    WritingModeType writingModes;
     GeometryCoordinates line;
     std::vector<float> tileDistances;
     std::vector<float> glyphOffsets;
-    WritingModeType writingMode;
     bool hidden;
 };
 
diff --git a/src/mbgl/text/placement.cpp b/src/mbgl/text/placement.cpp
index fa277b2b8b..bfdb6136a1 100644
--- a/src/mbgl/text/placement.cpp
+++ b/src/mbgl/text/placement.cpp
@@ -218,7 +218,10 @@ void Placement::updateBucketOpacities(SymbolBucket& bucket) {
         // TODO check if hasText is the right thing here, or if there are cases where hasText is true but it's not added to the buffers
         if (symbolInstance.hasText) {
             auto opacityVertex = SymbolOpacityAttributes::vertex(opacityState.text.placed, opacityState.text.opacity);
-            for (size_t i = 0; i < symbolInstance.glyphQuads.size() * 4; i++) {
+            for (size_t i = 0; i < symbolInstance.horizontalGlyphQuads.size() * 4; i++) {
+                bucket.text.opacityVertices.emplace_back(opacityVertex);
+            }
+            for (size_t i = 0; i < symbolInstance.verticalGlyphQuads.size() * 4; i++) {
                 bucket.text.opacityVertices.emplace_back(opacityVertex);
             }
             // TODO On JS we avoid setting this if it hasn't chnaged, but it may be cheap enough here we don't care