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

Optimize animated annotations #5385

Merged
merged 4 commits into from
Jun 17, 2016
Merged

Optimize animated annotations #5385

merged 4 commits into from
Jun 17, 2016

Conversation

jfirebaugh
Copy link
Contributor

I added an "Add Animated Annotation" menu item in the macOS test app, and then profiled it, which revealed several gross inefficiencies. After fixing those, the profile is heavily dominated by rendering performance -- updating the annotation is less than 3% of runtime.

@tobrun, can you test this out on Android?

/cc @1ec5 for macOS test app changes.

DroppedPinAnnotation *annotation = [[DroppedPinAnnotation alloc] init];
[self.mapView addAnnotation:annotation];

[NSTimer scheduledTimerWithTimeInterval:1/60
Copy link
Contributor

Choose a reason for hiding this comment

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

The time interval is 0 since no floats are involved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was copied from dropManyPins:. I'll fix both occurrences.

@tobrun
Copy link
Member

tobrun commented Jun 17, 2016

@jfirebaugh this a major improvement, we aren't dropping any frames anymore with this fix.
https://www.dropbox.com/s/z2vmjhldi4pndgl/device-2016-06-17-105618.mp4?dl=0

@tobrun
Copy link
Member

tobrun commented Jun 17, 2016

I'm having some issues bringing this from master to the release-v4.1.0. We have diverged a lot and I'm not sure which core pieces I need to pick and which to ignore.

To test, I reversed the approach and cherry picked the animating car example from release-v4.1.0 into this branch:

ezgif com-video-to-gif 8

Here is also an example with +100 markers:

ezgif com-video-to-gif 7

cc @cammace @ivovandongen @pveugen @zugaldia

@jfirebaugh
Copy link
Contributor Author

Ported by hand to the Android release branch in animated-annotation-android-4.1.0. @zugaldia can you test?

@zugaldia
Copy link
Member

@jfirebaugh This is the AnimatedMarkerActivity without your changes in the current release branch:

without

This is the same activity brought to your animated-annotation-android-4.1.0 branch:

with

Bottom line, your port is improving substantially the performance. Let's merge it into the release branch 👍 .

@zugaldia
Copy link
Member

@jfirebaugh Bringing animated-annotation-android-4.1.0 over to the release branch via #5392.

@jfirebaugh jfirebaugh merged commit 2921127 into master Jun 17, 2016
@jfirebaugh jfirebaugh deleted the animated-annotation branch June 17, 2016 19:51
@cammace
Copy link
Contributor

cammace commented Jun 17, 2016

getting a bunch of

06-17 15:42:25.385 718-718/com.mapbox.mapboxsdk.testapp D/mbgl: [JNI]: nativeUpdateMarker

output in logcat which makes it impossible to read without filtering. Anyway we could remove this from printing?

@tobrun
Copy link
Member

tobrun commented Jun 24, 2016

@cammace you can disable this type of logging if you build with BUILDTYPE=Release

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants