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

[core] - initialise frame_history with an out of range zoom value #6747

Closed
wants to merge 2 commits into from

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Oct 19, 2016

Closes #6667.

This PR initialises previousZoomIndex with -1 instead of 0 in frame_history.cpp. Solves the issue that painter->needsAnimation() keeps on returning true when a map is initialised with the default camera location. This currently results in constantly rerendering the map.

Below you can see it stops using cpu resources vs the before screenshot.

screen shot 2016-10-19 at 10 02 29

More information in #6667.

Review @kkaefer

@tobrun tobrun added the Core The cross-platform C++ core, aka mbgl label Oct 19, 2016
@mention-bot
Copy link

@tobrun, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ansis, @kkaefer and @brunoabinader to be potential reviewers.

@tobrun tobrun self-assigned this Oct 19, 2016
@tobrun tobrun changed the title [core] - initialise frame_history with an outrange zoom value [core] - initialise frame_history with an out of range zoom value Oct 19, 2016
@kkaefer
Copy link
Member

kkaefer commented Oct 26, 2016

There's already a variable called firstFrame. Can we use that to detect the presence of the first frame rendered?

@tobrun
Copy link
Member Author

tobrun commented Oct 31, 2016

@kkaefer thank you for 👀 , will attempt to make it work with firstFrame.

@tobrun
Copy link
Member Author

tobrun commented Oct 31, 2016

@kkaefer in f2b8339 I proposed an alternative solution. The underlying issue is that the following if check never returns true and thus never setting the previous time.

    if (zoomIndex != previousZoomIndex) {
        previousZoomIndex = zoomIndex;
        previousTime = now;
    }

In f2b8339 I'm setting the previousTime when the firstFrame is going to be rendered. This solves the issue that needsAnimation keeps returning true.

Do you feel this is a better approach?

@tobrun
Copy link
Member Author

tobrun commented Nov 9, 2016

closing in favour of #6979

@tobrun tobrun closed this Nov 9, 2016
@tobrun tobrun deleted the 6667-zoom-0-not-idle branch November 9, 2016 16:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants