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

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

gabbopalma
Copy link
Contributor

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
Copy link
Collaborator

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

gabbopalma commented Jul 26, 2024

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

@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.

…n Android.

Minor fix on iOS template and generate.dart.
@gabbopalma
Copy link
Contributor Author

Hi guys, any update on this?

Copy link

@Zverik Zverik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally seems ok to me, and solves a bug in theory.

@@ -65,7 +74,16 @@ static PropertyValue[] interpretSymbolLayerProperties(Object o) {
properties.add(PropertyFactory.textHaloBlur(expression));
break;
case "text-translate":
properties.add(PropertyFactory.textTranslate(expression));
if (jsonElement.isJsonArray()) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put all this logic into convertJsonToFloatArray to avoid repetition?..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't remember why I did this, it was a while ago. Maybe because in some cases the conversion was handled differently, but currently it seems logical to put everything in convertJsonToFloatArray.

@josxha josxha added this to the v0.20.1 milestone Oct 3, 2024
@josxha josxha enabled auto-merge (squash) December 31, 2024 12:47
danieljosua1 and others added 2 commits February 20, 2025 23:34
Hello,
I am currently developing a bike navigation app for a client
(RadroutenPlaner Bayern). After publishing the first version of the app,
we noticed that the location accuracy on Android was significantly poor
when switching to a Following Mode. The location puck updates very
slowly and with a noticeable delay, it felt like it was only using
coarse location.

Upon investigating, I discovered that MapLibre Android uses the
FusedLocationProvider by default, which only accepts a balanced location
priority. Observing other major navigation apps like Komoot and
Outdooractive via logcat, I found that they switch from the native fused
provider to the GPS provider when the device enters Following Mode.

To address this issue, I added the ability to manipulate the native
LocationEngine and LocationEngineRequest in flutter_maplibre. I
introduced a property to the MaplibreMap widget called
locationEnginePlatforms. This property currently allows you to adjust
settings like interval, displacement, and priority (only on Android).

I don't have extensive experience with Java and Kotlin, so please let me
know if there are any issues or if improvements are needed. Thanks for
your feedback!
This pull request includes several changes to the Android project setup
and dependencies, as well as some code refactoring in the
`GlobalMethodHandler` class. The most important changes are grouped by
theme and summarized below:

### Android Project Setup:
Upgrade to latest Flutter Gradle plugins configuration:

https://docs.flutter.dev/release/breaking-changes/flutter-gradle-plugin-apply
### Dependency Updates:

*
[`example/pubspec.yaml`](diffhunk://#diff-565b0869896732da3b937c64bc8fd5f0ca37ea1f629d579c29fc70e0f1e3e48eL20-R20):
Updated the `path_provider` dependency version from `2.0.15` to `2.1.5`.
* Set minimum supported Flutter version to 3.22.0

### Code Refactoring:

*
[`maplibre_gl/android/src/main/java/org/maplibre/maplibregl/GlobalMethodHandler.java`](diffhunk://#diff-9c836abc6c8048cf093a291198c86b17f16530e99328202318c54f9fe1c65c40L29-L37):
Removed the `registrar` field and related code, updating the constructor
and `openTilesDbFile` method to use `flutterAssets` instead.
[[1]](diffhunk://#diff-9c836abc6c8048cf093a291198c86b17f16530e99328202318c54f9fe1c65c40L29-L37)
[[2]](diffhunk://#diff-9c836abc6c8048cf093a291198c86b17f16530e99328202318c54f9fe1c65c40L155-R149)

---------

Co-authored-by: thanhdt-vietmap <[email protected]>
Co-authored-by: Joscha <[email protected]>
auto-merge was automatically disabled February 20, 2025 22:35

Head branch was pushed to by a user without write access

@josxha josxha removed this from the v0.21.0 milestone Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants