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

MGLMapView should be non-opaque by default #7256

Closed
1ec5 opened this issue Dec 1, 2016 · 8 comments
Closed

MGLMapView should be non-opaque by default #7256

1ec5 opened this issue Dec 1, 2016 · 8 comments
Assignees
Labels
iOS Mapbox Maps SDK for iOS
Milestone

Comments

@1ec5
Copy link
Contributor

1ec5 commented Dec 1, 2016

In #3101 (comment), @mxcl argues that MGLMapView’s isOpaque property should be set to NO by default on iOS. In #3096, I originally defaulted isOpaque to NO but switched it back to YES out of concern that performance might be impacted. Assuming the flash of black is still a problem, I’m in favor of making the map view non-opaque by default. On macOS, we render the map in an NSOpenGLLayer, which like all CALayers is transparent by default, and I haven’t noticed any problems stemming from that choice.

Alternatively, we could mitigate the flash without impacting performance by defaulting isOpaque to NO at initialization, then switching it to YES as soon as the style finishes loading for the first time (unless the developer has manually set isOpaque in the meantime).

/cc @incanus @frederoni

@1ec5 1ec5 added the iOS Mapbox Maps SDK for iOS label Dec 1, 2016
@friedbunny
Copy link
Contributor

friedbunny commented Dec 20, 2016

I’m also in favor of moving ahead with the switch to MGLMapView.opaque = NO. Being that this change that would be trivial to revert in a patch release (and if it caused problems we could advise users to simply set the property back themselves), would there be any issues with putting this on the v3.4.0 or v3.4.1 milestones?

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 20, 2016

I’d be fine with either v3.4.0 or v3.4.1. Any PR that changes the default should come with iosbench results comparing performance before and after.

@friedbunny friedbunny added this to the ios-3.4.1 milestone Dec 21, 2016
@friedbunny
Copy link
Contributor

Certainly we should run iosbench, but I suspect the results will be inconclusive as to the absolute effect — last time I ran it, we were maxing out at 60 FPS consistently. Perhaps an impetus to expand iosbench.

@incanus
Copy link
Contributor

incanus commented Dec 22, 2016

last time I ran it, we were maxing out at 60 FPS consistently

Hmm, this is in conflict with #5489. Are we sure about this, or am I misunderstanding what we're talking about?

@friedbunny
Copy link
Contributor

@incanus iosbench doesn’t stress the map terribly these days — it doesn’t add any sort of annotation and focuses almost solely on core rendering performance. The goal has always been to expand the range of tests in iosbench, and perhaps this is a good opportunity.

@incanus
Copy link
Contributor

incanus commented Dec 23, 2016 via email

@1ec5
Copy link
Contributor Author

1ec5 commented Dec 24, 2016

All iosbench does is render the same viewport over and over again. The framerate issues we were seeing over in #5489 are only triggered when panning, zooming, or rotating the map.

@1ec5
Copy link
Contributor Author

1ec5 commented Jan 26, 2017

Fixed in #7859 on the release-ios-v3.4.0 branch.

@1ec5 1ec5 closed this as completed Jan 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

No branches or pull requests

4 participants