Unify update and render into a single step #7574
Conversation
|
@kkaefer, thanks for your PR! By analyzing this pull request, we identified @jfirebaugh, @1ec5 and @incanus to be potential reviewers. |
| } | ||
|
|
||
| if (style->hasTransitions()) { | ||
| onUpdate(Update::RecalculateStyle); |
There was a problem hiding this comment.
In this and the painter->needsAnimation() case, should we be |'ing Update::RecalculateStyle / Update::Repaint with flags (from updateTransitions)?
| view, | ||
| annotationManager->getSpriteAtlas()); | ||
|
|
||
| painter->cleanup(); |
There was a problem hiding this comment.
Can Painter::cleanup be called internally at the end of Painter::render?
There was a problem hiding this comment.
In the static rendering case, we're calling cleanup after the callback (meaning that we don't have to wait for the cleanup to happen before the invoking implementation can do something with the result)
|
Labels are failing to fade in/out all the way:
|
|
Looks like this is caused by calling |
Update only when, and just prior to, rendering, giving no opportunity to interleave unexpected state changes. This means that every time anything about the state is changed, we'll have to attempt a render to reflect that change. In the case of continuous rendering this has happened before this change as well, but it leaves no room for time to pass between an update and a render. In the case of still image rendering, a render call will only actually paint something to the view when all resources have been loaded.
| backend.invalidate(); | ||
| } else { | ||
| renderStill(); | ||
| } |
There was a problem hiding this comment.
Moved the invalidation call back to happen after the async. The change to master is now that before, we had a render() => asyncUpdate() => update() => invalidate() => render() flow. It's now shortened to render() => asyncInvalidate() => invalidate() => update+render()
|
I'm noticing lots of unpleasant label flickering when testing this, but I suspect it's a pre-existing issue such as #7026. |
|
@jfirebaugh yes; I re-checked with master and saw the same types of flickering as documented in #7026 |
Update only when, and just prior to, rendering, giving no opportunity to interleave unexpected state changes. Abandon the use of
AsyncTaskin continuous mode; instead callbackend.invalidate()whenever something occurs that needs to trigger a update/render, relying on the platform implementation of invalidate to perform coalescing.This fixes #7040.
/cc @jfirebaugh @pveugen