mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.36k stars 1.33k forks source link

5.5.0 is not respecting line-dasharray #15930

Closed pappalar closed 4 years ago

pappalar commented 4 years ago

line-dasharray stopped working in Mapbox SDK 5.5.0.

https://docs.mapbox.com/mapbox-gl-js/style-spec/#paint-line-line-dasharray

example layer:

{
      "id": "my-id",
      "paint": {
        "line-color": "rgba(255, 255, 255, 255)",
        "line-dasharray": [
          0.006666667,
          1.3333334
        ],
        "line-width": 9
      },
      "layout": {
        "line-cap": "round",
        "line-join": "round",
        "visibility": "none"
      },
      "source": "mysource",
      "type": "line"
    },

Steps to reproduce

  1. Draw a line with dasharray in 5.4.0. The line is dotted
  2. Update SDK to 5.5.0
  3. Draw the same line: The line is not dotted anymore

Expected behavior

IMG_3895

Actual behavior

IMG_3894

Confguration

Mapbox SDK versions: 5.5.0

pappalar commented 4 years ago

My supposition is that #15756 might be breaking it?

pappalar commented 4 years ago

@julianrex same for this issue. Please let me know if I am opening issues in the wrong repo and I will move them to the main one! Thanks for the explanation

pappalar commented 4 years ago

Hello! Upping this because is business critical for me, but I can't rollback to 5.4.0 due to the other issues with insets in that version.

julianrex commented 4 years ago

@racer1988 moving this ticket to the gl-native repo as it appears to be a core rendering issue.

/cc @tmpsantos

pappalar commented 4 years ago

More investigation shows that:

It works in 5.4.0 it works on Android latest release.

so it's a 5.5.0 issue.

julianrex commented 4 years ago

Thanks for the clarification @racer1988. 🤔

tmpsantos commented 4 years ago

@racer1988 thanks for reporting and bisecting the issue. We will have a look monday. @julianrex thanks for flagging. I'm surprised our render tests have not caught this regression. :-(

astojilj commented 4 years ago

@racer1988 , Could you please share the full layer style to be used to reproduce the same as on screenshots?

{
      "id": "my-id",
      "paint": {
        "line-color": "rgba(255, 255, 255, 255)",
        "line-dasharray": [
          0.006666667,
          1.3333334
        ],
        "line-width": 9
      },
      "layout": {
        "line-cap": "round",
        "line-join": "round",
        "visibility": "none"
      },
      "source": "mysource",
      "type": "line"
    },

Apart from "visibility": "none" I don't see where the blue color is coming from. Some more layer code would help. Similar tests that we have in automatic render tests don't show the problem.

15756

Did you identify some specific issue in the patch or bisecting showed that it is the one causing the regression?

pappalar commented 4 years ago

@astojilj for number #15756 I just guessed. It seems to be the only change in the changelod between version 5.4.0 and 5.5.0 that changes the rendered/style part of the core.

I haven't seen any other issue/change talking about rendered/style, so I suggested that might be a good place to start bisecting the regression.


Here is the style we use to draw either the normal line and the dashed line with white border and blue content.

The same style is used across iOS and Android and works until iOS version 5.4.0 (and still works on Android)

