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

mapView:cameraDidChange:animated: called repeatedly after shape source set to nil #11180

Closed
jmkiley opened this issue Feb 13, 2018 · 7 comments
Closed
Assignees
Labels
bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling

Comments

@jmkiley
Copy link
Contributor

jmkiley commented Feb 13, 2018

Platform: iOS
Mapbox SDK version: master

Steps to trigger behavior

  1. Add a shape source to a map's style.
  2. Set the shape property of the shape source to nil in mapView:regionDidChange:animated:.

Expected behavior

For the layer that uses the shape source to be updated to contain no shapes.

Actual behavior

cameraDidChange is called repeatedly, and there is an infinite loop. This does not seem to occur when the shape source is set to nil in another method (such as in response to a tap gesture), or when the shape property is updated wit a MGLShape object.

I have reproduced this issue in iosapp.

- (void)mapView:(MGLMapView *)mapView didFinishLoadingStyle:(MGLStyle *)style
{
    for (NSUInteger i = 0; i <= 180; i+=10) {
        
        CLLocationCoordinate2D coord[4] = {
            CLLocationCoordinate2DMake(round(0), round(i)),
            CLLocationCoordinate2DMake(round(0), round(i / 2 )),
            CLLocationCoordinate2DMake(round(20), round(i / 2)),
            CLLocationCoordinate2DMake(round(20), round(i))};

        MGLPolygonFeature *feature = [MGLPolygonFeature polygonWithCoordinates:coord count:4];
        [_features addObject:feature];
        _count += 1;
        }
    
    _shapeSource = [[MGLShapeSource alloc] initWithIdentifier:@"source" features:_features options:nil];
    [style addSource:_shapeSource];
    
    MGLFillStyleLayer *layer = [[MGLFillStyleLayer alloc] initWithIdentifier:@"layer" source:_shapeSource];
    layer.fillOpacity = [NSExpression expressionForConstantValue:@0.5];
    [style addLayer:layer];
}

