Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Remove fragment flash #10717

Merged
merged 1 commit into from
Dec 19, 2017
Merged

Remove fragment flash #10717

merged 1 commit into from
Dec 19, 2017

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Dec 16, 2017

This PR optimises showing a Map inside a fragment by hiding the surface on startup and only making it visible when the map is ready. Additionally this PR also allows to use multiple subscribers for OnMapReady events.

before

ezgif com-video-to-gif 16

after

ezgif com-video-to-gif 17

@tobrun tobrun added the Android Mapbox Maps SDK for Android label Dec 16, 2017
@tobrun tobrun added this to the android-v5.2.2 milestone Dec 16, 2017
@tobrun tobrun self-assigned this Dec 16, 2017
Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

Maybe we should stick to a single OnMapReadyCallback to keep it consistent with by-hand initialization?


private final List<OnMapReadyCallback> mapReadyCallbackList = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we don't have an API with a list of listeners for OnMapReadyCallback when instantiating it by hand I don't know why should we include it here. Was that a requested feature?

Copy link
Member Author

@tobrun tobrun Dec 18, 2017

Choose a reason for hiding this comment

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

Should have added that to the OP, but #9923 is one. There is also this PR but it never got merged #4735. Additionally there are users that have complained about this when using RxJava, they call getMapAsync multiple times and later as the default lifecycle.

@Override
public void onMapReady(MapboxMap mapboxMap) {
this.mapboxMap = mapboxMap;
if (!mapReadyCallbackList.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check's redundant. Besides that, the same question as above about the list.

Copy link
Member Author

Choose a reason for hiding this comment

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

This check is not redundant, checking if a list is not empty before looping it will avoid creating an iterator. In code path that are called often this can make a difference. In this case though it doesn't result in any significant memory growth since it's only called once. I will remove it.

@tobrun tobrun force-pushed the tvn-remove-fragment-flash branch from 8b14456 to a82f959 Compare December 18, 2017 12:45
@tobrun tobrun merged commit d80e71b into release-agua Dec 19, 2017
@tobrun tobrun deleted the tvn-remove-fragment-flash branch December 19, 2017 05:39
@tobrun tobrun mentioned this pull request Dec 20, 2017
22 tasks
mappy-mobile pushed a commit to Mappy/mapbox-gl-native that referenced this pull request Dec 26, 2017
* mapbox_release_5.3: (25 commits)
  [android] - update changelog for 5.3.0
  [android] added map touch listeners api based on lists
  [android] Tweak TinySDF docs to better describe font-family behavior.
  Saving/restoring MyLocationViewSettings (mapbox#10746) (mapbox#10748)
  [android] Updated Spanish, Vietnamese translations
  Android SDK renaming (mapbox#10609)
  [android] - use default icon when compass icon fails to decode (mapbox#10694)
  [android] - remove startup flash from fragment, rework OnMapReady callback for multiple listeners (mapbox#10717)
  [android] Add Configuration hook for local ideograph font family and demo activity
  [android] Android implementation of local CJK glyph rendering  - Draws bold version of glyph if font stack contains string "bold"  - Not hooked up to global configuration yet
  [core] Hook LocalGlyphRasterizer "font family" configuration up to Renderer
  [core] Enable local glyph generation using TinySDF.  - Platform-specific LocalGlyphRasterizer is responsible for deciding which glyphs to rasterize locally and for implementing the rasterization.  - Default platform implementation doesn't locally generate any glyphs -> no behavior change  - Unit test uses StubLocalGlyphRasterizer, which returns a single fixed bitmap for all CJK glyphs  - Rename glyph_loader.test to glyph_manager.test
  [core] C++ port of TinySDF
  [ios, macos] Cleaned up base localization files
  [android] - update instrumented make target, move code style validation before building C++ code, replace code style checks with the wrapper code style check. (mapbox#10724)
  [android] - activate filesource when creating an offline region (mapbox#10718)
  [ios] update constraints when updating content inset
  [ios] Fix an Interface Builder crash for IBInspectable properties.
  [ios] Fix the content insets for the scale and compass view.
  Post camera listener invocations (mapbox#10690)
  ...

# Conflicts:
#	platform/android/MapboxGLAndroidSDK/gradle.properties
#	platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/http/HTTPRequest.java
#	platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapGestureDetector.java
#	platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapView.java
#	platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/MapboxMap.java
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants