[core] Global/Viewport Collision Detection #10436

ChrisLoer · 2017-11-09T23:33:15Z

This is the native port of mapbox/mapbox-gl-js#5150: it switches us to doing collision detection globally, in a grid based on viewport coordinates instead of tile coordinates. This allows us to solve many problems, but mainly boils down to (1) more accurate collision detection, and thus denser/more-stable labeling, and (2) cross-source collision detection.

This is a squashed/rebased version of WIP PR #10103.

Differences from JS

The core collision detection algorithm is a very close port of the JS code, and the shader code is fully shared. The biggest differences are in the CrossTileSymbolIndex code and the render thread placement logic. These differences were partly driven by the requirements of native, but mostly by us trying to make improvements based on hindsight.

CrossTileSymbolIndex uses the same logic to find duplicates, but instead of trying to update opacity information directly, it now just tags duplicates with a shared crossTileID. The foreground placement code then runs placement and updates opacities using those IDs to handle duplicates. This approach makes it easier to separate opacity updates from the actual placement logic, which makes it so that we can add tiles to the render tree w/ transparent labels without blocking waiting for a full placement to complete.
All placements are "full placements" in this PR -- we don't have an "interruptible" concept as in JS. @ansis made a branch, start-collision-async that successfully runs placement on a background thread, but so far it doesn't look like a significant performance win (less foreground CPU, more total CPU, same frame-rate), so we haven't merged it.

TODO

Implement MapMode::Tile to support single-tile api-gl use case
Diagnose two remaining intermittent test failures on CI (viewport-text-depthmodetest and viewport-overzoomed-single-glyph)
Decide what to do about "flicker" introduced by fade animations for changes that used to be instant (see PR Experiment: don't fade in line labels showing for the first time #10460 and PR viewport-collision: symbol cross-fading #10468 ).

Performance

mbgl-benchmark looks pretty similar, with the exception of API_renderStill_reuse_map, which takes about twice as long (this makes sense because we do full placement for each still frame, wheres before we could re-use the already-completed tile placements).

Store results somewhere public?

I've done the bulk of performance testing using the iOS bench app running on my iPhone 5s and an ancient iPad. I've modified the bench app to include two challenging scenes:

A ten second pitched rotation animation looking at LA city streets
A ten second animation zooming into LA city streets

I added printf-style timing logs to the render code to collect detailed frame timing information for analysis in a spreadsheet.

Store results somewhere public?

Both master and viewport-collision clocked in at 60 FPS for the zoom animation. Delving into the individual frame timing data, I could see that there were frames in which the cost of placement drove the frame time over 16ms -- but most of the skipped frames were driven by time spent in the render pass, so that overall the two branches looked pretty similar.

viewport-collision was far superior to master in the pitched rotation animation, coming in at 58-59 FPS vs 41-43 FPS. Again, although placement showed up as a significant cost in some frames, render time was far more important. My guess is that frequently re-uploading static symbol buffers is responsible for trashing the performance on master (@mourner?).

Binary size looks mostly unaffected. Running bloaty on release builds of the macOS framework showed a -0.7% change in binary size, and at a quick glance the individual function deltas seemed reasonable (less code for running in the background, more for running in the foreground).

/cc @ansis @jfirebaugh @kkaefer @anandthakker @mollymerp

kkaefer

We could be looking into replace std::unordered_* with their ordered counterparts to see if the different runtime characteristic (more uniform timing) has a beneficial effect, e.g. for reducing jank.

kkaefer · 2017-11-10T11:45:15Z

src/mbgl/text/cross_tile_symbol_index.hpp

+class IndexEntry {
+    Point<float> anchorPoint;
+
+};


This class is unused (and has only private members)

kkaefer · 2017-11-10T11:46:01Z

src/mbgl/text/cross_tile_symbol_index.hpp

+};
+
+class IndexedSymbolInstance {
+    public:


Nit: we're not indenting visibility keywords, and class members are only indented once, so overall the indentation level is one too many.

kkaefer · 2017-11-10T11:56:37Z

src/mbgl/text/placement.cpp

+}
+
+void Placement::updateLayerOpacities(RenderSymbolLayer& symbolLayer) {
+    std::unordered_set<uint32_t> seenCrossTileIDs;


We could be using std::set. uint32_t already has a well defined sort order, so using a set removes the need to "hash" integers.

Can we somehow estimate the size of this set beforehand so that we can reserve some memory as a larger chunk instead of having multiple smaller allocations?

Switched to std::set.

Without actually iterating over the tiles/buckets, I don't think we can know how many cross tile IDs will find. We could start with a guess reservation like "100", but I think that kind of change would only make sense if profiling led us to do it.

kkaefer · 2017-11-10T12:17:15Z

src/mbgl/tile/geometry_tile.cpp

@@ -134,14 +116,22 @@ void GeometryTile::setLayers(const std::vector<Immutable<Layer::Impl>>& layers)
    worker.invoke(&GeometryTileWorker::setLayers, std::move(impls), correlationID);
 }

+void GeometryTile::setShowCollisionBoxes(const bool showCollisionBoxes_) {
+    if (showCollisionBoxes != showCollisionBoxes_) {
+       showCollisionBoxes = showCollisionBoxes_;


Nit: indentation is off by one space ;)

kkaefer · 2017-11-10T12:17:50Z

src/mbgl/text/placement.hpp

+            MapMode mapMode;
+            TimePoint commitTime;
+
+            std::unordered_map<uint32_t,PlacementPair> placements;


Nit: space after comma

kkaefer · 2017-11-10T12:31:02Z

src/mbgl/geometry/feature_index.hpp

+        , y(y_)
+    {}
+
+    size_t index;


Can we make these fields constant?

🤔 Making them constant makes the class non-swappable and thus non-sortable. Is there a way to get around this with an explicit move constructor or something like that?

Ah ok; I haven't found a good way around that, since we need modifiable properties for movable types.

kkaefer · 2017-11-10T14:19:08Z

src/mbgl/util/grid_index.cpp

+        for (auto& element : circleElements) {
+            if (resultFn(element.first, convertToBox(element.second))) {
+                return;
+            };


Nit: no semicolon after block

kkaefer · 2017-11-10T14:39:06Z

src/mbgl/geometry/feature_index.cpp

@@ -18,16 +18,17 @@
 namespace mbgl {

 FeatureIndex::FeatureIndex()
-    : grid(util::EXTENT, 16, 0) {
+    : grid(util::EXTENT, util::EXTENT, util::EXTENT >> 5) {


Why >> 5?

Uh, I'm not sure -- that's dividing it into a 32x32 grid, when it was a 16x16 grid before. Both cell sizes probably perform pretty similarly, but I'll revert to 16x16 and get rid of the confusing bit shift operator.

kkaefer · 2017-11-10T14:55:01Z

I'm seeing situations where zooming in across a tile boundary results in most (but not all!) labels vanishing without fading, then different labels are fading in. It looks like the algorithm retains the labels that it can match across tiles of different zoom levels. For the labels we can't match, can we fade those out instead of removing them instantly?

3 successive screenshots

kkaefer · 2017-11-10T15:05:54Z

Similarly, I'm also sometimes labels fading forth and back between different tiles(?):

6 successive screenshots

ChrisLoer · 2017-11-10T16:56:05Z

I'm seeing situations where zooming in across a tile boundary results in most (but not all!) labels vanishing without fading, then different labels are fading in. It looks like the algorithm retains the labels that it can match across tiles of different zoom levels.

This looks like expected (although suboptimal) behavior, unless I'm missing something. The labels that are popping in and out are line labels, and since we do line label placement independently for each tile, it's rare for their anchors to match up exactly enough for the cross-tile-fading algorithm to pick them up. It would be great to address this issue, but I think it's outside the scope of this PR. Status quo on master is that the old line labels will instantly disappear and be replaced by new ones.

For the labels we can't match, can we fade those out instead of removing them instantly?

We'd like to do that, but I think it's also outside the scope of this PR. The tricky part (not terrible, but not trivial) is adjusting the retain/renderable algorithm to support an additional state for tiles ("render only symbol buckets until fade is complete"). When we looked at this on JS, we decided that doing a one-sided fade (fade in, but not out) still looked smoother than no fading at all -- although I don't know how sure we are about that decision (@nickidlugash?).

ChrisLoer · 2017-11-10T18:41:08Z

src/mbgl/text/placement.cpp

+
+            if (symbolInstance.placedTextIndex) {
+                PlacedSymbol& placedSymbol = bucket.text.placedSymbols.at(*symbolInstance.placedTextIndex);
+                const float fontSize = evaluateSizeForFeature(partiallyEvaluatedTextSize, placedSymbol);


We don't use the font size for point features, so we could avoid the calculation here. The fact that we don't dynamically evaluated font size for point feature collision boxes means they're a little off from their ideal size (and if a label has its font size changing with zoom, you can see a little jump in the box as you cross integer zoom levels), but the difference is usually pretty minor.

The fact that we don't dynamically evaluated font size for point feature collision boxes means they're a little off from their ideal size

Should we use the exact size?

🤷‍♂️ I'm not sure. It seems like a minor fidelity win in exchange for a minor performance cost.

jfirebaugh

A point of divergence that took me a minute to understand was that "symbol layout" is used to refer to different subsets of the process between JS and native.

In native, SymbolLayout is a class, instances of which (prior to this PR) form part of the persistent state of GeometryTileWorker. This state tracked information that was needed between subsequent background placement operations, and is now likely no longer necessary. SymbolLayout also handled the first steps of the process: calculating the needed glyphs and icon for each feature, and merging line geometries as appropriate. When the glyphs and icon data comes back from the main thread, SymbolLayout is then responsible for creating the SymbolBucket.

In JS, the SymbolBucket class itself handles the initial glyph and icon calculation, and then the bucket is passed in to code in symbol_layout.js which fills it out.

Doesn't have to be done in this PR, but if you're tracking things to un-diverge, this would be a good thing to add. Looking at the requirements now, I think a good way to structure it would be to extract the initial glyph and icon calculations to a plain old function that takes as inputs the layout properties and tile features, and returns symbol features and icon and glyph dependencies. The latter would be requested from the main thread, and when the response is received, the former would be passed into another function to create the bucket. Doesn't look like the bucket itself needs a features member.

ChrisLoer · 2017-11-11T17:54:13Z

@kkaefer Your comments have gotten me to pay a lot more attention to the transition between two zoom levels in cities with dense road networks. There are a lot of cases where road labels have relatively small position changes between tiles, and on master the non-animated transition from one position to the other is not that jarring. On this branch there's more of a "blink" effect because they disappear before fading back in. Being able to do a cross-fade would help here, but I also wonder if there might be some heuristics we could develop for when to just skip the fade animation entirely.

kkaefer · 2017-11-10T15:23:19Z

src/mbgl/text/placement.cpp

+        if (symbolInstance.hasIcon) {
+            auto opacityVertex = SymbolOpacityAttributes::vertex(opacityState.icon.placed, opacityState.icon.opacity);
+            if (symbolInstance.iconQuad) {
+                bucket.icon.opacityVertices.emplace_back(opacityVertex);


Before a known quantity of insertions, we could try calling reserve

The standard gives clear the option to release memory, but I don't think it usually does, which means these vectors will effectively be pre-reserved at the time they're initially built up on the background thread.

kkaefer · 2017-11-10T15:56:11Z

src/mbgl/text/collision_index.cpp

+    matrix::multiply(posMatrix, projMatrix, posMatrix);
+
+    // Two versions of the query here just because util::polygonIntersectsPolygon requires
+    // GeometryCoordinates (based on int16_t). Otherwise, work with floats.


Can we somehow fold this into just one query?

Our intersection test code is integer based, which is kind of an awkward fit now that we're using float/pixel units in the grid. I'll push a change that pulls the float->integer conversion out into a separate function so it doesn't interrupt the legibility of queryRenderedSymbols so much.

kkaefer · 2017-11-13T11:17:13Z

include/mbgl/map/mode.hpp

@@ -11,7 +11,8 @@ using EnumType = uint32_t;

 enum class MapMode : EnumType {
    Continuous, // continually updating map
-    Still, // a once-off still image
+    Static, // a once-off still image of an arbitrary viewport
+    Tile // a once-off still image of a single tile


Interesting idea!

kkaefer · 2017-11-13T11:18:41Z

src/mbgl/geometry/feature_index.hpp

+        , y(y_)
+    {}
+
+    size_t index;


Ah ok; I haven't found a good way around that, since we need modifiable properties for movable types.

kkaefer · 2017-11-13T11:36:57Z

src/mbgl/layout/symbol_projection.cpp

+
+        if ((writingModes == WritingModeType::Vertical) ?
+            (firstPoint.y < lastPoint.y) :
+            (firstPoint.x > lastPoint.x)) {


I assume this code is run after we do text shaping so that all of our horizontal text runs have been converted to LTR?

Yes, text shaping takes place much earlier in the layout phase in the background.

kkaefer · 2017-11-13T11:47:14Z

src/mbgl/renderer/frame_history.hpp

-    bool dirty = true;
-
-    mbgl::optional<gl::Texture> texture;
-};


kkaefer · 2017-11-13T11:48:41Z

src/mbgl/renderer/render_layer.hpp

@@ -81,14 +81,13 @@ class RenderLayer {

    friend std::string layoutKey(const RenderLayer&);

+    // Stores current set of tiles to be rendered for this layer.
+    std::vector<std::reference_wrapper<RenderTile>> renderTiles;


Why does this have to be public?

Placement and CrossTileSymbolIndex both need (non-const) access to the set of render tiles -- this is how they go through updating all the opacity information.

It'd just be a band-aid, but maybe it would be better to explicitly "friend" them instead of making renderTiles fully public?

kkaefer · 2017-11-13T13:16:12Z

src/mbgl/text/cross_tile_symbol_index.cpp

+CrossTileSymbolLayerIndex::CrossTileSymbolLayerIndex() {
+}
+
+uint32_t CrossTileSymbolLayerIndex::maxCrossTileID = 0;


What is the strategy for keeping this static value from overflowing when the map runs for a long time?

@ansis won me over with some back-of-the-envelope calculations suggesting at least many months of continuous panning of symbol-dense tiles before overflow would allow for the possibility of collisions.

also, it's only actually a problem if a label with that id has stuck around in the index for all that time with all those changes, which is unlikely

kkaefer · 2017-11-13T13:26:53Z

src/mbgl/text/placement.cpp

+    stale = false;
+    if (mapMode == MapMode::Continuous) {
+        // Only set in continuous mode because "now" isn't defined in still mode
+        recentUntil = now + Duration(std::chrono::milliseconds(300));


Should we make this configurable?

I'd lean against making placement frequency configurable with the idea that it's hard to reason about the effects of changing it.

kkaefer · 2017-11-13T13:27:12Z

src/mbgl/text/placement.hpp

+
+namespace mbgl {
+
+    class RenderSymbolLayer;


Nit: Indentation

jfirebaugh · 2017-11-13T20:21:31Z

src/mbgl/gl/context.hpp

@@ -75,11 +75,18 @@ class Context : private util::noncopyable {
    }

    template <class DrawMode>
-    IndexBuffer<DrawMode> createIndexBuffer(IndexVector<DrawMode>&& v) {
+    IndexBuffer<DrawMode> createIndexBuffer(IndexVector<DrawMode>&& v, const BufferUsage usage=BufferUsage::StaticDraw) {


Nit: spaces before and after =.

jfirebaugh · 2017-11-13T20:41:30Z

src/mbgl/tile/geometry_tile_worker.hpp

    ~GeometryTileWorker();

    void setLayers(std::vector<Immutable<style::Layer::Impl>>, uint64_t correlationID);
    void setData(std::unique_ptr<const GeometryTileData>, uint64_t correlationID);
-    void setPlacementConfig(PlacementConfig, uint64_t correlationID);


Any plans to take advantage of the work to reduce the state space of GeometryTileWorker? If we change collision box toggling to do a full tile relayout, we should be able to eliminate the "placement" side of the state transition diagram. (If that's for a later PR, can you at least update the block comment in the cpp file to indicate that "placement" is vestigial?)

Yeah, I definitely want to simplify GeometryTileWorker by removing the two-phase loading and all the accompanying state transitions. I started in on making those changes and realized it would significantly increase the footprint of this PR, so I decided it makes sense to defer to a follow-up.

I'll add an explanatory comment for now.

jfirebaugh · 2017-11-13T20:45:55Z

src/mbgl/tile/geometry_tile.hpp

@@ -132,10 +127,11 @@ class GeometryTile : public Tile, public GlyphRequestor, ImageRequestor {
    optional<PremultipliedImage> iconAtlasImage;

    std::unordered_map<std::string, std::shared_ptr<Bucket>> symbolBuckets;
-    std::unique_ptr<CollisionTile> collisionTile;
+    std::unordered_map<std::string, std::unique_ptr<SymbolLayout>> symbolLayouts;


Unused (so the added include can be removed too).

Oops, good catch.

jfirebaugh · 2017-11-13T20:52:40Z

src/mbgl/renderer/renderer_impl.cpp

@@ -607,7 +622,7 @@ void Renderer::Impl::render(const UpdateParameters& updateParameters) {

    observer->onDidFinishRenderingFrame(
        loaded ? RendererObserver::RenderMode::Full : RendererObserver::RenderMode::Partial,
-        updateParameters.mode == MapMode::Continuous && (hasTransitions() || frameHistory.needsAnimation(util::DEFAULT_TRANSITION_DURATION))
+        updateParameters.mode == MapMode::Continuous && (hasTransitions(parameters.timePoint))


Nit: remove the surrounding parentheses too.

jfirebaugh · 2017-11-13T20:53:48Z

src/mbgl/renderer/renderer_impl.hpp

@@ -105,6 +105,9 @@ class Renderer::Impl : public GlyphManagerObserver,
    std::unordered_map<std::string, std::unique_ptr<RenderLayer>> renderLayers;
    RenderLight renderLight;

+    std::unique_ptr<CrossTileSymbolIndex> crossTileSymbolIndex;


Do these need to be pointers?

CrossTileSymbolIndex definitely doesn't need to be, I'll change that.

@ansis can say better, but I think the idea with using unique_ptr<Placement> is that we can have one Placement "in progress" while the previous one is still being used, and then efficiently swap out the new one when it finishes. That's the approach the start-collision-async branch uses. I think we're interested in continuing to experiment with the asynchronous approach, so it might be nice to leave this in for now?

jfirebaugh · 2017-11-13T20:56:58Z

src/mbgl/programs/collision_box_program.hpp

+    }
+
+	template <class DrawMode>
+		void draw(gl::Context& context,


Fix whitespace.

jfirebaugh · 2017-11-13T21:16:44Z

src/mbgl/geometry/feature_index.hpp

+
+    // Only used for symbol features
+    std::string sourceID;
+    uint8_t z;


Use CanonicalTileID instead of individual x y z members.

Same problem as here #10436 (comment): CanonicalTileID has const members and thus prevents swap-ability and thus sort-ability.

We should make them non-const in CanonicalTileID. (I'm beginning to think const member values are largely an anti-pattern.)

jfirebaugh · 2017-11-13T21:22:57Z

src/mbgl/layout/symbol_instance.cpp

-    textCollisionFeature(line_, anchor, shapedTextOrientations.second ?: shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature),
-    iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, iconPlacement, indexedFeature),
+    textCollisionFeature(line_, anchor, shapedTextOrientations.first, textBoxScale, textPadding, textPlacement, indexedFeature, overscaling),
+    iconCollisionFeature(line_, anchor, shapedIcon, iconBoxScale, iconPadding, SymbolPlacementType::Point, indexedFeature),


Can you comment why SymbolPlacementType::Point is hard-coded for icons?

jfirebaugh · 2017-11-13T21:24:03Z

src/mbgl/layout/symbol_layout.cpp

+                    symbolInstances.size(),
+                    textBoxScale, textPadding, textPlacement, textOffset,
+                    iconBoxScale, iconPadding, iconOffset,
+                    glyphPositionMap, indexedFeature, index, feature.text ? *feature.text : std::u16string{}, overscaling);


feature.text.value_or(std::u16string())

This class is responsible for tracking which symbols are "the same" between tiles at different zoom levels, so that symbol opacities (fade animations) can be copied smoothly between tiles.

- Adds early exiting "hitTest" query for fast collision detection - GridIndex now determines cell count separately for x and y axes based on grid dimensions.

Responsible for running global collision detection/symbol placement algorithm and updating symbol opacity buffers accordingly.

- Switches from tile to viewport coordinates - Represents line labels with circle geometries - Projects line labels at collision detection time to improve accuracy - Adapts tile-based symbol queries to viewport coordinates

- CollisionTile - FrameHistory - PlacementConfig

Add 'GridIndex' unit test.

…items but in a different order.

`Tile` makes sure the symbols in the resulting tile are tileable while symbols in `Still` match rendering in `Continuous` mode.

…creen. Don't mark items that are outside the collision grid range as placed. Requires new ignore because GL JS issue #5654 allows insertion of symbols outside the CollisionIndex range, and those symbols can cascade in to affect items within the viewport.

This is necessary so that when there are mixed zoom levels, child symbols get placed before parent symbols.

Update ignore links to specific issues. Bump mapbox-gl-js version to get latest text-pitch-spacing test.

Hold onto tiles after they've been removed from the render tree long enough to run a fade animation on their symbols.

ChrisLoer requested review from kkaefer, jfirebaugh and ansis November 9, 2017 23:33

kkaefer reviewed Nov 10, 2017

View reviewed changes

ChrisLoer commented Nov 10, 2017

View reviewed changes

jfirebaugh reviewed Nov 10, 2017

View reviewed changes

ChrisLoer mentioned this pull request Nov 12, 2017

Remove ignores for passing text-pitch render tests. mapbox/mapbox-gl-js#5660

Merged

kkaefer reviewed Nov 13, 2017

View reviewed changes

jfirebaugh reviewed Nov 13, 2017

View reviewed changes

This was referenced Nov 13, 2017

Convert GeometryTileWorker to "one-phase" loading #10457

Closed

Experiment: don't fade in line labels showing for the first time #10460

Closed

kkaefer mentioned this pull request Nov 14, 2017

Fix collision box calculation #10462

Closed

ChrisLoer mentioned this pull request Nov 14, 2017

viewport-collision: small changes in response to jfire review #10458

Merged

ChrisLoer force-pushed the viewport-collision branch 2 times, most recently from 2540df8 to 110c6a8 Compare November 16, 2017 17:13

ChrisLoer changed the title ~~Global/Viewport Collision Detection~~ [core] Global/Viewport Collision Detection Nov 17, 2017

ChrisLoer added 5 commits November 17, 2017 08:40

[core] Add CrossTileSymbolIndex.

6c2bdad

This class is responsible for tracking which symbols are "the same" between tiles at different zoom levels, so that symbol opacities (fade animations) can be copied smoothly between tiles.

[core] Add circle geometries to GridIndex.

f1830ad

- Adds early exiting "hitTest" query for fast collision detection - GridIndex now determines cell count separately for x and y axes based on grid dimensions.

[core] Add Placement class.

6a27228

Responsible for running global collision detection/symbol placement algorithm and updating symbol opacity buffers accordingly.

[core] Add global CollisionIndex to replace CollisionTile.

2f0f9c8

- Switches from tile to viewport coordinates - Represents line labels with circle geometries - Projects line labels at collision detection time to improve accuracy - Adapts tile-based symbol queries to viewport coordinates

[core] Enable dynamic updates of index buffers.

0e5dd48

ChrisLoer and others added 12 commits November 17, 2017 08:40

[core] Remove dead code

90fc4c1

- CollisionTile - FrameHistory - PlacementConfig

[core] Update unit tests for viewport collision.

eea18bb

Add 'GridIndex' unit test.

[core] Removed ignores for fixed issues.

44bd873

[core] Added ignores for two query tests that return the same set of …

7810b4b

…items but in a different order.

[core] Cleanup in response to review comments

3aab596

[core] Split MapMode::Still into Static and Tile

cb36bc1

`Tile` makes sure the symbols in the resulting tile are tileable while symbols in `Still` match rendering in `Continuous` mode.

[core] Sort child symbol tiles before parent symbol tiles

2519b29

This is necessary so that when there are mixed zoom levels, child symbols get placed before parent symbols.

[core] Remove ignores for passing tests.

af903a3

Update ignore links to specific issues. Bump mapbox-gl-js version to get latest text-pitch-spacing test.

[core] Cleanup in response to review comments.

64b1668

[core] Add unit test for CrossTileSymbolIndex

b2b91bf

[core] Symbol cross-fading.

40e7356

Hold onto tiles after they've been removed from the render tree long enough to run a fade animation on their symbols.

ChrisLoer force-pushed the viewport-collision branch from 110c6a8 to 40e7356 Compare November 17, 2017 16:40

kkaefer approved these changes Nov 17, 2017

View reviewed changes

ChrisLoer merged commit 25cfb58 into master Nov 17, 2017

kkaefer mentioned this pull request Nov 24, 2017

Flickering on layer when changing zoom level #10552

Closed

ansis mentioned this pull request Dec 19, 2017

port CollisionIndex and Placement changes from -native mapbox/mapbox-gl-js#5890

Merged

6 tasks

This was referenced Jan 5, 2018

Update iOS, macOS changelogs #10839

Merged

Annotations inappropriately disappear en masse based on zoom #10845

Closed

rinigus mentioned this pull request Jan 21, 2018

[qt] Performance degradation with v1.2.0 #10644

Closed

jfirebaugh deleted the viewport-collision branch March 14, 2018 16:30

ChrisLoer mentioned this pull request Apr 11, 2018

Custom Markers And Street Names Disappear on Rotate Map Close Zoom Map Pitched #11622

Closed

ChrisLoer mentioned this pull request Apr 23, 2018

Only run placement for first layer per SymbolBucket #11764

Closed

This was referenced May 10, 2018

[iOS - Android] visibleFeaturesAtPoint #11780

Closed

Trying out 6.0.0: fade in animation on symbols #10916

Closed

brunoabinader mentioned this pull request Jul 10, 2018

Symbols canned up when switching between high and low zoom levels #12356

Closed

This was referenced Jul 23, 2018

Re-implement "avoid edges" for static map tile generation #12461

Closed

"icon-allow-overlap: true, text-allow-overlap: true, text-optional: false" displays icons when it shouldn't #12483

Closed

lilykaiser mentioned this pull request Sep 5, 2018

Add an option to restore per-source collision detection #12820

Closed

ChrisLoer mentioned this pull request Sep 6, 2018

Port "crossSourceCollisions" map option to gl-native #12842

Merged

[core] Global/Viewport Collision Detection #10436

[core] Global/Viewport Collision Detection #10436

Conversation

ChrisLoer commented Nov 9, 2017 • edited Loading

Differences from JS

TODO

Performance

kkaefer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkaefer commented Nov 10, 2017

kkaefer commented Nov 10, 2017

ChrisLoer commented Nov 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfirebaugh left a comment

Choose a reason for hiding this comment

ChrisLoer commented Nov 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisLoer commented Nov 9, 2017 •

edited

Loading