Calling setStyleURL() multiple times in a short period
of time will enqueue requests on the FileSource. The
order they get replied is not guaranteed to be the same
as the order we make the requests. The side effect was
the last style set not always being the one rendered.
Now we cancel the current request if we make a new one
(and there was no point on keeping it around anyway).
We now drive rendering from the main thread calling renderSync(),
but if we do it just after we set the style URL it will assert
because renderSync() expects the style to be fully loaded.
Whoever determined the boundaries of the District of Columbia did not have fit-to-bounds implementations in mind. With this change, shapes that are not unrotated rectangles fit much, much better.
Each side of the bounding box is specified independently, allowing more flexibility than the offset + padding construct supported in mapbox/mapbox-gl-js’ Camera.
The new implementation is now public and takes advantage of MGLCoordinateBounds. It is re-ported from `Camera.prototype.fitBounds()` in mapbox/mapbox-gl-js to ensure correct behavior. A new function, MGLCoordinateBoundsMake(), makes it easier to create an MGLCoordinateBounds for use with this method.
For now we set the FileSource for the Worker context
but in the future after fixing #1664 we can assert()
that FileSource are set only for the Map thread context.
Do not use the Environment anymore. The Environment ID is not
really necessary anymore as we are forwarding exceptions to the
main thread, so we always know which Map object triggered the
error.
mbgl::Thread will keep a ThreadContext for each running instance
in a thread_local so we don't need to lookup a man in the Environment
every time we need some info about the current thread.
This patch is moving the ::currentlyOn check used on Debug build from
the Environment class to the ThreadContext.
==7466== Invalid read of size 8
==7466== at 0x5F1D0B: mbgl::TransformState::getAngle() const (transform_state.cpp:159)
==7466== by 0x700CD2: mbgl::Source::tileLoadingCompleteCallback(mbgl::TileID const&, mbgl::TransformState const&, bool) (source.cpp:574)
==7466== Address 0xf1d1508 is 56 bytes inside a block of size 120 free'd
==7466== at 0x4C2A4BC: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7466== by 0x70AD61: std::_Function_base::_Base_manager<std::_Bind<std::_Mem_fn<void (mbgl::Source::*)(mbgl::TileID const&, mbgl::TransformState const&, bool)> ()(mbgl::Source*, mbgl::TileID, mbgl::TransformState, bool)> >::_M_destroy(std::_Any_data&, std::integral_constant<bool, false>) (functional:1894)
==7466== by 0x7080EB: std::_Function_base::_Base_manager<std::_Bind<std::_Mem_fn<void (mbgl::Source::*)(mbgl::TileID const&, mbgl::TransformState const&, bool)> ()(mbgl::Source*, mbgl::TileID, mbgl::TransformState, bool)> >::_M_manager(std::_Any_data&, std::_Any_data const&, std::_Manager_operation) (functional:1918)
==7466== by 0x535033: _ZNSt8functionIFvvEEaSEDn (functional:2277)
==7466== by 0x56A3F5: mbgl::WorkTask::cancel() (work_task.cpp:34)
==7466== by 0x56146C: mbgl::WorkRequest::~WorkRequest() (work_request.cpp:13)
==7466== by 0x50C5E0: std::default_delete<mbgl::WorkRequest>::operator()(mbgl::WorkRequest*) const (unique_ptr.h:76)
==7466== by 0x50C411: std::unique_ptr<mbgl::WorkRequest, std::default_delete<mbgl::WorkRequest> >::reset(mbgl::WorkRequest*) (unique_ptr.h:344)
==7466== by 0x50C374: std::unique_ptr<mbgl::WorkRequest, std::default_delete<mbgl::WorkRequest> >::operator=(std::unique_ptr<mbgl::WorkRequest, std::default_delete<mbgl::WorkRequest> >&&) (unique_ptr.h:251)
==7466== by 0x743136: mbgl::TileData::reparse(mbgl::Worker&, std::function<void ()()>) (tile_data.cpp:89)
==7466== by 0x6FEA7C: mbgl::Source::handlePartialTile(mbgl::TileID const&, mbgl::Worker&) (source.cpp:250)
==7466== by 0x700338: mbgl::Source::update(mbgl::MapData&, mbgl::TransformState const&, mbgl::Style&, mbgl::GlyphAtlas&, mbgl::GlyphStore&, mbgl::SpriteAtlas&, mbgl::util::ptr<mbgl::Sprite>, mbgl::TexturePool&, bool) (source.cpp:438)
==7466== by 0x68C1A5: mbgl::Style::update(mbgl::MapData&, mbgl::TransformState const&, mbgl::TexturePool&) (style.cpp:82)
==7466== by 0x548F5C: (anonymous namespace)::MockMapContext::update() (resource_loading.cpp:51)
==7466== by 0x548FA3: (anonymous namespace)::MockMapContext::onTileDataChanged() (resource_loading.cpp:56)
==7466== by 0x68CDE4: mbgl::Style::emitTileDataChanged() (style.cpp:212)
==7466== by 0x68CBDB: mbgl::Style::onTileLoaded(bool) (style.cpp:191)
==7466== by 0x700F38: mbgl::Source::emitTileLoaded(bool) (source.cpp:595)
==7466== by 0x700CA2: mbgl::Source::tileLoadingCompleteCallback(mbgl::TileID const&, mbgl::TransformState const&, bool) (source.cpp:573)
The invalid `TransformState::getAngle()` read comes from the following: the
storage for `TransformState` is the [bound parameter in this
callback](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae1
23c4e85cc05ae13bd5/src/mbgl/map/source.cpp#L292-292). The callback itself is
copied into various lambda bindings and eventually winds up as the [after
callback](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae1
23c4e85cc05ae13bd5/src/mbgl/map/tile_data.cpp#L89-89) for the worker task.
On the way out, `tileLoadingCompleteCallback` gets called via this callback
binding. It first does `emitTileLoaded`, which happens to cause [a _new_ work
request to be
queued](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae123
c4e85cc05ae13bd5/src/mbgl/map/source.cpp#L250-250). This cancels the
in-progress work request, [by nulling-out the `after`
callback](https://github.com/mapbox/mapbox-gl-native/blob/5e0775d276f9ea3652bae1
23c4e85cc05ae13bd5/src/mbgl/util/work_task.cpp#L34-34). But in fact, the after
callback is still executing. The next thing it does is [call
transformState.getAngle()](https://github.com/mapbox/mapbox-gl-native/blob/5e077
5d276f9ea3652bae123c4e85cc05ae13bd5/src/mbgl/map/source.cpp#L574), but the
transformState is now gone.
This eliminates a reference loop: VectorTileData holds StyleLayers which
hold StyleBuckets which held Sources which hold VectorTileDatas. This
breaks the StyleBucket -> Source link in favor of manually resolving the
reference where necessary.
Layers are added and removed dynamically on the Map thread when we use shape
annotations and we are iterating style.layers on the Worker thread without
any lock.
Now when we create a [Live|Vector]TileData at the Map thread we give it
a layers list (instead of the Style) and [Live|Vector]TileData will ultimately
make a copy of this list (on the Map thread, so no concurrency with
adding shape annotations).
The copy should be relatively cheap because we are using a shared pointer
for storing the layers on the list.
Use the callback provided for reporting the errors instead of throwing
an exception. The problem with the exception is it will happen on the
Map thread which makes it impossible to the user of our API to catch,
ultimately resulting in a crash.
In the scenario of not providing a callback, we just print an error.
