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

supporting fill-extrusion #211

Closed krupupakku closed 11 months ago

krupupakku commented 1 year ago

Pull Request

Description

Hey! I needed to use extrusions in my project and I thought maybe you want to add support to them.

This is the result in example Layer page, adding controller.addFillExtrusionLayer:

mariusvn commented 1 year ago

damn, did not expect that, would be a very nice improvent

mariusvn commented 1 year ago

we need to wait until @m0nac0 gives you the approval for the ci process to run, then it will be reviewed and tested

krupupakku commented 1 year ago

I saw it was running and the linter failed, I was looking for a log eheh

mariusvn commented 1 year ago

to fix the formatting, we follow the official guidlines so just flutter format . in your root directory and it should do the trick.

For info, your latest ci report is here if i'm not wrong https://github.com/m0nac0/flutter-maplibre-gl/actions/runs/4429707859.

thanks for the work !

mariusvn commented 1 year ago

I see you added a test for it on the example app, very nice too

krupupakku commented 1 year ago

to fix the formatting, we follow the official guidlines so just flutter format . in your root directory and it should do the trick.

For info, your latest ci report is here if i'm not wrong https://github.com/m0nac0/flutter-maplibre-gl/actions/runs/4429707859.

thanks for the work !

Formatting done! thanks for the suggestion, it worked perfectly

m0nac0 commented 1 year ago

@mariusvn Did you review and test this?

krupupakku commented 11 months ago

Hey! how is it going? do you have some time to review this? I believe I need to update the branch as it has some conflicts now.. :P

m0nac0 commented 11 months ago

Hey, sorry we missed your PR! If you can resolve the conflicts I'll be happy to review it. If you have trouble resolving the conflicts, let me know and I can take a look.

krupupakku commented 11 months ago

Hey, sorry we missed your PR! If you can resolve the conflicts I'll be happy to review it. If you have trouble resolving the conflicts, let me know and I can take a look.

Ok I'll resolve conflicts today ;) Thanks a lot

m0nac0 commented 11 months ago

@krupupakku If you want to take a look at the changes from my 3 commits together (excluding whitespace changes), you can do that here https://github.com/maplibre/flutter-maplibre-gl/pull/211/files/ff22c991fb02ffb2dc986bd0c2ac6fe24d126dbe..a6f86eb6d7a3586e7088959867af2f04e6d07eb7?diff=unified&w=1

Let me know if that looks good from your side.

krupupakku commented 11 months ago

@krupupakku If you want to take a look at the changes from my 3 commits together (excluding whitespace changes), you can do that here https://github.com/maplibre/flutter-maplibre-gl/pull/211/files/ff22c991fb02ffb2dc986bd0c2ac6fe24d126dbe..a6f86eb6d7a3586e7088959867af2f04e6d07eb7?diff=unified&w=1

Let me know if that looks good from your side.

Oh marvellous! @m0nac0 thanks for taking care of if! LGTM!

krupupakku commented 11 months ago

@krupupakku If you want to take a look at the changes from my 3 commits together (excluding whitespace changes), you can do that here https://github.com/maplibre/flutter-maplibre-gl/pull/211/files/ff22c991fb02ffb2dc986bd0c2ac6fe24d126dbe..a6f86eb6d7a3586e7088959867af2f04e6d07eb7?diff=unified&w=1 Let me know if that looks good from your side.

Oh marvellous! @m0nac0 thanks for taking care of if! LGTM!

@m0nac0 I also confirm you that the script now works (before it was giving me an error trying running it), and looks wonderful!

Robbendebiene commented 11 months ago

Thanks for implementing this! Unfortunately I get the following error (using Android emulator):


E/flutter ( 7457): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: PlatformException(error, null, null, java.lang.NullPointerException
E/flutter ( 7457):  at com.mapbox.mapboxsdk.style.layers.Layer.nativeSetPaintProperty(Native Method)
E/flutter ( 7457):  at com.mapbox.mapboxsdk.style.layers.Layer.setProperties(Layer.java:60)
E/flutter ( 7457):  at com.mapbox.mapboxgl.MapboxMapController.addFillExtrusionLayer(MapboxMapController.java:524)
E/flutter ( 7457):  at com.mapbox.mapboxgl.MapboxMapController.onMethodCall(MapboxMapController.java:1090)
E/flutter ( 7457):  at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler.onMessage(MethodChannel.java:258)
E/flutter ( 7457):  at io.flutter.embedding.engine.dart.DartMessenger.invokeHandler(DartMessenger.java:295)
E/flutter ( 7457):  at io.flutter.embedding.engine.dart.DartMessenger.lambda$dispatchMessageToQueue$0$io-flutter-embedding-engine-dart-DartMessenger(DartMessenger.java:322)
E/flutter ( 7457):  at io.flutter.embedding.engine.dart.DartMessenger$$ExternalSyntheticLambda0.run(Unknown Source:12)
E/flutter ( 7457):  at android.os.Handler.handleCallback(Handler.java:883)
E/flutter ( 7457):  at android.os.Handler.dispatchMessage(Handler.java:100)
E/flutter ( 7457):  at android.os.Looper.loop(Looper.java:214)
E/flutter ( 7457):  at android.app.ActivityThread.main(ActivityThread.java:7356)
E/flutter ( 7457):  at java.lang.reflect.Method.invoke(Native Method)
E/flutter ( 7457):  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
E/flutter ( 7457):  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)
E/flutter ( 7457): )
E/flutter ( 7457): #0      StandardMethodCodec.decodeEnvelope (package:flutter/src/services/message_codecs.dart:652:7)
E/flutter ( 7457): #1      MethodChannel._invokeMethod (package:flutter/src/services/platform_channel.dart:310:18)
E/flutter ( 7457): <asynchronous suspension>
E/flutter ( 7457): #2      MethodChannelMaplibreGl.addFillExtrusionLayer (package:maplibre_gl_platform_interface/src/method_channel_maplibre_gl.dart:709:5)
E/flutter ( 7457): <asynchronous suspension>
E/flutter ( 7457): #3      MaplibreMapController.addFillExtrusionLayer (package:maplibre_gl/src/controller.dart:520:5)
E/flutter ( 7457): <asynchronous suspension>
E/flutter ( 7457):
krupupakku commented 11 months ago

