supporting fill-extrusion #211

krupupakku · 2023-03-15T18:25:16Z

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 · 2023-03-17T15:44:31Z

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

mariusvn · 2023-03-21T15:54:47Z

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

krupupakku · 2023-03-21T16:00:19Z

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

mariusvn · 2023-03-21T16:19:30Z

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 · 2023-03-21T16:21:21Z

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

krupupakku · 2023-03-21T16:28:30Z

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 · 2023-07-06T09:42:00Z

@mariusvn Did you review and test this?

krupupakku · 2023-11-29T17:03:19Z

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

lib/src/controller.dart

m0nac0 · 2023-12-04T09:32:47Z

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 · 2023-12-04T09:33:40Z

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 · 2023-12-04T09:39:08Z

android/src/main/java/com/mapbox/mapboxgl/LayerPropertyConverter.java

Just to confirm: were the modifications to this file, to ios/Classes/LayerPropertyConverter.swift and to lib/src/layer_properties.dart generated with the script?

Hem... nope. which script? I added them manually.

EDIT: I didn't know anything about a script, and I didn't find any reference to it. if you tell me more, I can generate these modifications in the right way, sure :P

oooh sorry I just reviewed it better and found it. scripts/lib/generate.dart
Let me check it better. back to you in a min

OK I confirm you I added them manually. if you tell me how you run the generate.dart I will do the same way to be sure I keep consistency, instead of add them manually

You should be able to run the script with dart run .\scripts\lib\generate.dart.

That seems to add a few outdated comments though (referencing Mapbox instead of MapLibre)

Since this was a bit more complicated than I initially thought, I

made some fixes to the script's input style

added a README for the script

ran the script and checked the output

pushed those changes

I hope that's ok for you.

Hm, the script wants to generate a parameter name of fill-extrusionLayer for the iOS code, which is invalid. I pushed a new commit that hopefully fixes that.

into pr/211

m0nac0 · 2023-12-06T10:54:18Z

@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 · 2023-12-09T10:53:32Z

@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 · 2023-12-09T10:57:53Z

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

m0nac0

Thank you again for you contribution!

Robbendebiene · 2023-12-11T14:23:58Z

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 · 2023-12-11T14:32:10Z

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 · 2023-12-11T16:11:45Z

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

Robbendebiene · 2023-12-13T09:22:54Z

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:

flutter-maplibre-gl/lib/src/controller.dart

Lines 1314 to 1318 in dc21739

    
           addFillExtrusionLayer(sourceId, layerId, properties, 
        
               belowLayerId: belowLayerId, 
        
               sourceLayer: sourceLayer, 
        
               minzoom: minzoom, 
        
               maxzoom: maxzoom);

m0nac0 · 2023-12-14T11:05:49Z

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 · 2023-12-14T11:11:24Z

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

mavol24 added 2 commits March 15, 2023 19:14

supporting fill-extrusion

24b3829

update readme

df0e375

Merge branch 'main' into main

e5f0296

formatting code

71ae630

m0nac0 and others added 2 commits April 4, 2023 13:29

Merge branch 'main' into main

5684f0e

Merge branch 'main' into main

e7984e5

m0nac0 reviewed Dec 4, 2023

View reviewed changes

lib/src/controller.dart Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/main' into pr/211

e347f69

m0nac0 reviewed Dec 4, 2023

View reviewed changes

mavol24 and others added 7 commits December 4, 2023 10:54

Merge remote-tracking branch 'upstream/main'

546f9c9

Update Controller.dart docs

ba20384

renaming mapbox ref to maplibre

03b3eeb

Merge branch 'main' of https://github.com/krupupakku/flutter-maplibre-gl

ff22c99

into pr/211

re-run the script (with some comment fixes to the input style)

f521edf

script iOS template: use camelCase for parameter name

65af376

fix static analysis by adding const modifier

a6f86eb

Merge branch 'main' into main

d17a32f

m0nac0 approved these changes Dec 9, 2023

View reviewed changes

m0nac0 merged commit dc21739 into maplibre:main Dec 9, 2023
6 checks passed

m0nac0 mentioned this pull request Dec 14, 2023

Investigate/fix filter property for FillExtrusionLayer #360

Open

josxha added this to the v0.19.0 milestone May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

supporting fill-extrusion #211

supporting fill-extrusion #211

krupupakku commented Mar 15, 2023

mariusvn commented Mar 17, 2023

mariusvn commented Mar 21, 2023

krupupakku commented Mar 21, 2023

mariusvn commented Mar 21, 2023

mariusvn commented Mar 21, 2023

krupupakku commented Mar 21, 2023

m0nac0 commented Jul 6, 2023

krupupakku commented Nov 29, 2023

m0nac0 commented Dec 4, 2023

krupupakku commented Dec 4, 2023

m0nac0 Dec 4, 2023

krupupakku Dec 4, 2023 •

edited

Loading

krupupakku Dec 4, 2023

krupupakku Dec 4, 2023

m0nac0 Dec 6, 2023

m0nac0 Dec 6, 2023

m0nac0 Dec 6, 2023

m0nac0 Dec 6, 2023

m0nac0 commented Dec 6, 2023

krupupakku commented Dec 9, 2023 •

edited

Loading

krupupakku commented Dec 9, 2023

m0nac0 left a comment

Robbendebiene commented Dec 11, 2023 •

edited

Loading

krupupakku commented Dec 11, 2023

m0nac0 commented Dec 11, 2023

Robbendebiene commented Dec 13, 2023

m0nac0 commented Dec 14, 2023

krupupakku commented Dec 14, 2023

supporting fill-extrusion #211

supporting fill-extrusion #211

Conversation

krupupakku commented Mar 15, 2023

Pull Request

Description

mariusvn commented Mar 17, 2023

mariusvn commented Mar 21, 2023

krupupakku commented Mar 21, 2023

mariusvn commented Mar 21, 2023

mariusvn commented Mar 21, 2023

krupupakku commented Mar 21, 2023

m0nac0 commented Jul 6, 2023

krupupakku commented Nov 29, 2023

m0nac0 commented Dec 4, 2023

krupupakku commented Dec 4, 2023

m0nac0 Dec 4, 2023

Choose a reason for hiding this comment

krupupakku Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

krupupakku Dec 4, 2023

Choose a reason for hiding this comment

krupupakku Dec 4, 2023

Choose a reason for hiding this comment

m0nac0 Dec 6, 2023

Choose a reason for hiding this comment

m0nac0 Dec 6, 2023

Choose a reason for hiding this comment

m0nac0 Dec 6, 2023

Choose a reason for hiding this comment

m0nac0 Dec 6, 2023

Choose a reason for hiding this comment

m0nac0 commented Dec 6, 2023

krupupakku commented Dec 9, 2023 • edited Loading

krupupakku commented Dec 9, 2023

m0nac0 left a comment

Choose a reason for hiding this comment

Robbendebiene commented Dec 11, 2023 • edited Loading

krupupakku commented Dec 11, 2023

m0nac0 commented Dec 11, 2023

Robbendebiene commented Dec 13, 2023

m0nac0 commented Dec 14, 2023

krupupakku commented Dec 14, 2023

krupupakku Dec 4, 2023 •

edited

Loading

krupupakku commented Dec 9, 2023 •

edited

Loading

Robbendebiene commented Dec 11, 2023 •

edited

Loading