search

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
226 stars 125 forks source link

Fix: Update of generate.dart and its template files. Fixed Offset, Translate and expressions arrays on iOS. #481

Open gabbopalma opened 3 months ago

gabbopalma commented 3 months ago

Hi guys, this PR aims to solve the #480 issue that I opened recently.

I started by refactoring all template files, used in generate.dart, updating them because they were obsolete (there were still some mapbox references inside). Next, I fixed some problems with the paths in generate.dart, which were also obsolete.

Testing #480, I realized that the problem wasn't only with the “line-dasharray” property, but with all arrays that were part of a linear expression or that were arrays of doubles (generally numbers). I introduced some checks in LayerPropertyConverter.swift and now every array or expression, for every property (even on Offset and Translate, which work with the CGVector class), works correctly on the iOS platform.

kuhnroyal commented 3 months ago

Thanks for all the namespace/path fixes, I completely missed those in the generator. Do we need adjustments for #480 on the Android side as well or is everything working there?

gabbopalma commented 3 months ago

Thanks for all the namespace/path fixes, I completely missed those in the generator.

Do we need adjustments for #480 on the Android side as well or is everything working there?

@kuhnroyal On Android everything was fine for line-patharray, but it only accepts expressions and not arrays, when using an array a console error appears with "array not allowed". Other than that, I haven't checked the properties of offsets or translations, but I think everything is fine on Android, I might check on Monday.

Do we want to accept only expressions on iOS as well? Currently, with my edits, arrays are also allowed on iOS (I think this must be the correct way also on Android, for a better UX).

gabbopalma commented 3 months ago

@kuhnroyal I did some checking on Android, and well, the bug isn't only on iOS but also on Android. But on Android it doesn't cause a crash, just an error log:

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

These days I will fix the LayerPropertConverter.java, also applying some optimization fixes, in case I will update you with commits and comments here.

gabbopalma commented 2 months ago

Hi guys, any update on this?