From 85ec3eb0a5739c4f7b160d82715d3be51e10a02b Mon Sep 17 00:00:00 2001 From: Leith Bade Date: Thu, 24 Sep 2015 17:18:30 +1000 Subject: [PATCH] Refactor Android render loop to limit frame rate to 60 FPS Move expensive cache size calulation to initialisation Fix setGestureInProgress bug Renamed several functions to match current usage Refactor map updates to reduce GPS marker lag Fixes #1676 Fixes #2290 Fixes #2396 --- android/cpp/jni.cpp | 8 +- android/cpp/native_map_view.cpp | 33 ++--- .../com/mapbox/mapboxgl/views/MapView.java | 139 ++++++++++-------- .../mapbox/mapboxgl/views/NativeMapView.java | 6 +- include/mbgl/android/native_map_view.hpp | 4 +- 5 files changed, 98 insertions(+), 92 deletions(-) diff --git a/android/cpp/jni.cpp b/android/cpp/jni.cpp index 5ed0a730c..dcf64eed2 100644 --- a/android/cpp/jni.cpp +++ b/android/cpp/jni.cpp @@ -470,11 +470,11 @@ void JNICALL nativeUpdate(JNIEnv *env, jobject obj, jlong nativeMapViewPtr) { nativeMapView->getMap().update(mbgl::Update::Repaint); } -void JNICALL nativeOnInvalidate(JNIEnv *env, jobject obj, jlong nativeMapViewPtr) { - mbgl::Log::Debug(mbgl::Event::JNI, "nativeOnInvalidate"); +void JNICALL nativeRenderSync(JNIEnv *env, jobject obj, jlong nativeMapViewPtr) { + mbgl::Log::Debug(mbgl::Event::JNI, "nativeRenderSync"); assert(nativeMapViewPtr != 0); NativeMapView *nativeMapView = reinterpret_cast(nativeMapViewPtr); - nativeMapView->onInvalidate(); + nativeMapView->renderSync(); } void JNICALL nativeViewResize(JNIEnv *env, jobject obj, jlong nativeMapViewPtr, jint width, jint height) { @@ -1625,7 +1625,7 @@ extern "C" JNIEXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void *reserved) { {"nativePause", "(J)V", reinterpret_cast(&nativePause)}, {"nativeResume", "(J)V", reinterpret_cast(&nativeResume)}, {"nativeUpdate", "(J)V", reinterpret_cast(&nativeUpdate)}, - {"nativeOnInvalidate", "(J)V", reinterpret_cast(&nativeOnInvalidate)}, + {"nativeRenderSync", "(J)V", reinterpret_cast(&nativeRenderSync)}, {"nativeViewResize", "(JII)V", reinterpret_cast(static_cast(&nativeViewResize))}, diff --git a/android/cpp/native_map_view.cpp b/android/cpp/native_map_view.cpp index cc04f089e..31fb5453a 100644 --- a/android/cpp/native_map_view.cpp +++ b/android/cpp/native_map_view.cpp @@ -78,6 +78,16 @@ NativeMapView::NativeMapView(JNIEnv *env, jobject obj_, float pixelRatio_, int a fileSource = std::make_unique(fileCache.get()); map = std::make_unique(*this, *fileSource, MapMode::Continuous); + float zoomFactor = map->getMaxZoom() - map->getMinZoom() + 1; + float cpuFactor = availableProcessors; + float memoryFactor = static_cast(totalMemory) / 1000.0f / 1000.0f / 1000.0f; + float sizeFactor = (static_cast(map->getWidth()) / mbgl::util::tileSize) * + (static_cast(map->getHeight()) / mbgl::util::tileSize); + + size_t cacheSize = zoomFactor * cpuFactor * memoryFactor * sizeFactor * 0.5f; + + map->setSourceTileCacheSize(cacheSize); + map->pause(); } @@ -148,8 +158,6 @@ void NativeMapView::deactivate() { void NativeMapView::invalidate() { mbgl::Log::Debug(mbgl::Event::Android, "NativeMapView::invalidate()"); - clean.clear(); - assert(vm != nullptr); assert(obj != nullptr); @@ -169,7 +177,7 @@ void NativeMapView::beforeRender() { } void NativeMapView::afterRender() { - mbgl::Log::Debug(mbgl::Event::Android, "NativeMapView::swap"); + mbgl::Log::Debug(mbgl::Event::Android, "NativeMapView::afterRender"); if ((display != EGL_NO_DISPLAY) && (surface != EGL_NO_SURFACE)) { if (!eglSwapBuffers(display, surface)) { @@ -701,28 +709,15 @@ void NativeMapView::updateFps() { detach_jni_thread(vm, &env, detach); } -void NativeMapView::onInvalidate() { - mbgl::Log::Debug(mbgl::Event::Android, "NativeMapView::onInvalidate()"); +void NativeMapView::renderSync() { + mbgl::Log::Debug(mbgl::Event::Android, "NativeMapView::renderSync()"); if (map->isPaused()) { mbgl::Log::Debug(mbgl::Event::Android, "Not rendering as map is paused"); return; } - const bool dirty = !clean.test_and_set(); - if (dirty) { - float zoomFactor = map->getMaxZoom() - map->getMinZoom() + 1; - float cpuFactor = availableProcessors; - float memoryFactor = static_cast(totalMemory) / 1000.0f / 1000.0f / 1000.0f; - float sizeFactor = (static_cast(map->getWidth()) / mbgl::util::tileSize) * - (static_cast(map->getHeight()) / mbgl::util::tileSize); - - size_t cacheSize = zoomFactor * cpuFactor * memoryFactor * sizeFactor * 0.5f; - - map->setSourceTileCacheSize(cacheSize); - - map->renderSync(); - } + map->renderSync(); } void NativeMapView::resizeView(int w, int h) { diff --git a/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/MapView.java b/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/MapView.java index b4eece5ec..70fb5e138 100644 --- a/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/MapView.java +++ b/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/MapView.java @@ -31,6 +31,7 @@ import android.support.v4.view.GestureDetectorCompat; import android.support.v4.view.ScaleGestureDetectorCompat; import android.support.v7.app.AlertDialog; import android.text.TextUtils; +import android.util.Log; import android.view.GestureDetector; import android.view.Gravity; import android.view.ScaleGestureDetector; @@ -63,7 +64,7 @@ import com.mapzen.android.lost.api.LocationListener; import com.mapzen.android.lost.api.LocationRequest; import com.mapzen.android.lost.api.LocationServices; import com.mapzen.android.lost.api.LostApiClient; -import com.squareup.okhttp.HttpUrl; + import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; @@ -138,6 +139,9 @@ public class MapView extends FrameLayout implements LocationListener { // Used to call JNI NativeMapView private NativeMapView mNativeMapView; + // Used to track rendering + private Boolean mDirty = false; + // Used to handle DPI scaling private float mScreenDensity = 1.0f; @@ -400,6 +404,12 @@ public class MapView extends FrameLayout implements LocationListener { .setSmallestDisplacement(1) .setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY); + // Setup User Location UI + mGpsMarker = new ImageView(getContext()); + mGpsMarker.setImageResource(R.drawable.location_marker); + mGpsMarker.setVisibility(View.INVISIBLE); + addView(mGpsMarker); + // Setup Compass mSensorManager = (SensorManager) mContext.getSystemService(Context.SENSOR_SERVICE); mSensorAccelerometer = mSensorManager.getDefaultSensor(Sensor.TYPE_ACCELEROMETER); @@ -411,6 +421,7 @@ public class MapView extends FrameLayout implements LocationListener { mCompassView.setContentDescription(getResources().getString(R.string.compassContentDescription)); LayoutParams lp = new FrameLayout.LayoutParams((int)(48 * mScreenDensity), (int)(48 * mScreenDensity)); mCompassView.setLayoutParams(lp); + mCompassView.setVisibility(View.INVISIBLE); addView(mCompassView); mCompassView.setOnClickListener(new CompassOnClickListener()); @@ -883,7 +894,9 @@ public class MapView extends FrameLayout implements LocationListener { addOnMapChangedListener(new OnMapChangedListener() { @Override public void onMapChanged(MapChange change) { - updateMap(change); + if (change.equals(MapChange.MapChangeRegionWillChange) || change.equals(MapChange.MapChangeRegionWillChangeAnimated)) { + deselectAnnotation(); + } } }); } @@ -957,7 +970,7 @@ public class MapView extends FrameLayout implements LocationListener { if (mIsMyLocationEnabled) { toggleGps(false); - }; + } mNativeMapView.pause(); } @@ -1074,7 +1087,8 @@ public class MapView extends FrameLayout implements LocationListener { @Override public boolean onTouchEvent(@NonNull MotionEvent event) { // Check and ignore non touch or left clicks - if ((android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.ICE_CREAM_SANDWICH) && (event.getButtonState() != 0) && (event.getButtonState() != MotionEvent.BUTTON_PRIMARY)) { + + if ((event.getButtonState() != 0) && (event.getButtonState() != MotionEvent.BUTTON_PRIMARY)) { return false; } @@ -1103,7 +1117,6 @@ public class MapView extends FrameLayout implements LocationListener { long tapInterval = event.getEventTime() - event.getDownTime(); boolean isTap = tapInterval <= ViewConfiguration.getTapTimeout(); boolean inProgress = mRotateGestureDetector.isInProgress() || mScaleGestureDetector.isInProgress(); - mNativeMapView.setGestureInProgress(false); if (mTwoTap && isTap && !inProgress) { PointF focalPoint = TwoFingerGestureDetector.determineFocalPoint(event); @@ -1113,6 +1126,7 @@ public class MapView extends FrameLayout implements LocationListener { } mTwoTap = false; + mNativeMapView.setGestureInProgress(false); break; case MotionEvent.ACTION_CANCEL: @@ -1780,12 +1794,30 @@ public class MapView extends FrameLayout implements LocationListener { // Called when the map needs to be rerendered // Called via JNI from NativeMapView protected void onInvalidate() { - post(new Runnable() { + synchronized (mDirty) { + if (!mDirty) { + mDirty = true; + postRender(); + } + } + } + + private void postRender() { + Runnable mRunnable = new Runnable() { @Override public void run() { - mNativeMapView.invalidate(); + updateCompass(); + updateGpsMarker(); + mNativeMapView.renderSync(); + mDirty = false; } - }); + }; + + if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) { + postOnAnimation(mRunnable); + } else { + postDelayed(mRunnable, 1000 / 60); + } } /** @@ -1876,7 +1908,6 @@ public class MapView extends FrameLayout implements LocationListener { public final void setMyLocationEnabled (boolean enabled) { mIsMyLocationEnabled = enabled; toggleGps(enabled); - updateMap(MapChange.MapChangeNullChange); } /** @@ -1898,18 +1929,20 @@ public class MapView extends FrameLayout implements LocationListener { mLocationClient.connect(); updateLocation(LocationServices.FusedLocationApi.getLastLocation()); LocationServices.FusedLocationApi.requestLocationUpdates(mLocationRequest, this); - mSensorManager.registerListener(mCompassListener, mSensorAccelerometer, SensorManager.SENSOR_DELAY_UI); - mSensorManager.registerListener(mCompassListener, mSensorMagneticField, SensorManager.SENSOR_DELAY_UI); + //mSensorManager.registerListener(mCompassListener, mSensorAccelerometer, SensorManager.SENSOR_DELAY_UI); + //mSensorManager.registerListener(mCompassListener, mSensorMagneticField, SensorManager.SENSOR_DELAY_UI); } } else { if (mLocationClient.isConnected()) { LocationServices.FusedLocationApi.removeLocationUpdates(this); mLocationClient.disconnect(); mGpsLocation = null; - mSensorManager.unregisterListener(mCompassListener, mSensorAccelerometer); - mSensorManager.unregisterListener(mCompassListener, mSensorMagneticField); + //mSensorManager.unregisterListener(mCompassListener, mSensorAccelerometer); + //mSensorManager.unregisterListener(mCompassListener, mSensorMagneticField); } } + + onInvalidate(); } /** @@ -1931,18 +1964,8 @@ public class MapView extends FrameLayout implements LocationListener { * @param compassEnabled true to enable the compass; false to disable the compass. */ public void setCompassEnabled (boolean compassEnabled) { - // Set value this.mIsCompassEnabled = compassEnabled; - - // Toggle UI - if (mIsCompassEnabled) { - mCompassView.setVisibility(View.VISIBLE); - } else { - mCompassView.setVisibility(View.GONE); - } - - // Update Map - updateMap(MapChange.MapChangeNullChange); + onInvalidate(); } public void setCompassGravity(int gravity){ @@ -2022,7 +2045,7 @@ public class MapView extends FrameLayout implements LocationListener { } } - updateMap(MapChange.MapChangeNullChange); + onInvalidate(); } @Override @@ -2088,7 +2111,7 @@ public class MapView extends FrameLayout implements LocationListener { private void updateLocation(Location location) { if (location != null) { mGpsLocation = location; - updateMap(MapChange.MapChangeNullChange); + updateGpsMarker(); } } @@ -2102,22 +2125,17 @@ public class MapView extends FrameLayout implements LocationListener { } // Updates the UI to match the current map's position - private void updateMap(MapChange change) { - // Using direct access to mIsCompassEnabled instead of isCompassEnabled() for - // small performance boost as this method is called rapidly. - if (mIsCompassEnabled) { + private void updateCompass() { + if (isCompassEnabled()) { + mCompassView.setVisibility(VISIBLE); rotateImageView(mCompassView, (float) getDirection()); + } else { + mCompassView.setVisibility(INVISIBLE); } + } + private void updateGpsMarker() { if (isMyLocationEnabled() && mGpsLocation != null) { - if (mGpsMarker == null) { - // Setup User Location UI - // NOTE: mIsMyLocationEnabled == false to begin with - mGpsMarker = new ImageView(getContext()); - mGpsMarker.setImageResource(R.drawable.location_marker); - addView(mGpsMarker); - } - mGpsMarker.setVisibility(View.VISIBLE); LatLng coordinate = new LatLng(mGpsLocation); PointF screenLocation = toScreenLocation(coordinate); @@ -2130,26 +2148,6 @@ public class MapView extends FrameLayout implements LocationListener { mGpsMarker.setLayoutParams(lp); rotateImageView(mGpsMarker, 0.0f); mGpsMarker.requestLayout(); - - // Update direction if tracking mode - if(mUserLocationTrackingMode == UserLocationTrackingMode.FOLLOW_BEARING && mCompassValid){ - // TODO need to do proper filtering (see branch filter-compass) or else map will lock up because of all the compass events - long t = new Date().getTime(); - if((t-t0)>1000){ - t0 = t; - setDirection(-mCompassBearing, true); - } - } - -/* - // TODO - Too much overhead on main thread. Needs to be refactored before it - // can be re-enabled - // Update map position if NOT in NONE mode - if (mUserLocationTrackingMode != UserLocationTrackingMode.NONE) { - setCenterCoordinate(new LatLng(mGpsLocation)); - } -*/ - /* // Used For User Location Bearing UI if (mGpsLocation.hasBearing() || mCompassValid) { @@ -2169,13 +2167,28 @@ public class MapView extends FrameLayout implements LocationListener { mGpsMarker.setVisibility(View.INVISIBLE); } } - - if (change.equals(MapChange.MapChangeRegionWillChange) || change.equals(MapChange.MapChangeRegionWillChangeAnimated)) { - deselectAnnotation(); - } - } + // Old tracking code + // Update direction if tracking mode + /*if(mUserLocationTrackingMode == UserLocationTrackingMode.FOLLOW_BEARING && mCompassValid){ + // TODO need to do proper filtering (see branch filter-compass) or else map will lock up because of all the compass events + long t = new Date().getTime(); + if((t-t0)>1000){ + t0 = t; + setDirection(-mCompassBearing, true); + } + }*/ + +/* + // TODO - Too much overhead on main thread. Needs to be refactored before it + // can be re-enabled + // Update map position if NOT in NONE mode + if (mUserLocationTrackingMode != UserLocationTrackingMode.NONE) { + setCenterCoordinate(new LatLng(mGpsLocation)); + } +*/ + private void selectAnnotation(Annotation annotation) { if (annotation == null) { diff --git a/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/NativeMapView.java b/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/NativeMapView.java index 3cf56a8de..1023fa1ff 100644 --- a/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/NativeMapView.java +++ b/android/java/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxgl/views/NativeMapView.java @@ -96,8 +96,8 @@ class NativeMapView { nativeUpdate(mNativeMapViewPtr); } - public void invalidate() { - nativeOnInvalidate(mNativeMapViewPtr); + public void renderSync() { + nativeRenderSync(mNativeMapViewPtr); } public void resizeView(int width, int height) { @@ -475,7 +475,7 @@ class NativeMapView { private native void nativeUpdate(long nativeMapViewPtr); - private native void nativeOnInvalidate(long nativeMapViewPtr); + private native void nativeRenderSync(long nativeMapViewPtr); private native void nativeViewResize(long nativeMapViewPtr, int width, int height); diff --git a/include/mbgl/android/native_map_view.hpp b/include/mbgl/android/native_map_view.hpp index fe619ba6f..0908f7563 100644 --- a/include/mbgl/android/native_map_view.hpp +++ b/include/mbgl/android/native_map_view.hpp @@ -50,7 +50,7 @@ public: void enableFps(bool enable); void updateFps(); - void onInvalidate(); + void renderSync(); void resizeView(int width, int height); void resizeFramebuffer(int width, int height); @@ -93,8 +93,6 @@ private: std::shared_ptr fileCache; std::unique_ptr fileSource; std::unique_ptr map; - - std::atomic_flag clean = ATOMIC_FLAG_INIT; }; } }