maplibre / flutter-maplibre-gl

Customizable, performant and vendor-free vector and raster maps, flutter wrapper for maplibre-native and maplibre-gl-js (fork of flutter-mapbox-gl/maps)
https://pub.dev/packages/maplibre_gl
Other
228 stars 125 forks source link

[BUG] Offsets, translations, and other properties that accept arrays get crash on iOS. #480

Open gabbopalma opened 4 months ago

gabbopalma commented 4 months ago

Platforms

Version of flutter maplibre_gl

main branch

Bug Description

The property line-dasharray for Line layers doesn't correctly work on iOS Platform. When attempting to assign the property with an expression as a value, upon Line creation, the app crashes.

Actually, this issue is only on iOS, on Android correctly works. I don't know for web platform.

Steps to Reproduce

  1. Create a Line Layer using controller.addLineLayer.
  2. Use the property "lineDasharray" in the LineLayerProperties.
    • Assign this expression to the property:
      lineDasharray: [
       "literal",
       [4, 2],
      ]
  3. When the map attempts to add the Line, the app crashes.
    • On Android the app doesn't crash but an error log appears:

      {aplibre.example}[JNI]: Property setting error: icon-offset value must be an array of 2 numbers.

Expected Results

  1. Create a Line Layer using controller.addLineLayer.
  2. Use the property "lineDasharray" in the LineLayerProperties.
    • Assign this expression to the property:
      lineDasharray: [
       "literal",
       [4, 2],
      ]
  3. The controller correctly adds the Line and renders it.

Actual Results

image

Code Sample

await controller?.addLineLayer(
  "$collectionName-source",
  layerId,
  LineLayerProperties(
     lineColor: getRandomColor,
     lineWidth: 2.0,
     lineDasharray: [
        "literal",
        [4, 2],
     ],
  ),
  enableInteraction: true, 
);
gabbopalma commented 4 months ago

I tried to solve this problem by making a few attempts. I tried to refactor the expression value assignment, which was the cause of the crash, by making the following changes to the interpretExpression function in LayerPropertyConverter.swift

private class func interpretExpression(propertyName: String, expression: String) -> NSExpression? {
        let isColor = propertyName.contains("color");
        let isDashArray = propertyName.contains("dasharray");

        do {
            let json = try JSONSerialization.jsonObject(with: expression.data(using: .utf8)!, options: .fragmentsAllowed)
            // this is required because NSExpression.init(mglJSONObject: json) fails to create
            // a proper Expression if the data of is a hexString
            if isColor {
                if let color = json as? String {
                    return NSExpression(forConstantValue: UIColor(hexString: color))
                }
            }
            // this is required because NSExpression.init(mglJSONObject: json) fails to create
            // a proper Expression if the data of a literal is an array
            if let offset = json as? [Any]{
                if offset.count == 2 && offset.first is String && offset.first as? String == "literal" {
                    if let vector = offset.last as? [Any]{
                        if(vector.count == 2) {
                            if let x = vector.first as? Double, let y = vector.last as? Double {
                                if isDashArray {
                                    return NSExpression(forConstantValue: [NSNumber(value: x), NSNumber(value: y)])
                                }
                                return NSExpression(forConstantValue: NSValue(cgVector: CGVector(dx: x, dy: y)))
                            }

                        }
                    }
                }
            }
            return NSExpression.init(mglJSONObject: json)
        } catch {
        }
        return nil
    }

There are two changes I made:

  1. Add the special case for the line-dasharray property, checking if the current property converts to an expression with the boolean isDashArray;
  2. If isDashArray, it returns an NSExpression formed by an array of two NSNumbers, with x as 0 and y as 1.

Obviously, these are just attempts and I know that this isn't the correct and generic solution, but now at least we know that the assignment of NSExpression is the cause of the crash (thus the CGVector in NSValue). If anyone has any suggestions to modify this solution, it is welcome.

gabbopalma commented 4 months ago

As I investigated the problem further, I found that this was useful for the “Offset” and “Translation” properties because of the CGVector used for two-dimensional structures. Now, I think the best solution is to enclose fields that have “Offset” or “Translation” in the property name in a condition and use the CGVector expression for these cases. For other cases, use the "return NSExpression.init(mglJSONObject: json)" instead.

Let me know what you think about it :)

gabbopalma commented 3 months ago

The bug evolves with new findings: Offsets, translations, and other properties that accept arrays get crashed on iOS.