- (void)mapView:(MGLMapView *)mapView regionDidChangeAnimated:(BOOL)animated {
// No crash with this if-statement
//    if ([_features count] >= _count && _count > 0) {
//        [_features removeObjectAtIndex:0];
//           MGLShapeCollectionFeature *collection = [MGLShapeCollectionFeature shapeCollectionWithShapes:_features];
//        _shapeSource.shape = collection;
//        _count -= 1;
// }
       
   _shapeSource.shape = nil;
   _count = 0;
}
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x7ffee22d9f98)
  * frame #0: 0x000000010d950c2c Mapbox`mapbox::geometry::feature_collection<short, std::__1::vector>::feature_collection(this=<unavailable>) at feature.hpp:82
    frame #1: 0x000000010d945805 Mapbox`mapbox::geometry::feature_collection<short, std::__1::vector>::feature_collection(this=0x00007ffee22da708) at feature.hpp:82
    frame #2: 0x000000010dffb939 Mapbox`mapbox::geojsonvt::Tile::Tile(this=0x00007ffee22da708) at tile.hpp:10
    frame #3: 0x000000010dffb525 Mapbox`mapbox::geojsonvt::Tile::Tile(this=0x00007ffee22da708) at tile.hpp:10
    frame #4: 0x000000010dffaea2 Mapbox`mapbox::geojsonvt::detail::InternalTile::InternalTile(this=0x00007ffee22da6b8, source=size=1, z_='\0', x_=0, y_=0, extent_=8192, buffer=2048, tolerance_=0.000732421875) at tile.hpp:30
    frame #5: 0x000000010dff7c4b Mapbox`mapbox::geojsonvt::detail::InternalTile::InternalTile(this=0x00007ffee22da6b8, source=size=1, z_='\0', x_=0, y_=0, extent_=8192, buffer=2048, tolerance_=0.000732421875) at tile.hpp:43
    frame #6: 0x000000010dfd66e1 Mapbox`mapbox::geojsonvt::GeoJSONVT::splitTile(this=0x00006040026cc4e8, features=size=1, z='\0', x=0, y=0, cz='\0', cx=0, cy=0) at geojsonvt.hpp:201
    frame #7: 0x000000010dfd52ec Mapbox`mapbox::geojsonvt::GeoJSONVT::GeoJSONVT(this=0x00006040026cc4e8, features_=0x00007ffee22db3f0, options_=0x00007ffee22db598) at geojsonvt.hpp:103
    frame #8: 0x000000010dfd4165 Mapbox`mapbox::geojsonvt::GeoJSONVT::GeoJSONVT(this=0x00006040026cc4e8, features_=0x00007ffee22db3f0, options_=0x00007ffee22db598) at geojsonvt.hpp:96
    frame #9: 0x000000010dfd4018 Mapbox`mapbox::geojsonvt::GeoJSONVT::GeoJSONVT(this=0x00006040026cc4e8, geojson_=0x00007ffee22dc3a0, options_=0x00007ffee22db598) at geojsonvt.hpp:107
    frame #10: 0x000000010dfd3fa1 Mapbox`mbgl::style::GeoJSONVTData::GeoJSONVTData(this=0x00006040026cc4e0, geoJSON=0x00007ffee22dc3a0, options=0x00007ffee22db598) at geojson_source_impl.cpp:18
    frame #11: 0x000000010dfd3f45 Mapbox`mbgl::style::GeoJSONVTData::GeoJSONVTData(this=0x00006040026cc4e0, geoJSON=0x00007ffee22dc3a0, options=0x00007ffee22db598) at geojson_source_impl.cpp:18
    frame #12: 0x000000010dfbd73f Mapbox`mbgl::style::GeoJSONSource::Impl::Impl(mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&) [inlined] std::__1::__unique_if<mbgl::style::GeoJSONVTData>::__unique_single std::__1::make_unique<mbgl::style::GeoJSONVTData, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&, mapbox::geojsonvt::Options&>(__args=0x00007ffee22dc3a0, __args=0x00007ffee22db598) at memory:3006
    frame #13: 0x000000010dfbd6f2 Mapbox`mbgl::style::GeoJSONSource::Impl::Impl(this=0x00006040002b1418, other=0x00006040002ade78, geoJSON=0x00007ffee22dc3a0) at geojson_source_impl.cpp:67
    frame #14: 0x000000010dfbdc45 Mapbox`mbgl::style::GeoJSONSource::Impl::Impl(this=0x00006040002b1418, other=0x00006040002ade78, geoJSON=0x00007ffee22dc3a0) at geojson_source_impl.cpp:49
    frame #15: 0x000000010dfb874c Mapbox`std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl>::make_shared<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(mbgl::style::GeoJSONSource::Impl const&&&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&&&) [inlined] std::__1::__libcpp_compressed_pair_imp<std::__1::allocator<mbgl::style::GeoJSONSource::Impl>, mbgl::style::GeoJSONSource::Impl, 1u>::__libcpp_compressed_pair_imp<std::__1::allocator<mbgl::style::GeoJSONSource::Impl>&, mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&, 0ul, 0ul, 1ul>(this=0x00006040002b1418, __first_args=tuple<std::__1::allocator<mbgl::style::GeoJSONSource::Impl> &> @ 0x00007ffee22dbfa8, __second_args=tuple<const mbgl::style::GeoJSONSource::Impl &, const mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::vector> > &> @ 0x00007ffee22dbf98) at memory:2172
    frame #16: 0x000000010dfb86bc Mapbox`std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl>::make_shared<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(mbgl::style::GeoJSONSource::Impl const&&&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&&&) [inlined] std::__1::__compressed_pair<std::__1::allocator<mbgl::style::GeoJSONSource::Impl>, mbgl::style::GeoJSONSource::Impl>::__compressed_pair<std::__1::allocator<mbgl::style::GeoJSONSource::Impl>&, mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(this=0x00006040002b1418, __first_args=tuple<std::__1::allocator<mbgl::style::GeoJSONSource::Impl> &> @ 0x00007ffee22dbf80, __second_args=tuple<const mbgl::style::GeoJSONSource::Impl &, const mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::vector> > &> @ 0x00007ffee22dbf70) at memory:2329
    frame #17: 0x000000010dfb863b Mapbox`std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl>::make_shared<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(mbgl::style::GeoJSONSource::Impl const&&&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&&&) [inlined] std::__1::__compressed_pair<std::__1::allocator<mbgl::style::GeoJSONSource::Impl>, mbgl::style::GeoJSONSource::Impl>::__compressed_pair<std::__1::allocator<mbgl::style::GeoJSONSource::Impl>&, mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(this=0x00006040002b1418, __pc=piecewise_construct_t @ 0x00007ffee22dbf38, __first_args=tuple<std::__1::allocator<mbgl::style::GeoJSONSource::Impl> &> @ 0x00007ffee22dbf30, __second_args=tuple<const mbgl::style::GeoJSONSource::Impl &, const mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::vector> > &> @ 0x00007ffee22dbf20) at memory:2332
    frame #18: 0x000000010dfb85f5 Mapbox`std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl>::make_shared<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(mbgl::style::GeoJSONSource::Impl const&&&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&&&) [inlined] std::__1::__shared_ptr_emplace<mbgl::style::GeoJSONSource::Impl, std::__1::allocator<mbgl::style::GeoJSONSource::Impl> >::__shared_ptr_emplace<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(this=0x00006040002b1400, __a=allocator<mbgl::style::GeoJSONSource::Impl> @ 0x00007ffee22dbef0, __args=0x00006040002ade78, __args=0x00007ffee22dc3a0) at memory:3827
    frame #19: 0x000000010dfb82d4 Mapbox`std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl>::make_shared<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(mbgl::style::GeoJSONSource::Impl const&&&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&&&) [inlined] std::__1::__shared_ptr_emplace<mbgl::style::GeoJSONSource::Impl, std::__1::allocator<mbgl::style::GeoJSONSource::Impl> >::__shared_ptr_emplace<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(this=0x00006040002b1400, __a=allocator<mbgl::style::GeoJSONSource::Impl> @ 0x00007ffee22dbeb0, __args=0x00006040002ade78, __args=0x00007ffee22dc3a0) at memory:3828
    frame #20: 0x000000010dfb8291 Mapbox`std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl>::make_shared<mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(__args=0x00006040002ade78, __args=0x00007ffee22dc3a0) at memory:4444
    frame #21: 0x000000010dfb6426 Mapbox`mbgl::Mutable<mbgl::style::GeoJSONSource::Impl> mbgl::makeMutable<mbgl::style::GeoJSONSource::Impl, mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(mbgl::style::GeoJSONSource::Impl const&&&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&&&) [inlined] std::__1::enable_if<!(is_array<mbgl::style::GeoJSONSource::Impl>::value), std::__1::shared_ptr<mbgl::style::GeoJSONSource::Impl> >::type std::__1::make_shared<mbgl::style::GeoJSONSource::Impl, mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(__args=0x00006040002ade78, __args=0x00007ffee22dc3a0) at memory:4810
    frame #22: 0x000000010dfb63f6 Mapbox`mbgl::Mutable<mbgl::style::GeoJSONSource::Impl> mbgl::makeMutable<mbgl::style::GeoJSONSource::Impl, mbgl::style::GeoJSONSource::Impl const&, mapbox::util::variant<mapbox::geometry::geometry<double>, mapbox::geometry::feature<double>, mapbox::geometry::feature_collection<double, std::__1::vector> > const&>(args=0x00006040002ade78, args=0x00007ffee22dc3a0) at immutable.hpp:46
    frame #23: 0x000000010dfb6345 Mapbox`mbgl::style::GeoJSONSource::setGeoJSON(this=0x00006040002c04d0, geoJSON=0x00007ffee22dc3a0) at geojson_source.cpp:35
    frame #24: 0x000000010d5a4000 Mapbox`::-[MGLShapeSource setShape:](self=0x0000604000461580, _cmd="setShape:", shape=0x0000000000000000) at MGLShapeSource.mm:80
    frame #25: 0x000000010d1443a1 Mapbox GL`-[MBXViewController mapView:regionDidChangeAnimated:](self=0x00007fe98ef05280, _cmd="mapView:regionDidChangeAnimated:", mapView=0x00007fe99007c200, animated=YES) at MBXViewController.m:1948
    frame #26: 0x000000010d7e2094 Mapbox`::-[MGLMapView cameraDidChangeAnimated:](self=0x00007fe99007c200, _cmd="cameraDidChangeAnimated:", animated=YES) at MGLMapView.mm:5406
    frame #27: 0x000000010d7eb3ef Mapbox`MBGLView::onCameraDidChange(this=0x0000600000445700, mode=Animated) at MGLMapView.mm:5996
    frame #28: 0x000000010d7ec1bf Mapbox`non-virtual thunk to MBGLView::onCameraDidChange(this=0x0000600000445700, mode=Animated) at MGLMapView.mm:0
    frame #29: 0x000000010da710e5 Mapbox`mbgl::Transform::startTransition(this=0x00006040001ee210)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4::operator()() const at transform.cpp:614
    frame #30: 0x000000010da7103d Mapbox`void std::__1::__invoke_void_return_wrapper<void>::__call<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&>(mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&&&) [inlined] decltype(__f=0x00006040001ee210)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&>(mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&&&) at type_traits:4291
    frame #31: 0x000000010da7102c Mapbox`void std::__1::__invoke_void_return_wrapper<void>::__call<mbgl::Transform::startTransition(__args=0x00006040001ee210)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&>(mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4&&&) at __functional_base:359
    frame #32: 0x000000010da70e49 Mapbox`std::__1::__function::__func<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4, std::__1::allocator<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_4>, void ()>::operator(this=0x00006040001ee200)() at functional:1552
    frame #33: 0x000000010e29978b Mapbox`std::__1::function<void ()>::operator(this=0x00006040001ee200)() const at functional:1903
    frame #34: 0x000000010da6f1f1 Mapbox`mbgl::Transform::startTransition(this=0x00007fe98ef00a30, now=mbgl::TimePoint @ 0x00007ffee22dc778)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3::operator()(std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >) const at transform.cpp:598
    frame #35: 0x000000010da6e908 Mapbox`void std::__1::__invoke_void_return_wrapper<void>::__call<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >(mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&&&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >&&) [inlined] decltype(__f=0x00007fe98ef00a30, __args=0x00007ffee22dcb90)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&>(fp)(std::__1::forward<std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >(fp0))) std::__1::__invoke<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >(mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&&&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >&&) at type_traits:4291
    frame #36: 0x000000010da6e8e0 Mapbox`void std::__1::__invoke_void_return_wrapper<void>::__call<mbgl::Transform::startTransition(__args=0x00007fe98ef00a30, __args=0x00007ffee22dcb90)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > > >(mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3&&&, std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >&&) at __functional_base:359
    frame #37: 0x000000010da6e5b9 Mapbox`std::__1::__function::__func<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3, std::__1::allocator<mbgl::Transform::startTransition(mbgl::CameraOptions const&, mbgl::AnimationOptions const&, std::__1::function<void (double)>, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > const&)::$_3>, void (std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >)>::operator(this=0x00007fe98ef00a20, __arg=0x00007ffee22dcb90)(std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >&&) at functional:1552
    frame #38: 0x000000010da67ef9 Mapbox`std::__1::function<void (std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >)>::operator(this=0x00007fe98ef00a20, __arg=time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1, 1000000000> > > @ 0x00007ffee22dcb90)(std::__1::chrono::time_point<std::__1::chrono::steady_clock, std::__1::chrono::duration<long long, std::__1::ratio<1l, 1000000000l> > >) const at functional:1903
    frame #39: 0x000000010da67f96 Mapbox`mbgl::Transform::updateTransitions(this=0x00007fe98ed10010, now=0x00007ffee22dcde0) at transform.cpp:628
    frame #40: 0x000000010da602dd Mapbox`mbgl::Map::Impl::onUpdate(this=0x00007fe98ed0ffd0) at map.cpp:753
    frame #41: 0x000000010e03b718 Mapbox`mbgl::style::Style::Impl::onSourceChanged(this=0x00007fe98ed10250, source=0x00006040002c04d0) at style_impl.cpp:300
    frame #42: 0x000000010e03b74c Mapbox`non-virtual thunk to mbgl::style::Style::Impl::onSourceChanged(this=0x00007fe98ed10250, source=0x00006040002c04d0) at style_impl.cpp:0
    frame #43: 0x000000010dfb6399 Mapbox`mbgl::style::GeoJSONSource::setGeoJSON(this=0x00006040002c04d0, geoJSON=0x00007ffee22dd1a0) at geojson_source.cpp:36

cc @julianrex @friedbunny

@jmkiley jmkiley added the iOS Mapbox Maps SDK for iOS label Feb 13, 2018
@jmkiley jmkiley changed the title mapView:cameraDidChange:animated: called repeated after shape source set to nil mapView:cameraDidChange:animated: called repeatedly after shape source set to nil Feb 13, 2018
@1ec5
Copy link
Contributor

1ec5 commented Feb 13, 2018

It’s unexpected that merely changing a source’s data (mbgl::style::GeoJSONSource::setGeoJSON) would cause -[MGLMapViewDelegate mapView:regionDidChangeAnimated:] to be called. Looks like mbgl is unable to distinguish between a transition that occurs due to a property change and a transition that occurs due to a camera change.

/cc @asheemmamoowala

@1ec5 1ec5 added bug macOS Mapbox Maps SDK for macOS runtime styling labels Feb 13, 2018
@jfirebaugh
Copy link
Contributor

Is this the same issue or underlying cause as #10941?

@julianrex
Copy link
Contributor

@jmkiley Been doing some digging, and this is my reading:

Doing _shapeSource.shape = nil; or adding an annotation in these delegate methods triggers Map::Impl::onUpdate() which in turn triggers Transform::updateTransitions(...).

That calls the update lambda that has previously been set up by Transform::startTransition(...). However the delegate method is called from this update lambda, and so the loop continues :(

There are a couple of solutions that might work:

  1. For the -mapView:regionDidChangeAnimated: case, the transition is coming to an end, so replacing the existing code (comments removed):

    transitionFinishFn();
    transitionFinishFn = nullptr;
    transitionFrameFn = nullptr;
    

    with:

    std::function<void()> tempFn = transitionFinishFn;
    
    transitionFinishFn = nullptr;
    transitionFrameFn = nullptr;
    
    if (tempFn)
        tempFn();
    

    allows the transition to terminate BEFORE calling the delegate method.

  2. Considering the case of -mapViewRegionIsChanging: the above isn't going to be sufficient. We could add a check to see if the transitionFrameFn has already been called that frame:

    void Transform::updateTransitions(const TimePoint& now) {
        if (transitionFrameFn) {
            if (transitionFrameFnCalled) {
                // Already called this frame.
                // Do stuff
            } else {
                transitionFrameFnCalled = true;
                transitionFrameFn(now);
            }
        }
        transitionFrameFnCalled = false;
    }
    

    This covers both cases, though I think it's worth implementing (1) regardless.

  3. Longer term, perhaps the Map::Impl::onUpdate() needs to be called independently and regularly (e.g. from a CADisplayLink), and interested parties set something like a setNeedsUpdate?

@jfirebaugh Pretty sure this is a duplicate of #10941.

/cc @asheemmamoowala

@1ec5
Copy link
Contributor

1ec5 commented Feb 15, 2018

#10941 (comment) indicates that this is a regression introduced in v3.7.x. Did the logic around transitions in mbgl change in v3.7.0?

@1ec5
Copy link
Contributor

1ec5 commented Mar 14, 2018

Given the reports of this working correctly in v3.6.x, we should bisect between v3.6.x and v3.7.0 to find out what introduced the regression.

@1ec5
Copy link
Contributor

1ec5 commented Apr 10, 2018

Relatedly, #5833 is about infinite recursion caused by setting the camera within a camera change’s completion handler.

julianrex pushed a commit that referenced this issue Apr 11, 2018
julianrex pushed a commit that referenced this issue Apr 17, 2018
julianrex pushed a commit that referenced this issue Apr 19, 2018
julianrex pushed a commit that referenced this issue Apr 23, 2018
julianrex pushed a commit that referenced this issue Apr 24, 2018
julianrex pushed a commit that referenced this issue Apr 26, 2018
julianrex pushed a commit that referenced this issue May 18, 2018
julianrex pushed a commit that referenced this issue May 21, 2018
@julianrex
Copy link
Contributor

Fixed in #11614. Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling
Projects
None yet
Development

No branches or pull requests

4 participants