-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add style image missing binding integration #14320
Conversation
b98f4e9
to
a55a121
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, just a couple of questions and nitpicks.
...rm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapChangeReceiver.java
Outdated
Show resolved
Hide resolved
...rm/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapChangeReceiver.java
Outdated
Show resolved
Hide resolved
platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
Outdated
Show resolved
Hide resolved
platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
Outdated
Show resolved
Hide resolved
...AndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/ImageMissingTest.kt
Outdated
Show resolved
Hide resolved
...DKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/style/SymbolLayerActivity.java
Outdated
Show resolved
Hide resolved
a55a121
to
0ff5913
Compare
To resolve multiple callback invocations, I looked into profiling the usefulness of back grounding the image conversions. Test were performed with simple map activity and adding 20 images to the style. The profiler for CPU and memory didn't really show an actionable difference. The execution time of non async addition of images was faster for the map to become idle. The GPU profiler does show a small difference, you can see that the third bar in the images below is higher for the async variant as there is overhead created by the 20 asynctasks. When using the Atm, not seeing an actual evidence that the background conversion is actually usefull. |
🤔 What device did you use for testing? High-end devices might disguise the slowdown. |
0ff5913
to
bea7f0e
Compare
Capturing from chat, moving forward with both exposing synchronous as asynchronous addImage overloads on Style. With this in place no additional bookkeeping is required to use the image missing callback. |
bea7f0e
to
b3742cd
Compare
platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java
Outdated
Show resolved
Hide resolved
...AndroidSDKTestApp/src/androidTest/java/com/mapbox/mapboxsdk/testapp/maps/ImageMissingTest.kt
Outdated
Show resolved
Hide resolved
image overloads to style
b3742cd
to
8735d3d
Compare
closes #14319, refs core integration in #14253 and iOS integration in #14302.