Thanks for implementing this! Unfortunately I get the following error (using Android emulator):


E/flutter ( 7457): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: PlatformException(error, null, null, java.lang.NullPointerException
E/flutter ( 7457):    at com.mapbox.mapboxsdk.style.layers.Layer.nativeSetPaintProperty(Native Method)
E/flutter ( 7457):    at com.mapbox.mapboxsdk.style.layers.Layer.setProperties(Layer.java:60)
E/flutter ( 7457):    at com.mapbox.mapboxgl.MapboxMapController.addFillExtrusionLayer(MapboxMapController.java:524)
E/flutter ( 7457):    at com.mapbox.mapboxgl.MapboxMapController.onMethodCall(MapboxMapController.java:1090)
E/flutter ( 7457):    at io.flutter.plugin.common.MethodChannel$IncomingMethodCallHandler.onMessage(MethodChannel.java:258)
E/flutter ( 7457):    at io.flutter.embedding.engine.dart.DartMessenger.invokeHandler(DartMessenger.java:295)
E/flutter ( 7457):    at io.flutter.embedding.engine.dart.DartMessenger.lambda$dispatchMessageToQueue$0$io-flutter-embedding-engine-dart-DartMessenger(DartMessenger.java:322)
E/flutter ( 7457):    at io.flutter.embedding.engine.dart.DartMessenger$$ExternalSyntheticLambda0.run(Unknown Source:12)
E/flutter ( 7457):    at android.os.Handler.handleCallback(Handler.java:883)
E/flutter ( 7457):    at android.os.Handler.dispatchMessage(Handler.java:100)
E/flutter ( 7457):    at android.os.Looper.loop(Looper.java:214)
E/flutter ( 7457):    at android.app.ActivityThread.main(ActivityThread.java:7356)
E/flutter ( 7457):    at java.lang.reflect.Method.invoke(Native Method)
E/flutter ( 7457):    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
E/flutter ( 7457):    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)
E/flutter ( 7457): )
E/flutter ( 7457): #0      StandardMethodCodec.decodeEnvelope (package:flutter/src/services/message_codecs.dart:652:7)
E/flutter ( 7457): #1      MethodChannel._invokeMethod (package:flutter/src/services/platform_channel.dart:310:18)
E/flutter ( 7457): <asynchronous suspension>
E/flutter ( 7457): #2      MethodChannelMaplibreGl.addFillExtrusionLayer (package:maplibre_gl_platform_interface/src/method_channel_maplibre_gl.dart:709:5)
E/flutter ( 7457): <asynchronous suspension>
E/flutter ( 7457): #3      MaplibreMapController.addFillExtrusionLayer (package:maplibre_gl/src/controller.dart:520:5)
E/flutter ( 7457): <asynchronous suspension>
E/flutter ( 7457):

🤔 super strange, I'm using it in my project and it's working in both android and iOS (i tried in simulator and physical device).

m0nac0 commented 11 months ago

@Robbendebiene Could you share your Flutter code for adding the fill extrusion layer?

Robbendebiene commented 11 months ago

I'm sorry for the confusion.

For testing I just copied a style that worked when loaded through URL without checking whether it uses the old or new (expression) syntax.

Changing this:

'fill-extrusion-base': {
  'type':'identity',
  'property':'render_min_height'
},

to

'fill-extrusion-base': ['get', 'render_min_height'],

solves the problem.

One thing I noticed when trying to create a minimal reproducible example is that the addLayer method doesn't forward the filter property for FillExtrusionLayerProperties: https://github.com/maplibre/flutter-maplibre-gl/blob/dc21739b9d4395b7df7f181e96d678ab0383294d/lib/src/controller.dart#L1314-L1318

m0nac0 commented 11 months ago

One thing I noticed when trying to create a minimal reproducible example is that the addLayer method doesn't forward the filter property for FillExtrusionLayerProperties:

Good catch! @krupupakku Was that intentional? If not, no worries, we can open an issue to add that.

krupupakku commented 11 months ago

One thing I noticed when trying to create a minimal reproducible example is that the addLayer method doesn't forward the filter property for FillExtrusionLayerProperties:

Good catch! @krupupakku Was that intentional? If not, no worries, we can open an issue to add that.

Nope, must me an error from my side :P but it's very strange because I'm using filter in my project and it's working ahah

@m0nac0 let's open an issue as you said, I can take a better look later today or tomorrow ;) Thanks for noticing! @Robbendebiene