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

[core] style loaded signal #6371

Merged
merged 1 commit into from
Sep 20, 2016
Merged

[core] style loaded signal #6371

merged 1 commit into from
Sep 20, 2016

Conversation

ivovandongen
Copy link
Contributor

Fixes #4527

Needed to properly time callbacks in #6032

@ivovandongen ivovandongen added the Core The cross-platform C++ core, aka mbgl label Sep 19, 2016
@ivovandongen ivovandongen self-assigned this Sep 19, 2016
@mention-bot
Copy link

@ivovandongen, thanks for your PR! By analyzing this pull request, we identified @jfirebaugh, @1ec5 and @incanus to be potential reviewers.

@tmpsantos tmpsantos changed the title 4527 style loaded signal [core] style loaded signal Sep 19, 2016
@@ -352,6 +352,8 @@ void Map::Impl::loadStyleJSON(const std::string& json) {

// force style cascade, causing all pending transitions to complete.
style->cascade(Clock::now(), mode);

view.notifyMapChange(MapChangeDidFinishLoadingStyle);
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need a signal for style loading failures. In case of parsing error, the MapChangeDidFinishLoadingStyle will be emitted here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that case, does it make more sense to add the signals to the Style itself?

In https://github.com/mapbox/mapbox-gl-native/blob/master/src/mbgl/style/style.cpp#L95 we can best check for parse errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tmpsantos to keep things clean, I've added a method to the observer on Style and kept the signal emitting in Map.


// But not when the style couldn't be parsed
emitted = false;
test.view.setMapChangeCallback([&](MapChange change) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to redefine this callback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point...

@ivovandongen
Copy link
Contributor Author

@tmpsantos I've committed the changes you requested. Let me know if there's anything else or that it can be merged.

@tmpsantos
Copy link
Contributor

:shipit:

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