{
      "id": "white-border-normal",
      "paint": {
        "line-color": "rgba(255, 255, 255, 255)",
        "line-width": 9,
        "line-opacity": {
          "stops": [
            [
              14,
              1
            ],
            [
              15,
              0.6
            ]
          ]
        }
      },
      "layout": {
        "line-cap": "round",
        "line-join": "round",
        "visibility": "none"
      },
      "source": "theline",
      "type": "line",
      "filter": [
        "==",
        [
          "get",
          "custom_line_type"
        ],
        "normal"
      ]
    },

    {
      "id": "blue-normal",
      "paint": {
        "line-color": [
          "case",
          [
            "to-boolean",
            [
              "get",
              "selected"
            ]
          ],
          [
            "rgba",
            0,
            64,
            148,
            1
          ],
          [
            "rgba",
            0,
            88,
            202,
            1
          ]
        ],
        "line-width": 6,
        "line-opacity": {
          "stops": [
            [
              14,
              1
            ],
            [
              15,
              0.6
            ]
          ]
        }
      },
      "layout": {
        "line-cap": "round",
        "line-join": "round",
        "visibility": "none"
      },
      "source": "theline",
      "type": "line",
      "filter": [
        "==",
        [
          "get",
          "custom_line_type"
        ],
        "normal"
      ]
    },
    {
      "id": "dashed-border",
      "paint": {
        "line-color": "rgba(255, 255, 255, 255)",
        "line-dasharray": [
          0.006666667,
          1.3333334
        ],
        "line-width": 9,
        "line-opacity": {
          "stops": [
            [
              14,
              1
            ],
            [
              15,
              0.6
            ]
          ]
        }
      },
      "layout": {
        "line-cap": "round",
        "line-join": "round",
        "visibility": "none"
      },
      "source": "theline",
      "type": "line",
      "filter": [
        "==",
        [
          "get",
          "custom_line_type"
        ],
        "dashed"
      ]
    },
    {
      "id": "dashed",
      "paint": {
        "line-color": [
          "case",
          [
            "to-boolean",
            [
              "get",
              "selected"
            ]
          ],
          [
            "rgba",
            0,
            64,
            148,
            1
          ],
          [
            "rgba",
            0,
            88,
            202,
            1
          ]
        ],
        "line-dasharray": [
          0.01,
          2
        ],
        "line-width": 6,
        "line-opacity": {
          "stops": [
            [
              14,
              1
            ],
            [
              15,
              0.6
            ]
          ]
        }
      },
      "layout": {
        "line-cap": "round",
        "line-join": "round",
        "visibility": "none"
      },
      "source": "theline",
      "type": "line",
      "filter": [
        "==",
        [
          "get",
          "custom_line_type"
        ],
        "dashed"
      ]
    },
astojilj commented 4 years ago

@racer1988 The issue seems to be related to wrong evaluation of this code - custom_line_type is apparently evaluated to normal. How is it set in your code?

[
        "==",
        [
          "get",
          "custom_line_type"
        ],
        "dashed"
      ]
pappalar commented 4 years ago

@astojilj there is another similar example in https://github.com/mapbox/mapbox-gl-native-ios/issues/62 that does not use the custom parameter, so that might give you another clue in where the issue might be

as this was working until 5.4.0, I don't believe this is a issue with my style.

To set this I create a MGLPolilyneFeature and then I set the attribute

attributes["custom_line_type"] = "dashed"

before assigning the shape to the source and enabling the layers

astojilj commented 4 years ago

We don't have render tests combining round line-cap and line-dasharray and this wasn't caught by tests.

Related to https://github.com/mapbox/mapbox-gl-native/commit/84f2ff1084ccaec68c7a26243367ae657f6ebe60

Reproducible using iOS Demo App, using patch:

diff --git a/platform/ios/app/MBXViewController.m b/platform/ios/app/MBXViewController.m
index 82a68e074..667c27149 100644
--- a/platform/ios/app/MBXViewController.m
+++ b/platform/ios/app/MBXViewController.m
@@ -1484,6 +1484,9 @@ - (void)styleRouteLine
     baseRouteLayer.lineOpacity = [NSExpression expressionForConstantValue:@0.5];
     baseRouteLayer.lineCap = [NSExpression expressionForConstantValue:@"round"];
     baseRouteLayer.lineJoin = [NSExpression expressionForConstantValue:@"round"];
+    NSExpression *constantExpression = [NSExpression expressionWithFormat:@"{4, 4}"];
+    baseRouteLayer.lineDashPattern = constantExpression;
+
     [self.mapView.style addLayer:baseRouteLayer];

     MGLLineStyleLayer *routeLayer = [[MGLLineStyleLayer alloc] initWithIdentifier:@"style-route-layer" source:routeSource];
@@ -1492,6 +1495,7 @@ - (void)styleRouteLine
     routeLayer.lineOpacity = [NSExpression expressionForConstantValue:@0.8];
     routeLayer.lineCap = [NSExpression expressionForConstantValue:@"round"];
     routeLayer.lineJoin = [NSExpression expressionForConstantValue:@"round"];
+    routeLayer.lineDashPattern = constantExpression;
     [self.mapView.style addLayer:routeLayer];
 }
kkaefer commented 4 years ago

Fix is in https://github.com/mapbox/mapbox-gl-native/pull/15947

pappalar commented 4 years ago

@julianrex How will the GL-native fixes end up in iOS SDK now?

Will this be in 5.6.0?

julianrex commented 4 years ago

@racer1988 Yes it is!

/cc @jmkiley