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

[ios] Add preferred FPS setting; vary maximum FPS by device capability #12501

Merged
merged 1 commit into from
Jul 31, 2018

Conversation

friedbunny
Copy link
Contributor

@friedbunny friedbunny commented Jul 27, 2018

New in this PR

  • Add MGLMapView.preferredFramesPerSecond, which can be set with the provided MGLMapViewPreferredFramesPerSecond enum values or directly with an integer.
  • Adaptively set the preferred FPS based on the capabilities of the device: the oldest and least powerful devices are now capped at 30 FPS, which results in a more consistent/smoother experience (as they can rarely maintain 60 FPS).

Notes

  • This essentially exposes CADisplayLink.preferredFramesPerSecond.
  • Older devices were chosen by generation — the “legacy” devices here are Apple A7 processor and older from 2011-2013, which is a somewhat conservative choice. (I can see including A8 devices, such as iPhone 6 and iPad Air 2, if testing shows the need.)
  • I chose to keep our default at 60 FPS, despite the fact that newer iPad Pro hardware can do 120 FPS — we will need to do more testing before we can decide if defaulting to 120 FPS provides a better experience at our current level of performance. A developer can choose to opt-in to 120 FPS by setting this new property to 0 (“native”).
  • This can replace the forward-declarations added in Reinvent the course tracking wheel mapbox-navigation-ios#402.

Tailwork

  • Test on devices from ~2014 to see if they should be throttled.
  • Test on iPad Pro to see if 120 FPS can be made the default.
  • Also throttle older device simulators.
  • Consider having an even lower setting for the oldest devices (~20 FPS for iPhone 5?).

/cc @julianrex @fabian-guerra @ChrisLoer @1ec5 @bsudekum @frederoni @kkaefer @jfirebaugh

@friedbunny friedbunny added feature iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Jul 27, 2018
@friedbunny friedbunny added this to the ios-v4.3.0 milestone Jul 27, 2018
@friedbunny friedbunny self-assigned this Jul 27, 2018
@friedbunny friedbunny force-pushed the fb-displaylink-fps branch 2 times, most recently from a5762eb to 9432d47 Compare July 27, 2018 20:50
@@ -79,6 +79,21 @@ typedef NS_ENUM(NSUInteger, MGLUserTrackingMode) {
MGLUserTrackingModeFollowWithCourse,
};

/** Options for `MGLMapView.preferredFramesPerSecond`. */
typedef NSInteger MGLMapViewPreferredFramesPerSecond NS_TYPED_EXTENSIBLE_ENUM;
Copy link
Contributor Author

@friedbunny friedbunny Jul 27, 2018

Choose a reason for hiding this comment

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

This is implemented with NS_TYPED_EXTENSIBLE_ENUM (instead of using NS_ENUM or the like) to enable Swift users to omit the full key/const name and use the favored type-aware dot syntax (e.g., mapView.preferredFramesPerSecond = .lowPower).

The preferred frame rate at which the map view is rendered.

The default value for this property is
`MGLMapViewPreferredFramesPerSecondDefault`, which will adaptively set the
Copy link
Contributor

@fabian-guerra fabian-guerra Jul 27, 2018

Choose a reason for hiding this comment

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

which will adaptively set the ...

does this means that it will be overridden by the sdk according to the device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, MGLMapViewPreferredFramesPerSecondDefault means that the SDK will decide, based on the list of legacy devices in UIDevice.mgl_isLegacyDevice, whether or not to set the FPS limit to 30 or 60.


@see `CADisplayLink.preferredFramesPerSecond`
*/
@property (nonatomic, assign) MGLMapViewPreferredFramesPerSecond preferredFramesPerSecond;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose for letting devs set 0...60? does it makes a difference than restricting to -1/30/60?

Copy link
Contributor Author

@friedbunny friedbunny Jul 27, 2018

Choose a reason for hiding this comment

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

Allowing developers to set arbitrary values means that we can allow:

