Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gabbopalma opened this issue Jul 24, 2024 · 3 comments · May be fixed by #481
Open

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

gabbopalma opened this issue Jul 24, 2024 · 3 comments · May be fixed by #481
Labels
bug Something isn't working
Milestone

Comments

@gabbopalma
Copy link
Contributor

gabbopalma commented Jul 24, 2024

Platforms

  • iOS
  • Android

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 gabbopalma added the bug Something isn't working label Jul 24, 2024
@gabbopalma
Copy link
Contributor Author

gabbopalma commented Jul 25, 2024

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
Copy link
Contributor Author

gabbopalma commented Jul 25, 2024

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 gabbopalma changed the title [BUG] Line Layer line-dasharray crashes the app on iOS. [BUG] LineLayerProperties line-dasharray crashes the app on iOS. Jul 25, 2024
@gabbopalma gabbopalma changed the title [BUG] LineLayerProperties line-dasharray crashes the app on iOS. [BUG] Offsets, translations, and other properties that accept arrays get crash on iOS. Jul 29, 2024
@gabbopalma
Copy link
Contributor Author

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

@josxha josxha added this to the v0.20.1 milestone Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants