From 6cbeeff6898211d3c3955e34503358e8b2ee127b Mon Sep 17 00:00:00 2001 From: Chris Loer Date: Fri, 25 Aug 2017 10:45:24 -0700 Subject: [core] Fix issue #9843 (overzoomed line label rendering glitch). Ports fix for GL JS issue #5112. Line label projection can't be based on tile geometry that's behind the plane of the camera. The relevant tests are still ignored because the overzoomed collision behavior is different between native and JS. --- platform/node/test/ignores.json | 4 +- src/mbgl/layout/symbol_projection.cpp | 72 ++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/platform/node/test/ignores.json b/platform/node/test/ignores.json index 3e8b21f9ad..90e0e22e70 100644 --- a/platform/node/test/ignores.json +++ b/platform/node/test/ignores.json @@ -62,8 +62,8 @@ "render-tests/text-pitch-alignment/auto-text-rotation-alignment-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/9732", "render-tests/text-pitch-alignment/map-text-rotation-alignment-map": "https://github.com/mapbox/mapbox-gl-native/issues/9732", "render-tests/text-pitch-alignment/map-text-rotation-alignment-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/9732", - "render-tests/text-pitch-alignment/viewport-overzoomed": "https://github.com/mapbox/mapbox-gl-native/issues/9843", - "render-tests/text-pitch-alignment/viewport-overzoomed-single-glyph": "https://github.com/mapbox/mapbox-gl-native/issues/9843", + "render-tests/text-pitch-alignment/viewport-overzoomed": "https://github.com/mapbox/mapbox-gl-js/issues/5095", + "render-tests/text-pitch-alignment/viewport-overzoomed-single-glyph": "https://github.com/mapbox/mapbox-gl-js/issues/5095", "render-tests/text-pitch-alignment/viewport-text-rotation-alignment-map": "https://github.com/mapbox/mapbox-gl-native/issues/9732", "render-tests/text-pitch-alignment/viewport-text-rotation-alignment-viewport": "https://github.com/mapbox/mapbox-gl-native/issues/9732", "render-tests/text-pitch-scaling/line-half": "https://github.com/mapbox/mapbox-gl-native/issues/9732", diff --git a/src/mbgl/layout/symbol_projection.cpp b/src/mbgl/layout/symbol_projection.cpp index 8ccd6a0410..279d251f8f 100644 --- a/src/mbgl/layout/symbol_projection.cpp +++ b/src/mbgl/layout/symbol_projection.cpp @@ -94,10 +94,12 @@ namespace mbgl { } - Point project(const Point& point, const mat4& matrix) { + typedef std::pair,float> PointAndCameraDistance; + + PointAndCameraDistance project(const Point& point, const mat4& matrix) { vec4 pos = {{ point.x, point.y, 0, 1 }}; matrix::transformMat4(pos, pos, matrix); - return { static_cast(pos[0] / pos[3]), static_cast(pos[1] / pos[3]) }; + return {{ static_cast(pos[0] / pos[3]), static_cast(pos[1] / pos[3]) }, pos[3] }; } float evaluateSizeForFeature(const ZoomEvaluatedSize& zoomEvaluatedSize, const PlacedSymbol& placedSymbol) { @@ -150,9 +152,20 @@ namespace mbgl { NotEnoughRoom, NeedsFlipping }; + + Point projectTruncatedLineSegment(const Point& previousTilePoint, const Point& currentTilePoint, const Point& previousProjectedPoint, const float minimumLength, const mat4& projectionMatrix) { + // We are assuming "previousTilePoint" won't project to a point within one unit of the camera plane + // If it did, that would mean our label extended all the way out from within the viewport to a (very distant) + // point near the plane of the camera. We wouldn't be able to render the label anyway once it crossed the + // plane of the camera. + const Point projectedUnitVertex = project(previousTilePoint + util::unit(previousTilePoint - currentTilePoint), projectionMatrix).first; + const Point projectedUnitSegment = previousProjectedPoint - projectedUnitVertex; + + return previousProjectedPoint + (projectedUnitSegment * (minimumLength / util::mag(projectedUnitSegment))); + } optional placeGlyphAlongLine(const float offsetX, const float lineOffsetX, const float lineOffsetY, const bool flip, - Point anchorPoint, const uint16_t anchorSegment, const GeometryCoordinates& line, const mat4& labelPlaneMatrix) { + const Point& projectedAnchorPoint, const Point& tileAnchorPoint, const uint16_t anchorSegment, const GeometryCoordinates& line, const mat4& labelPlaneMatrix) { const float combinedOffsetX = flip ? offsetX - lineOffsetX : @@ -172,8 +185,8 @@ namespace mbgl { int32_t currentIndex = dir > 0 ? anchorSegment : anchorSegment + 1; - Point current = anchorPoint; - Point prev = anchorPoint; + Point current = projectedAnchorPoint; + Point prev = projectedAnchorPoint; float distanceToPrev = 0.0; float currentSegmentDistance = 0.0; const float absOffsetX = std::abs(combinedOffsetX); @@ -185,7 +198,18 @@ namespace mbgl { if (currentIndex < 0 || currentIndex >= static_cast(line.size())) return {}; prev = current; - current = project(convertPoint(line.at(currentIndex)), labelPlaneMatrix); + PointAndCameraDistance projection = project(convertPoint(line.at(currentIndex)), labelPlaneMatrix); + if (projection.second > 0) { + current = projection.first; + } else { + // The vertex is behind the plane of the camera, so we can't project it + // Instead, we'll create a vertex along the line that's far enough to include the glyph + const Point previousTilePoint = distanceToPrev == 0 ? + tileAnchorPoint : + convertPoint(line.at(currentIndex - dir)); + const Point currentTilePoint = convertPoint(line.at(currentIndex)); + current = projectTruncatedLineSegment(previousTilePoint, currentTilePoint, prev, absOffsetX - distanceToPrev + 1, labelPlaneMatrix); + } distanceToPrev += currentSegmentDistance; currentSegmentDistance = util::dist(prev, current); @@ -211,29 +235,28 @@ namespace mbgl { const mat4& posMatrix, const mat4& labelPlaneMatrix, const mat4& glCoordMatrix, - gl::VertexVector& dynamicVertexArray) { + gl::VertexVector& dynamicVertexArray, + const Point& projectedAnchorPoint) { const float fontScale = fontSize / 24.0; const float lineOffsetX = symbol.lineOffset[0] * fontSize; const float lineOffsetY = symbol.lineOffset[1] * fontSize; - const Point anchorPoint = project(symbol.anchorPoint, labelPlaneMatrix); - std::vector placedGlyphs; if (symbol.glyphOffsets.size() > 1) { const float firstGlyphOffset = symbol.glyphOffsets.front(); const float lastGlyphOffset = symbol.glyphOffsets.back(); - optional firstPlacedGlyph = placeGlyphAlongLine(fontScale * firstGlyphOffset, lineOffsetX, lineOffsetY, flip, anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); + optional firstPlacedGlyph = placeGlyphAlongLine(fontScale * firstGlyphOffset, lineOffsetX, lineOffsetY, flip, projectedAnchorPoint, symbol.anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); if (!firstPlacedGlyph) return PlacementResult::NotEnoughRoom; - optional lastPlacedGlyph = placeGlyphAlongLine(fontScale * lastGlyphOffset, lineOffsetX, lineOffsetY, flip, anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); + optional lastPlacedGlyph = placeGlyphAlongLine(fontScale * lastGlyphOffset, lineOffsetX, lineOffsetY, flip, projectedAnchorPoint, symbol.anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); if (!lastPlacedGlyph) return PlacementResult::NotEnoughRoom; - const Point firstPoint = project(firstPlacedGlyph->point, glCoordMatrix); - const Point lastPoint = project(lastPlacedGlyph->point, glCoordMatrix); + const Point firstPoint = project(firstPlacedGlyph->point, glCoordMatrix).first; + const Point lastPoint = project(lastPlacedGlyph->point, glCoordMatrix).first; if (keepUpright && !flip && (symbol.useVerticalMode ? firstPoint.y < lastPoint.y : firstPoint.x > lastPoint.x)) { @@ -244,7 +267,7 @@ namespace mbgl { for (size_t glyphIndex = 1; glyphIndex < symbol.glyphOffsets.size() - 1; glyphIndex++) { const float glyphOffsetX = symbol.glyphOffsets[glyphIndex]; // Since first and last glyph fit on the line, we're sure that the rest of the glyphs can be placed - auto placedGlyph = placeGlyphAlongLine(glyphOffsetX * fontScale, lineOffsetX, lineOffsetY, flip, anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); + auto placedGlyph = placeGlyphAlongLine(glyphOffsetX * fontScale, lineOffsetX, lineOffsetY, flip, projectedAnchorPoint, symbol.anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); placedGlyphs.push_back(*placedGlyph); } placedGlyphs.push_back(*lastPlacedGlyph); @@ -252,15 +275,23 @@ namespace mbgl { // Only a single glyph to place // So, determine whether to flip based on projected angle of the line segment it's on if (keepUpright && !flip) { - const Point a = project(convertPoint(symbol.line.at(symbol.segment)), posMatrix); - const Point b = project(convertPoint(symbol.line.at(symbol.segment + 1)), posMatrix); + const Point a = project(symbol.anchorPoint, posMatrix).first; + const Point tileSegmentEnd = convertPoint(symbol.line.at(symbol.segment + 1)); + const PointAndCameraDistance projectedVertex = project(tileSegmentEnd, posMatrix); + // We know the anchor will be in the viewport, but the end of the line segment may be + // behind the plane of the camera, in which case we can use a point at any arbitrary (closer) + // point on the segment. + const Point b = (projectedVertex.second > 0) ? + projectedVertex.first : + projectTruncatedLineSegment(symbol.anchorPoint,tileSegmentEnd, a, 1, posMatrix); + if (symbol.useVerticalMode ? b.y > a.y : b.x < a.x) { return PlacementResult::NeedsFlipping; } } assert(symbol.glyphOffsets.size() == 1); // We are relying on SymbolInstance.hasText filtering out symbols without any glyphs at all const float glyphOffsetX = symbol.glyphOffsets.front(); - optional singleGlyph = placeGlyphAlongLine(fontScale * glyphOffsetX, lineOffsetX, lineOffsetY, flip, anchorPoint, symbol.segment, + optional singleGlyph = placeGlyphAlongLine(fontScale * glyphOffsetX, lineOffsetX, lineOffsetY, flip, projectedAnchorPoint, symbol.anchorPoint, symbol.segment, symbol.line, labelPlaneMatrix); if (!singleGlyph) return PlacementResult::NotEnoughRoom; @@ -275,6 +306,7 @@ namespace mbgl { return PlacementResult::OK; } + void reprojectLineLabels(gl::VertexVector& dynamicVertexArray, const std::vector& placedSymbols, const mat4& posMatrix, const style::SymbolPropertyValues& values, const RenderTile& tile, const SymbolSizeBinder& sizeBinder, const TransformState& state, const FrameHistory& frameHistory) { @@ -311,12 +343,14 @@ namespace mbgl { const float pitchScaledFontSize = values.pitchAlignment == style::AlignmentType::Map ? fontSize * perspectiveRatio : fontSize / perspectiveRatio; + + const Point anchorPoint = project(placedSymbol.anchorPoint, labelPlaneMatrix).first; - PlacementResult placeUnflipped = placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, false /*unflipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray); + PlacementResult placeUnflipped = placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, false /*unflipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray, anchorPoint); if (placeUnflipped == PlacementResult::NotEnoughRoom || (placeUnflipped == PlacementResult::NeedsFlipping && - placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, true /*flipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray) == PlacementResult::NotEnoughRoom)) { + placeGlyphsAlongLine(placedSymbol, pitchScaledFontSize, true /*flipped*/, values.keepUpright, posMatrix, labelPlaneMatrix, glCoordMatrix, dynamicVertexArray, anchorPoint) == PlacementResult::NotEnoughRoom)) { hideGlyphs(placedSymbol.glyphOffsets.size(), dynamicVertexArray); } } -- cgit v1.2.1