The -1 of MGLMapViewPreferredFramesPerSecondDefault is a placeholder so that we can name the key, but it has no direct meaning in terms of FPS (#12501 (comment)).

@julianrex
Copy link
Contributor

julianrex commented Jul 30, 2018

A few discussion points:

  • Should we synchronize rendering to the vsync? If so, being able to set an FPS of "59" doesn't make sense. Should we restrict to the set of enums? EDIT: Just read that CADisplayLink sounds like it'll do the right thing here, but I think that might also add a layer of confusion.
  • What happens on macOS?
  • What about external displays?

FOUNDATION_EXTERN MGL_EXPORT const MGLMapViewPreferredFramesPerSecond MGLMapViewPreferredFramesPerSecondDefault;

/** A conservative frame rate; typically 30 FPS. */
FOUNDATION_EXTERN MGL_EXPORT const MGLMapViewPreferredFramesPerSecond MGLMapViewPreferredFramesPerSecondLowPower;
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if lowPower would be better for a even-lower setting. 30fps is still be pretty decent frame rate. Did you run any energy tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’m not entirely happy with lowPower as the name for this setting, but I couldn’t think of anything better. Google calls this mode conservative, which avoids the potential confusion of “does this setting use less energy or is it meant for underpowered devices?” — both are true, I suppose.

@@ -87,6 +88,10 @@
const CGFloat MGLMapViewDecelerationRateFast = UIScrollViewDecelerationRateFast;
const CGFloat MGLMapViewDecelerationRateImmediate = 0.0;

const MGLMapViewPreferredFramesPerSecond MGLMapViewPreferredFramesPerSecondDefault = -1;
const MGLMapViewPreferredFramesPerSecond MGLMapViewPreferredFramesPerSecondLowPower = 30;
const MGLMapViewPreferredFramesPerSecond MGLMapViewPreferredFramesPerSecondMaximum = 60;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should .lowPower and .maximum also be based per-device type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there’s room for that kind of functionality, yeah — testing on an iPhone 5, it does seem like 15 FPS may be more appropriate for that device.

@friedbunny
Copy link
Contributor Author

friedbunny commented Jul 30, 2018

Should we synchronize rendering to the vsync?

As our property is essentially a very thin wrapper around CADisplayLink.preferredFramesPerSecond, we can look to CADisplayLink’s docs for guidance:

You can control a display link's frame rate, i.e. the number of times the specified selector of its target is called per second, by setting its preferredFramesPerSecond. However, the actual frames per second may differ from the preferred value you set: actual frame rates are always a factor of the maximum refresh rate of the device.

This tech note is also interesting:

The actual frame rate chosen by the system is usually a factor of the maximum refresh rate of the screen to provide a consistent frame rate. For example, if the maximum refresh rate of the screen is 60 frames per second, that is also the highest frame rate the display link sets as the actual frame rate; however, if you ask for a lower frame rate, the display link might choose 30, 20, or 15 frames per second (or another rate) as the actual frame rate. Consequently, it's important to consider preferredFramesPerSecond as a hint or heuristic and not a guarantee.

... so in the case of 59, I’d expect the display link to ignore that and run at 60 FPS.

Should we restrict to the set of enums?

I’d argue that exposing the ability to set the preferred FPS directly is a feature — see #12501 (comment).

What happens on macOS?

Nothing — this PR doesn’t affect that platform (and I don’t expect there to be any need to implement this feature there, for various hardware/product/development-effort reasons).

What about external displays?

I don’t know, but I’d expect this setting to generally work on external displays. External displays are a rather uncommon use case on iOS and not something we’ve really ever considered. Exposing the ability to set the underlying property arbitrarily would allow a developer to fine-tune performance in this and other edge cases we might not consider/optimize for.

@bsudekum
Copy link

Should we restrict to the set of enums?
I’d argue exposing the ability to set the preferred FPS directly is a feature — see #12501 (comment).

I agree wholeheartedly.

Copy link
Contributor

@julianrex julianrex left a comment

Choose a reason for hiding this comment

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

👍

@frederoni
Copy link
Contributor

frederoni commented Jul 31, 2018

Should we temporarily toggle default framerate when interacting with the map view via gesture recognizers? Try panning the map using LowPower or lower. The gesture feels sluggish and sometimes even decoupled.

@friedbunny
Copy link
Contributor Author

friedbunny commented Jul 31, 2018

Should we temporarily toggle default framerate when interacting with the map view via gesture recognizers? Try panning the map using LowPower or lower. The gesture feels sluggish and sometimes even decoupled.

I don’t think the SDK should do this, but a developer is welcome to. I don’t see lowPower as a mode you’ll want to set for modern devices showing interactive maps — it’s clearly an inferior experience.

For weaker devices, though, lowering the number of frames rendered can be a huge improvement in usability. Panning and drifting after a pan are particularly intensive operations; if we were to boost the frame rate beyond the capabilities of a device in interactive scenarios, we’d be eliminating most of the benefit of capping the frame rate.

From some rough measurement, it appears that gesture recognizers will continue to fire at up to the device’s maximum supported delivery rate (60Hz, in the case of all current iPhones) regardless of the frame rate. In the future, I think it would definitely be worth making sure that we invalidate or skip gesture-triggered work that won’t result in a frame draw.

- Add `MGLMapView.preferredFramesPerSecond`, which can be set with the provided `MGLMapViewPreferredFramesPerSecond` enum values or directly with an integer.
- Adaptively set the preferred FPS based on the capabilities of the device: the oldest and least powerful devices are now capped at 30 FPS, which results in a more consistent/smoother experience.
@friedbunny
Copy link
Contributor Author

There are still a variety of aspects of this PR that we may want to tweak (thresholds, devices, naming, etc.), but let’s merge this now and give it a test in the forthcoming ios-v4.3.0 pre-releases.

@friedbunny friedbunny merged commit 885f6e3 into master Jul 31, 2018
@friedbunny friedbunny deleted the fb-displaylink-fps branch July 31, 2018 19:20
@friedbunny
Copy link
Contributor Author

friedbunny commented Aug 2, 2018

If you wanted to enable 120 FPS, here’s how I’d suggest doing it:

if #available(iOS 10.3, *), UIScreen.main.maximumFramesPerSecond > 60 {
    mapView.preferredFramesPerSecond = MGLMapViewPreferredFramesPerSecond(rawValue: 0)
}

This maintains the default throttling of older devices.

@1ec5
Copy link
Contributor

1ec5 commented Aug 14, 2018

What happens on macOS?

The macOS implementation of MGLMapView doesn’t explicitly set a frame rate. In fact, MGLOpenGLLayer leaves the frame rate up to WindowServer. This could be an issue with regard to #7820, but explicitly setting a frame rate would require having Core Video drive the animation, which in my early experiments turned my MacBook Pro into a space heater and white noise generator.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants