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

[core] Don't allow style mutations to be overwritten by revalidation #5753

Merged
merged 4 commits into from
Aug 23, 2016

Conversation

jfirebaugh
Copy link
Contributor

Fixes #5665.

This fix is lenient about the case where the style is mutated between when the loading of a style URL is requested and when it completes. GL JS throws in this case, and native probably should prohibit it as well.

@mention-bot
Copy link

@jfirebaugh, thanks for your PR! By analyzing the annotation information on this pull request, we identified @kkaefer, @tmpsantos and @brunoabinader to be potential reviewers

@jfirebaugh jfirebaugh force-pushed the style-limit-revalidation branch 3 times, most recently from a4115d5 to 3a8f0dc Compare July 22, 2016 22:35
@jfirebaugh jfirebaugh force-pushed the style-limit-revalidation branch from 3a8f0dc to 9030b80 Compare August 22, 2016 22:47
@jfirebaugh
Copy link
Contributor Author

Rebased on master. This PR does not currently include the class list mutators (addClass, etc.) in the set of methods that trigger cancelling revalidation, although they do control state held by Style. We need to make a decision on #6118 regarding this state.

@jfirebaugh
Copy link
Contributor Author

@kkaefer, @tmpsantos and @brunoabinader can one of you review please?

@@ -67,6 +67,7 @@ class Map::Impl : public style::Observer {

std::string styleURL;
std::string styleJSON;
bool styleMutated = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this to the Style object and have it keep track of whether it was mutated itself?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to do it that way, but the issue is that we want some style mutations to be excluded from this logic, specifically those that AnnotationManager::updateStyle performs. Adding annotations should not cancel the initial revalidation. (I'll add a test for this.)

@jfirebaugh jfirebaugh force-pushed the style-limit-revalidation branch from 577e9e1 to eb9fe89 Compare August 23, 2016 16:55
@jfirebaugh jfirebaugh merged commit eb9fe89 into master Aug 23, 2016
@jfirebaugh jfirebaugh deleted the style-limit-revalidation branch August 23, 2016 16:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

style revalidation vs. runtime styling
5 participants