This repository has been archived by the owner on Jun 21, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 122
Fix map stuttering by switching render call to use setNeedsDisplay #411
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julianrex
force-pushed
the
jrex/update-render-calls
branch
from
September 3, 2020 00:46
9d6ae4d
to
ed43f41
Compare
julianrex
requested review from
a team,
nishant-karajgikar,
knov and
fabian-guerra
and removed request for
1ec5 and
a team
September 3, 2020 20:06
julianrex
changed the title
Switch render call to use setNeedsDisplay
Fix map stuttering by switching render call to use setNeedsDisplay
Sep 3, 2020
nishant-karajgikar
approved these changes
Sep 3, 2020
@@ -168,6 +148,7 @@ void bind() override { | |||
} | |||
|
|||
#ifdef MGL_RECREATE_GL_IN_AN_EMERGENCY | |||
// TODO: Fix or remove | |||
// See https://github.com/mapbox/mapbox-gl-native/issues/14232 | |||
void MGLMapViewOpenGLImpl::emergencyRecreateGL() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing we should never be getting into this situation where we need to recreate the glView anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤞
I did test this function, but it's currently broken, and it was never a good option anyway, and an even worse one for this particular issue. Let's remove in a future PR.
katydecorah
pushed a commit
that referenced
this pull request
Sep 4, 2020
* master: Release 6.2.0 beta.1 prep (#417) Update for Xcode 12 release (#415) Switch background snapshot to periodic. (#412) Fix map stuttering by switching render call to use setNeedsDisplay (#411) Add a flag to enforce using catched tiles (#416) Improve header documentation for MGLObservable (#398) Expose styling options for the user location annotation. (#403)
This was referenced Sep 29, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes #350, where the map "stutters" while being animated. The issue is reproducible in iOS 14 beta 2 onwards (as of 2020/9/3).
The PR supersedes changes made for similar bug first reported in mapbox/mapbox-gl-native#14232 (
FB5350728
) and:-[GLKView display]
directly (via theCADisplayLink
call), to calling-[GLKView setNeedsDisplay]
,os_signpost
s for performance analysis,Mapbox GL
.Background
This issue is seen when enabling
CAEAGLLayer.presentsWithTransaction
and calling-[GLKView display]
from aCADisplayLink
.presentsWithTransaction
is used to synchronize theUIView
annotations to the map. The stutter that is seen is the CPU blocking withwait for next drawable
. The issue can also be reproduced with a blank map (essentially just aglClear
).This effect can be mitigated by setting
preferredFramesPerSecond
to 30.This issue has been reported as
FB8549369
.