Performance regression between iOS 0.5.2 and 0.5.3 #1975
Comments
friedbunny
added
iOS
|
This sounds like it would be related to @brunoabinader's label work in #1548. /cc @incanus |
We have a fix for that in #1928 (commit 5e2f3b1), which landed on version |
|
Thanks - I'm away from my machine at the moment, but I'm pretty confident I tested against 0.5.4 with the same results. I'll get you the animation code when I get back.
|
|
OK, here's the animation driver code: https://gist.github.com/tomtaylor/91d29f47f281b9d96fcd And I can confirm the same behaviour on 0.5.4. Let me know if I can give you any more info. |
|
@tomtaylor I just merged #1979 into |
|
@brunoabinader happy to, do you think you could make me a new podspec file, pointing to a precompiled .zip version? |
|
@brunoabinader I just managed to try this out on 2.1.0-pre.1, and I don't think it's quite fixed yet. I recorded a video through Quicktime from my 5s, which shows what I'm experiencing. For context, I'm touching on the scrubber at the bottom to zoom in at that point on the route, driven by the animation code I shared previously. Let me know if I can give you any more information. https://s3-eu-west-1.amazonaws.com/files.tomtaylor.co.uk/data/mapbox-lag.mov (On the flip side, it seems to perform even better when all the labels have rendered!) |
|
@brunoabinader @incanus should I open a new issue for this? |
|
Nope, we'll re-investigate this here. |
|
@tomtaylor You might be better served here by actually using the camera API in the upcoming |
|
@tomtaylor e.g. check out this little demo I whipped up using the latest pre-release: https://github.com/incanus/UISyncedMap It uses a slider to trace along a polyline annotation, and not even using the camera API, but rather just normal center/zoom setting.
It seems like an approach like this might work for your use case (tracking the elevation profile rather than a normal slider, though). |
|
Thanks @incanus, using setCoordinate with For my use, I really want to to zoom in when the user touches the scrubber, so I need to use an animation when I set the position. This regression only exhibits when zooming in - my CADisplayLink approach is nice and smooth when the user is already at the correct zoom level and is just panning around. My simplistic understanding is that if there's any outstanding label render tasks going on, those should be cancelled if a new setCoordinate request comes in, especially if it changes the zoom level. That would ensure the map waits until the zoom level has settled, before trying to rerender the labels, and the rerender process doesn't block any subsequent attempts to move the map. Does that make sense? I tried to implement it using the camera API, to see if that behaves differently in this regard, but got stuck because of #2266, so I'll give it another go when that lands. |
|
OK, using setCoordinate with |
|
Hmm, these aren't self-canceling, so that seems like it might happen @tomtaylor. I think this is also the case with MapKit. I think where we want to be is #1581 but also with cancelling handlers maybe. |
|
That'd be better, but ideally I'd like |
|
I see; it’s more about the interaction between viewport changes and label rendering holding things up (or vice-versa). |
|
Yep, understood - let me know if I can help diagnose or test anything. |
|
Moving from iOS milestone for now. |
|
@adam-mapbox commit 0753aad checks for pending faded property animations to call for another render pass. This is need in cases where we zoom in/out via gestures and causes the labels to get in a semi-transparent state because the fade animation has not finished yet - related issue/pr is #1548. After that patch, lots of changes related to render pass triggering happened. I wonder if the same issue happens on the latest version? If so, we might think about adding an option to ignore |
|
Next step here is to determine why the addition of |
|
From chat with @brunoabinader:
|
|
So, after a few hours of investigation, here's my preliminary thoughts. It looks like, when we enter this kind of use case, we're doing a sort of "double-render". Let me explain. Inside Someone who knows more about this please chime in, but is there some way to get |
|
@adam-mapbox I've been debugging this together with @tmpsantos and we found out the bottleneck isn't |
|
@brunoabinader Do you know why exactly the async update introduces stuttering? My understanding is:
So I'm keen to have somebody pinpoint the exact issue here. |
|
@jfirebaugh If the results from XCode are to be believed, then it seems that the FPS is in fact higher when the async update is triggered...but the observable result is stuttering. |
|
@adam-mapbox we had a similar approach today with Qt - rendering frames are always 60fps, but animation ticks (animation update calls from client) were raised from 39 per sec to ~60 with that asyncUpdate comment. We yet have to find out why. |
|
Interesting. On Android I have had problems where the underlying MapView says it is rendering at 60fps, but only every second frame makes it to the screen (#2773). Probably not related to this however |
|
OK. I have a theory, or at least some interesting data. I wired myself up to -[EAGLContext presentRenderbuffer], and got some interesting results. When the updateSync is wired up, we drive the rendering at almost 60 FPS. When it isn't, we drive rendering at about 12 FPS. Why does 12 FPS feel better than 60? Perhaps it has to do with the standard deviation of the frame time, or our ability to maintain a consistent framerate. Here's a snapshot of the frame times with updateSync on: Note the frametime of .0315 followed immediately by .0065. The human eye perceives that poorly. Now look at the frametimes with updateSync off: Yes, the frametime is longer, but it's also more consistent. Basically, when we drive the loop too hard, we can't keep it up, and we get choppy. So it isn't even so much about the second loop, it's more about frame-limiting, or putting a cap on our frame time. I'll look into how things change if I artificially frame-limit the app next. |
|
This issue continues to fascinate. Keep digging and great work @adam-mapbox.
|
|
Hallelujah! The promised land arrives. I have a fix. It's simple, it's correct, and it fixes the problem, I do believe. Here it is: #2922. Enjoy. @incanus @jfirebaugh @tomtaylor @brunoabinader @kkaefer @1ec5 For those who want to know more: the first clue was @jfirebaugh 's comment about de-duplicating updates. Then the second clue was the erratic way that the GL view was being driven when we were doing updates, as shown by my investigation into presentRenderbuffer. I started thinking: why are we trying to draw at 60 FPS? Then I went to rate-limit the app, and I realized: we weren't obeying any kind of smoothing or rate limiting at all. We were literally just rendering as fast (or as slow) as we could. Anytime anyone called Et voila, the app stops rendering when we don't need to, and renders smooth as butter when we want it to. There's only one final detail to think about, and that's our desired frame rate. Right now I capped it at 30. That works pretty well on almost all devices and in 95% of all cases. We may have select reasons to want to go up or down, though, in specific situations. For example on an older iPod touch or a crappy old iPhone, we may want it to be 20. And on a very fast machine under certain circumstances we may want to shoot for 60. I would also be in favor of making this a user-controllable setting, if we think that's wise. It's worth mentioning that, in my limited testing on an iPod Touch 6G, this change seems to improve performance across the board; at least, perceived performance. Pans and zooms are much smoother to the eye. |
|
@adam-mapbox - interesting so part of the problem is that we are too slow for 60fps? perhaps I should rethink #2773 and aim for capped 30fps on Android too |
|
Android used to be happily 60fps so perhaps things in core GL have gone backwards in the last year as far as performance goes. Either that or out GL styles have become to intensive. |
|
Proposed solution discussion over in #2922 (comment). |
@adam-mapbox I should add that if at all possible, I would use Other, similar issues have come up in past (see search) but we want to remain easy for devs to integrate as a standalone view. |
|
Makes sense. I also don't know how we would use GLKViewController in the manner that we ended up needing to; it isn't designed to work the way that we need. Sent from my iPhone
|
I've noticed a performance regression between iOS 0.5.2 and 0.5.3. I have a UI element which acts as a scrubber along a polyline (video here). I use a custom animation, driven by a CADisplayLink, to set the zoom and centre coordinate of the map every frame using a 3D spring behaviour.
In 0.5.2, the labels don't redraw until the animation stops, and during the animation the movement is smooth.
In 0.5.3, if the animation changes the zoom to the extent that the labels are due for refresh, it becomes jerky. If you pause the movement long enough for the labels to redraw, it becomes smooth again.
Let me know if you need any more information.
The text was updated successfully, but these errors were encountered: