[core] Implement data-driven styling for {text,icon}-{color,opacity,halo-color,halo-blur,halo-width} #7939

anandthakker · 2017-02-03T21:10:26Z

No description provided.

mention-bot · 2017-02-03T21:10:27Z

@anandthakker, thanks for your PR! By analyzing this pull request, we identified @jfirebaugh and @ansis to be potential reviewers.

anandthakker · 2017-02-17T22:26:01Z

Currently running into the following problem:

SymbolIconProgram and SymbolSDFProgram both use SymbolPaintProperties when instantiating Program<>.
SymbolPaintProperties, being generated from the style spec, includes properties that bind to GL attributes like a_halo_color that are not present in the (non-sdf-)icon shader.

I think this is the reason that I'm getting a glBindAttribLocation(id, location, name): Error GL_INVALID_VALUE error: according to the debugger, this is happening during mbgl::gl::bindAttributeLocation(id, location, "a_halo_color_max"), during the instantiation of the symbolIcon program.

update: I believe the reason this isn't blowing up in GL JS is that, there, we:

inspect each program, storing off the indexes for each attribute & uniform that are actually present in the shaders.
and then simply bail if a program configuration includes an attribute that isn't actually available to bind to.

jfirebaugh · 2017-02-17T22:48:36Z

I don't think that's the reason -- that pattern occurs elsewhere, like with a_outline_color and fill programs. I suspect the reason is rather that we've now introduced more than GL_MAX_VERTEX_ATTRIBS attributes. This is a problem. We're either going to have to find creative ways of packing multiple attributes into one, or move to a system more like mapbox-gl-js, where shaders are dynamically generated that use only as many attributes as required by data-driven properties, and use uniforms otherwise -- this moves the failure condition to the point when someone tries to use too many data-driven properties at the same time.

anandthakker · 2017-02-17T23:25:31Z

Yeah - so it looks like we're ending up with attribute ids >= 16 because each data-driven icon- / text- property is mapping to a separate attribute, so we're ending up with many more than we need. E.g., text-opacity and icon-opacity both become a_opacity, but they take up two separate positions in the parameter list:

mbgl::gl::Attributes<
mbgl::attributes::a_pos,
mbgl::attributes::a_offset<2>,
mbgl::attributes::a_texture_pos,
mbgl::attributes::a_data<4>,
mbgl::attributes::Min<mbgl::attributes::a_opacity>,
mbgl::attributes::Min<mbgl::attributes::a_fill_color>,
mbgl::attributes::Min<mbgl::attributes::a_halo_color>,
mbgl::attributes::Min<mbgl::attributes::a_halo_width>,
mbgl::attributes::Min<mbgl::attributes::a_halo_blur>,
mbgl::attributes::Min<mbgl::attributes::a_opacity>,
mbgl::attributes::Min<mbgl::attributes::a_fill_color>,
mbgl::attributes::Min<mbgl::attributes::a_halo_color>,
mbgl::attributes::Min<mbgl::attributes::a_halo_width>,
mbgl::attributes::Min<mbgl::attributes::a_halo_blur>,
mbgl::attributes::Max<mbgl::attributes::a_opacity>,
mbgl::attributes::Max<mbgl::attributes::a_fill_color>,
mbgl::attributes::Max<mbgl::attributes::a_halo_color>,
mbgl::attributes::Max<mbgl::attributes::a_halo_width>,
mbgl::attributes::Max<mbgl::attributes::a_halo_blur>,
mbgl::attributes::Max<mbgl::attributes::a_opacity>,
mbgl::attributes::Max<mbgl::attributes::a_fill_color>,
mbgl::attributes::Max<mbgl::attributes::a_halo_color>,
mbgl::attributes::Max<mbgl::attributes::a_halo_width>,
mbgl::attributes::Max<mbgl::attributes::a_halo_blur> >

anandthakker · 2017-02-20T17:51:19Z

src/mbgl/programs/symbol_program.hpp

@@ -94,7 +94,7 @@ class SymbolIconProgram : public Program<
                                       const TransformState&);
 };

-class SymbolSDFProgram : public Program<
+class SymbolSDFIconProgram : public Program<


I initially tried to just parameterize SymbolSDFProgram on the paint property types, but it was proving sort of unweildy. Opted to duplicate the declarations and minimize code duplication in implementation via the shared makeSDFValues function

How about:

template <class PaintProperties> class SymbolSDFProgram : public Program< ... PaintProperties> { ... }; using SymbolSDFIconProgram = SymbolSDFProgram<style::IconPaintProperties>; using SymbolSDFTextProgram = SymbolSDFProgram<style::TextPaintProperties>;

Yeah, that's something like what I was trying before and having trouble with -- but it's very possible that it was just other mid-refactor issues that were making it hard to see clearly. Lemme try again.

anandthakker · 2017-02-20T20:21:18Z

@jfirebaugh Okay, I think this is ready for a 👀 .

History is hopefully reasonable to read commit-by-commit (with the generated code in its own commit).
There are three render tests failing for what seem like maybe platform-specific detail reasons? In two of the failures the difference between expected/actual isn't visible to the naked eye... The text-halo-blur one could actually be related to the blur property, but it seems more like something weird w/ the background.

anandthakker · 2017-02-20T20:24:30Z

src/mbgl/layout/symbol_instance.hpp

@@ -28,6 +28,7 @@ class SymbolInstance {
    CollisionFeature textCollisionFeature;
    CollisionFeature iconCollisionFeature;
    WritingModeType writingModes;
+    std::size_t featureIndex;


In GL JS, we did this with a direct reference to the associated feature. Would it be better to do a similar thing here, making this a SymbolFeature *?

^ note: the reason I didn't was because, since SymbolInstance:SymbolFeature is many:1, it seemed like there would have to be some weird acrobatics using std::shared_ptr. Rather than introduce that machinery, I figured since both of these objects are really internal to SymbolLayout, this index-based coupling seemed okay.

I think this is okay for now. I started learning more about the various classes used during symbol placement and I'm thinking maybe we can move to a hierarchy where SymbolFeature owns SymbolInstances, and can provide a direct SymbolFeature& reference at the appropriate time.

anandthakker · 2017-02-21T14:48:37Z

Almost there, I think:

Working on trying to run the android tests locally (as @ivovandongen indicated that most are skipped in CI bc of timing issues)
One of the Qt travis builds failed the Memory.Footprint test. Re-running it now just in case it was a fluke... but no real clue where to start on solving that one.

Update: @tmpsantos explained the context of the Memory.Footprint test. Some additional memory usage is necessary, because we now need to hold on to feature properties after SymbolLayout::prepare() (whereas before, we cleared the features vector when we were done). I tried to mitigate it some in 13d1d30 by clearing each feature's geometry, but it doesn't seem to have helped.

@tmpsantos indicates that this isn't necessarily a blocker.

cc @jfirebaugh (also FYI the remaining tests appear to be passing now)

jfirebaugh · 2017-02-21T19:52:45Z

So we're going ~660kb over the Memory.Footprint test limit. How much under the limit are we before this change, i.e. how much is the total increase?

anandthakker · 2017-02-21T23:33:03Z

So we're going ~660kb over the Memory.Footprint test limit. How much under the limit are we before this change, i.e. how much is the total increase?

Force-logged it off of master (by setting the threshold to 0 in the unit test, because apparently googletest doesn't let you just arbitrary informational stuff 🙄 ):

Expected: (vectorFootprint) < (0), actual: 6.71485e+07 vs 0

( https://travis-ci.org/mapbox/mapbox-gl-native/jobs/204010132#L1013 )

On this branch, we get:

Expected: (vectorFootprint) < (65 * 1024 * 1024), actual: 6.88349e+07 vs 68157440

So that's a 1.7MB increase. Seems pretty significant.

anandthakker · 2017-02-22T12:30:51Z

It occurs to me that this test is measuring RSS, and so I'm not sure whether these literal numbers are meaningfully interpretable as "the increase in memory usage".

tmpsantos · 2017-02-22T13:05:34Z

It occurs to me that this test is measuring RSS, and so I'm not sure whether these literal numbers are meaningfully interpretable as "the increase in memory usage".

What this test is doing is:

"Warming up" the memory by rendering 10x and deleting the map object after rendering.
Measure the RSS, which should now include all the static data, thread locals and 3rd party libraries initialization data.
Render N times (currently 15), but keep the map objects.
Measure the current RSS compared to the first measurement and divide by N (15). This should be roughly how much each Map object is using. I set the bar at 65 MB per Map for vector and 25 MB per Map for raster. These numbers are arbitrary.
Fail if it goes over this limit.

Few notes:

We use jmalloc because it is very aggressive at freeing memory, making the measurement more precise.
This test is surprisingly stable, and has protected us from memory growth in the past. Last one I remember was a PR that would pre-allocate a buffer for the annotations layer even if you don't have any annotation on the map. The regression was prevented by making this allocation lazy.
We could have used more sophisticated ways of tracking memory allocations, but RSS snapshots look reliable and fast enough for running on Travis. That said, any suggestion is welcome here.

tmpsantos · 2017-02-22T13:12:21Z

One last thing that is worth mentioning. This test is focused on Map objects rather than tiles. If your code increases tile memory usage by 250 Kb per tile it is unlikely that this test will catch it, but the effects will be seen in an app when rendering 10 tiles at once in a tilted map (~2.5 MB memory increase).

Additional manual testing with larger maps is advised, I personally like this tool:
https://github.com/pixelb/ps_mem

anandthakker · 2017-02-22T13:40:18Z

Capturing some notes from chat:

@anandthakker:

Agreed that this test seems like a good/reliable way to detect when a regression has occurred in memory usage.
My question is because, now that I know that the regression exists in my branch, I am interested in assessing more quantitatively what the memory impact of these changes are

@tmpsantos:

the other thing is, as we add more features, eventually we will have to use more memory
so it is just a matter of raise the memory bar there and life goes on

@anandthakker:

yeah, this branch is one such case, for sure: it includes a necessary change wherein SymbolLayout (one per symbol style layer) now holds on to a vector of features, rather than clearing it
we might, in the end, have to raise the memory bar. But if we’re going to do that, then ideally I’d like to:

Determine how much more memory usage we’re incurring for this change.
Actually account for (roughly) that amount based on the known increases in the change (e.g. the symbol features)

I’m working on 2 by doing some napkin math on the number of symbol layers / symbols that show up in this test, but I’m not sure how to do 1 with any confidence/accuracy (edited)

tmpsantos · 2017-02-22T14:27:19Z

Did some tests to measure the memory impact of this change. On my Linux box, for rendering this map (1920x1200) using GLFW:

$ cat /tmp/mbgl-native.cfg 
-73.9976
40.7333
14.128
-57.7533
60
2

I started the map with the same configuration 10x with this PR applied and without (the hash just before the first commit of this branch.

In average ps_mem was giving me 139.17 MB with this PR and 128.01 MB without. An increase of 11.16 MB.

Repeating the method with a 1024x768 map, the observed increase was 8 MB.

anandthakker · 2017-02-22T19:22:11Z

Looks like in the Memory.Footprint test, we're dealing with 980 symbol features total... that's 1.7KB per symbol feature, which would be plausible if we're keeping the geometries around but seems too big for just the text, placement, properties.

@jfirebaugh two questions:

Do you see any reason why clearing each feature's geometry here is failing to release the memory?
Could the paint vertex buffers be part of the problem here? I've been assuming not thus far, because I figured that, like in GL JS, they'd be destroyed after uploading to GL.

anandthakker · 2017-02-22T21:15:46Z

stats from profiling in Instruments:

master

dds-symbol

Obtained by profiling the Memory.Footprint test, and the filtering the Instruments time range to go from the beginning to point of peak heap usage.

The +43MB delta in heap usage includes:

+8MB - Malloc 4.00 MiB
+8MB - Malloc 1.00 MiB
+9.5MB - Malloc 80 Bytes
+3.91MB - SymbolFeature

Other than SymbolFeature, hard to say what these others are... probably allocations by various container classes?

jfirebaugh · 2017-02-22T22:27:34Z

Here are a couple things I would try to reduce the memory growth:

Have SymbolFeature assume and hold ownership of the std::unique_ptr<GeometryTileFeature>, rather than copying its constituent parts. This may help reduce fragmentation.
Remove SymbolFeature::iconOffset and iconRotation in favor of calculating them as needed, like with the other properties in this PR.

jfirebaugh · 2017-02-23T00:52:39Z

src/mbgl/layout/symbol_layout.cpp

@@ -229,7 +232,8 @@ void SymbolLayout::prepare(uintptr_t tileUID,
    const bool textAlongLine = layout.get<TextRotationAlignment>() == AlignmentType::Map &&
        layout.get<SymbolPlacement>() == SymbolPlacementType::Line;

-    for (const auto& feature : features) {
+    for (auto it = features.begin(); it != features.end(); ++it) {
+        auto feature = *it;


This is creating a copy; it should be auto& feature = *it;. Then feature.geometry.clear(); will have a meaningful effect.

Alas, this change only seems to take us down by 51K in the Memory.Footprint test: 6.88349e+07 => 6.87836e+07.

jfirebaugh · 2017-02-23T00:53:10Z

src/mbgl/layout/symbol_layout.cpp

@@ -496,6 +502,12 @@ std::unique_ptr<SymbolBucket> SymbolLayout::place(CollisionTile& collisionTile)
                    layout.get<IconKeepUpright>(), iconPlacement, collisionTile.config.angle, symbolInstance.writingModes);
            }
        }
+
+        const auto feature = features.at(symbolInstance.featureIndex);


Same here, should be const auto& feature = ....

anandthakker · 2017-02-23T16:20:29Z

Have SymbolFeature assume and hold ownership of the std::unique_ptr, rather than copying its constituent parts. This may help reduce fragmentation.

I think that one consequence of this is that it would be problematic to then clear the geometries after we didn't need them anymore (unless we added something like a clearGeometries() mutator to the GeometryTileFeature interface). I guess since clearing them didn't do much, maybe that's okay.

anandthakker · 2017-02-23T19:33:06Z

src/mbgl/layout/symbol_feature.hpp

-        properties(feature->getProperties())
+    SymbolFeature(std::unique_ptr<GeometryTileFeature> feature_) :
+        feature(std::move(feature_)),
+        geometry(feature->getGeometries()) // we need a mutable copy of the geometry for mergeLines()


☝️ @jfirebaugh note - mergeLines relies on mutating the SymbolFeatures' geometries, but getGeometries() returns a copy, so this bit of copying still needed to happen.

anandthakker · 2017-02-23T20:29:35Z

@jfirebaugh implemented your two suggestions from #7939 (comment). Moving iconOffset and iconRotate off only helped marginally, but transferring ownership of the GeometryTileFeatures made a pretty big difference: brought us down to 6.81918e+07 in the Memory.Footprint test -- so that's a +1MB delta from master (instead of +1.7MB).

jfirebaugh · 2017-02-23T23:24:39Z

src/mbgl/layout/symbol_layout.cpp

    }

    // Determine and load glyph ranges
    const size_t featureCount = sourceLayer.featureCount();
    for (size_t i = 0; i < featureCount; ++i) {
-        auto feature = sourceLayer.getFeature(i);
-        if (!leader.filter(feature->getType(), feature->getID(), [&] (const auto& key) { return feature->getValue(key); }))
+        SymbolFeature ft(sourceLayer.getFeature(i));


It would be better to construct SymbolFeature only for features that pass the filter, to avoid unnecessarily building and copying the geometry.

Sorry, should have been more explicit:

auto feature = sourceLayer.getFeature(i); if (!filter) continue; SymbolFeature ft(std::move(feature));

We don't want to call sourceLayer.getFeature(i) twice.

Oh whoops, yeah that's what I'd intended -- trying to go too fast heh

@jfirebaugh amended.

jfirebaugh · 2017-02-23T23:26:08Z

src/mbgl/programs/symbol_program.cpp

@@ -1,14 +1,21 @@
+#include <iostream>


Leftover from debugging?

jfirebaugh · 2017-02-23T23:28:26Z

src/mbgl/programs/symbol_program.cpp


 namespace mbgl {

 using namespace style;

 static_assert(sizeof(SymbolLayoutVertex) == 16, "expected SymbolLayoutVertex size");

+MBGL_DEFINE_ENUM(SymbolSDFPart, {


MBGL_DEFINE_ENUM is needed only where enum ↔ string conversion is required, which isn't the case with SymbolSDFPart.

jfirebaugh · 2017-02-23T23:28:56Z

src/mbgl/programs/symbol_program.hpp

@@ -8,6 +8,7 @@
 #include <mbgl/util/geometry.hpp>
 #include <mbgl/util/size.hpp>
 #include <mbgl/style/layers/symbol_layer_properties.hpp>
+#include <mbgl/style/layers/symbol_layer_impl.hpp>


Shouldn't be necessary.

This is needed here because it's where style::{Icon,Text}PaintProperties are declared.

jfirebaugh · 2017-02-23T23:29:58Z

src/mbgl/programs/symbol_program.hpp

 using SymbolLayoutVertex = SymbolLayoutAttributes::Vertex;
-using SymbolAttributes = SymbolIconProgram::Attributes;
+using SymbolIconAttributes = SymbolIconProgram::Attributes;
+using SymbolGlyphAttributes = SymbolSDFTextProgram::Attributes;


Rename this to SymbolTextAttributes.

anandthakker · 2017-02-24T21:16:43Z

A little more on the memory usage increase. Running a single iteration from the Memory.Footprint test and estimating each symbol feature's footprint as 4 * # coordinates + sum(length(each string property)) + 8 * # numeric properties yields a total of 1,481,348 bytes (213,664 from geometry, the rest from properties).

update: Property values are shared among features within a tile, so this is actually overestimate.

update 2: Corrected to not double-count shared property values in each tile layer: 434,632

jfirebaugh · 2017-02-28T00:14:46Z

src/mbgl/tile/vector_tile.cpp

@@ -15,9 +15,17 @@ class VectorTileLayer;

 using packed_iter_type = protozero::iterator_range<protozero::pbf_reader::const_uint32_iterator>;

+struct VectorTileLayerData {


This should also have a shared_ptr<const std::string> data member, to ensure that the data backing the tags_iter and geometry_iter is retained.

@jfirebaugh 👍 done.

@anandthakker @jfirebaugh - Landed here in a revised effort to integrate https://github.com/mapbox/vector-tile back into mbgl. I have some questions.

Without VectorTileLayerData and is shared_ptr<const std::string> data member what exactly would go wrong? I figure there must be somewhere in the code that expects to create a VectorTileLayer and use it/cache it in such a way beyond when the underlying data would be kept alive. So keeping a shared_ptr to the data keeps it alive as long as all VectorTileLayer instances are alive? Can you point to the code that needs this? (I could not find it)

Why does the VectorTileFeature need a std::shared_ptr<VectorTileLayerData> layerData rather than just const VectorTileLayer&. This indicates to me that somewhere in the code expects to keep a VectorTileFeature instance alive longer than the parent VectorTileLayer. Is this happening and can you point to the place in the code that requires is? Or is this perhaps unneeded - in that the VectorTileLayer will always be alive as long as the VectorTileFeature created from it?

Why does the VectorTileFeature need a std::shared_ptr layerData rather than just const VectorTileLayer&. This indicates to me that somewhere in the code expects to keep a VectorTileFeature instance alive longer than the parent VectorTileLayer

@springmeyer Yeah -- this is a possibility that did not exist before, but was introduced in this PR. (And apologies: I should have left a comment justifying the shared_ptrs introduced here.)

The reason it can now happen is over in SymbolLayout, and specifically SymbolFeature (here). SymbolLayout is constructed with a vector tile layer, and then later on--when there's no guarantee that the vector tile data is still around--expected to produce a "bucket" containing final rendering data. Before this PR, we copied the necessary data into SymbolFeature structs which SymbolLayout could hang on to; but because we need feature properties for data-driven styling--and because these turn out to be quite expensive to copy--SymbolFeature now holds a reference to the VectorTileFeature.

Thus, VectorTileFeatures may need to outlive their parent VectorTileLayers, so they need shared ownership of VectorTileLayerData.

As for the shared_ptr<const std::string> data that's held by VectorTileLayerData -- I think we need this basically as a consequence of the above, since VectorTileFeatures may parse PBF data lazily. But even without these symbol-layout-related changes, it's making explicit a dependency that already existed, since there's no explicit guarantee that the parent VectorTileData would outlive a particular VectorTileLayer. The lifecycles happened to be okay before this, but only because of implicit constraints on how we used these objects.

@jfirebaugh should probably check my math on this, as I haven't spent time in the parts of our vector tile parsing/caching stuff outside of SymbolLayout.

Yes, that's correct. One minor note is that shared_ptr<const std::string> predated the addition of VectorTileLayerData and comes from the definition of Response. I believe this was originally done to avoid string copies. Might be better to use move semantics now.

@anandthakker and @jfirebaugh - awesome thank you for the confirmation. I'm grateful for the confirmation and will now proceed with my PR revising this code and keep all the explicit ownership relationships fully intact. The context I should have mentioned before is that I hope my PR will help performance so I was looking closely at the nearby code for any optimization potential, which is what lead me to the question of whether the shared pointers were absolutely needed. Great to know they are.

Ticketed @jfirebaugh's idea: #9167

jfirebaugh · 2017-02-28T00:16:32Z

src/mbgl/tile/vector_tile.cpp

-VectorTileFeature::VectorTileFeature(protozero::pbf_reader feature_pbf, const VectorTileLayer& layer_)
-    : layer(layer_) {
+VectorTileFeature::VectorTileFeature(protozero::pbf_reader feature_pbf, std::shared_ptr<VectorTileLayerData> layerData_)
+    : layerData(layerData_) {


layerData(std::move(layerData_))

anandthakker · 2017-02-28T03:04:31Z

Latest changes bring us down to 68096819, which is under the Memory.Footprint ceiling. This is great! (Except: I really am not sure why this helped. If anything, I thought that using shared_ptrs to a separate VectorTileLayerData, rather than reference to VectorTileLayer, should have cause a small increase in memory footprint...)

Prepares for enabling DDS on symbol paint properties by allowing the SymbolFeatures, which we keep around after constructing SymbolLayout, to be used in evaluating data-driven paint properties later in the layout process.

The `Program` types are set up to bind GL attributes to each of the data-driven paint properties specified in the `PaintProperties` type provided. Since `SymbolPaintProperties` specifies both `Text*` and `Icon*` properties, the symbolIcon, symbolIconSDF, and symbolGlyph programs each attempt to bind roughly double the number of attributes that they actually need. This change addresses this by: - Adding the more specific `IconPaintProperties` and `TextPaintProperties` types, which are subsets of the full `SymbolPaintProperties`. - The symbol layer continues to use its `SymbolPaintProperties paint` member to track layer property state, but it provides helpers that construct objects of each the specific `{Icon,Text}PaintProperties::Evaluated` type, for use by the painter. - The three symbol programs instantiate `Program<>` using the appropriate `{Icon,Text}PaintProperties` type.

anandthakker force-pushed the dds-symbol branch 2 times, most recently from 5a6df6d to 9a2d6e0 Compare February 17, 2017 21:49

anandthakker changed the title ~~WIP - dds for {text,icon}-{color,halo-color}~~ WIP - dds for {text,icon}-{color,halo-color,halo-blur,halo-width} Feb 17, 2017

anandthakker changed the title ~~WIP - dds for {text,icon}-{color,halo-color,halo-blur,halo-width}~~ WIP - dds for {text,icon}-{color,opacity,halo-color,halo-blur,halo-width} Feb 17, 2017

anandthakker added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Feb 17, 2017

anandthakker changed the title ~~WIP - dds for {text,icon}-{color,opacity,halo-color,halo-blur,halo-width}~~ [core] Implement data-driven styling for {text,icon}-{color,opacity,halo-color,halo-blur,halo-width} Feb 17, 2017

anandthakker added Core The cross-platform C++ core, aka mbgl GL JS parity For feature parity with Mapbox GL JS labels Feb 17, 2017

anandthakker commented Feb 20, 2017

View reviewed changes

anandthakker mentioned this pull request Feb 20, 2017

Enable native integration tests for symbol DDS properties mapbox/mapbox-gl-js#4300

Merged

anandthakker requested a review from jfirebaugh February 20, 2017 19:54

anandthakker force-pushed the dds-symbol branch from e2d307e to 1400ed8 Compare February 20, 2017 20:13

anandthakker commented Feb 20, 2017

View reviewed changes

anandthakker force-pushed the dds-symbol branch from 13d1d30 to dcacc17 Compare February 21, 2017 18:31

jfirebaugh suggested changes Feb 23, 2017

View reviewed changes

anandthakker commented Feb 23, 2017

View reviewed changes

jfirebaugh suggested changes Feb 23, 2017

View reviewed changes

anandthakker force-pushed the dds-symbol branch from 9a44bd0 to 32a6d22 Compare February 24, 2017 01:07

anandthakker mentioned this pull request Feb 24, 2017

Reduce the number of vertex attributes used for data-driven styling. #8183

Closed

anandthakker force-pushed the dds-symbol branch from 32a6d22 to 7d78614 Compare February 27, 2017 23:32

jfirebaugh mentioned this pull request Feb 27, 2017

Reduce memory requirements for symbol layers #8212

Merged

jfirebaugh suggested changes Feb 28, 2017

View reviewed changes

anandthakker force-pushed the dds-symbol branch from def4a1e to c99444a Compare February 28, 2017 22:04

Anand Thakker and others added 8 commits February 28, 2017 14:06

Update to mapbox-gl-js/master

285e1c3

Add symbol dds attributes and adapt style code generation

8bf3b55

check in generated style code

f2b2ba4

Refactor SymbolFeature as a subclass of GeometryTileFeature

656ac13

Prepares for enabling DDS on symbol paint properties by allowing the SymbolFeatures, which we keep around after constructing SymbolLayout, to be used in evaluating data-driven paint properties later in the layout process.

Populate paint buffers for symbol DDS properties

c9c5875

Address first round of review comments

bcbf716

Refactor VectorTile{Layer,Feature} to explicitly share data

7a7c2ae

anandthakker force-pushed the dds-symbol branch from c99444a to 7a7c2ae Compare February 28, 2017 22:07

jfirebaugh approved these changes Feb 28, 2017

View reviewed changes

boundsj mentioned this pull request Feb 28, 2017

Per-attribute transition properties on MGLStyleLayer #8225

Merged

Update submodule

9206635

anandthakker merged commit f901e77 into master Mar 1, 2017

anandthakker deleted the dds-symbol branch March 1, 2017 03:54

anandthakker mentioned this pull request Mar 1, 2017

Recover some headroom under the Memory.Footprint ceiling #8247

Closed

1ec5 removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label May 6, 2017

springmeyer mentioned this pull request Jun 1, 2017

Use move semantics rather than shared_ptr for Vector Tile data #9167

Closed

		@@ -15,9 +15,17 @@ class VectorTileLayer;

		using packed_iter_type = protozero::iterator_range<protozero::pbf_reader::const_uint32_iterator>;

		struct VectorTileLayerData {

[core] Implement data-driven styling for {text,icon}-{color,opacity,halo-color,halo-blur,halo-width} #7939

[core] Implement data-driven styling for {text,icon}-{color,opacity,halo-color,halo-blur,halo-width} #7939

Conversation

anandthakker commented Feb 3, 2017

mention-bot commented Feb 3, 2017

anandthakker commented Feb 17, 2017 • edited Loading

jfirebaugh commented Feb 17, 2017

anandthakker commented Feb 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anandthakker commented Feb 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anandthakker commented Feb 21, 2017 • edited Loading

jfirebaugh commented Feb 21, 2017

anandthakker commented Feb 21, 2017 • edited Loading

anandthakker commented Feb 22, 2017

tmpsantos commented Feb 22, 2017

tmpsantos commented Feb 22, 2017

anandthakker commented Feb 22, 2017

tmpsantos commented Feb 22, 2017

anandthakker commented Feb 22, 2017

anandthakker commented Feb 22, 2017

jfirebaugh commented Feb 22, 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

anandthakker commented Feb 23, 2017

Choose a reason for hiding this comment

anandthakker commented Feb 23, 2017 • edited Loading

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

anandthakker commented Feb 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

springmeyer Apr 5, 2017 • edited Loading

Choose a reason for hiding this comment

anandthakker Apr 5, 2017 • edited Loading

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

anandthakker commented Feb 28, 2017

anandthakker commented Feb 17, 2017 •

edited

Loading

anandthakker commented Feb 21, 2017 •

edited

Loading

anandthakker commented Feb 21, 2017 •

edited

Loading

anandthakker commented Feb 23, 2017 •

edited

Loading

anandthakker commented Feb 24, 2017 •

edited

Loading

springmeyer Apr 5, 2017 •

edited

Loading

anandthakker Apr 5, 2017 •

edited

Loading