Feature state updates #15872

jmalanen · 2019-10-30T15:15:38Z

[glfw] Update feature state only when the layer is visible
-- small optimization to the glfw test app

[core] Add feature state support to composite crossfade binder
-- This adds feature state support for eg. fill-pattern
-- Related gl-js PR: mapbox/mapbox-gl-js#8927

[core] Add getMissingImages API to GeometryTile
[core] Pass GeometryTile to paint property binder
-- These two are requirements for the above composite crossfade binder patch

[core] Upload buckets only when there are feature state changes
-- Optimization

Fixes: #15895

julianrex · 2019-10-31T13:15:38Z

platform/darwin/scripts/generate-style-code.js

@@ -159,7 +159,7 @@ global.objCTestValue = function (property, layerType, arraysAsStructs, indent) {
                `@"'${_.startCase(propertyName)}'"` :
                `@"${_.startCase(propertyName)}"`;
        case 'string':
-        case 'image': // TODO: replace once we implement image expressions
+        case 'resolvedImage': // TODO: replace once we implement image expressions


We'll want to make these changes in the https://github.com/mapbox/mapbox-gl-native-ios repo.

Similarly the Android ones in https://github.com/mapbox/mapbox-gl-native-android. Correct @tobrun?

yes ticket in mapbox/mapbox-gl-native-android#33

alexshalamov · 2019-11-01T09:57:27Z

src/mbgl/tile/geometry_tile.cpp

+    if (!missingImagesRequested) {
+        worker.self().invoke(&GeometryTileWorker::onImagesAvailable,
+                             std::move(images),
+                             std::move(patterns),
+                             std::move(versionMap),
+                             imageCorrelationID);
+    } else {
+        layoutResult->iconAtlas = makeImageAtlas(images, patterns, versionMap);
+        missingImagesRequested = false;
+    }


This needs a unit test, if for some reason missingImagesRequested is stale and set to true, we will never trigger re-layout when image manager provides requested images.

pozdnyakov · 2019-11-01T09:48:45Z

platform/glfw/glfw_view.cpp

+    using namespace mbgl;
+    FeatureState newState;
+
+    if (result.size() > 0) {


nit: !result.empty()

pozdnyakov · 2019-11-01T09:51:10Z

platform/glfw/glfw_view.cpp

+            }
+        }
+    } else {
+        if (view->featureID) {


nit: else if

pozdnyakov · 2019-11-01T09:54:59Z

src/mbgl/renderer/bucket.hpp

@@ -50,7 +52,9 @@ class Bucket {
    bool needsUpload() const {
        return hasData() && !uploaded;
    }
-
+
+    virtual bool uploadPending() const { return needsUpload() || stateChanged; }


why couldn't we just check stateChanged insideneedsUpload()?

pozdnyakov · 2019-11-01T09:58:00Z

src/mbgl/renderer/buckets/fill_extrusion_bucket.cpp

    }
+    stateChanged = updated;


I see this snippet being repeated several times, can it be shared?

maybe put to a basic class?

pozdnyakov · 2019-11-01T10:01:55Z

src/mbgl/renderer/paint_property_binder.hpp

@@ -109,9 +110,19 @@ class PaintPropertyBinder {
                                      const ImagePositions&, const optional<PatternDependency>&,
                                      const style::expression::Value&) = 0;

-    virtual void updateVertexVectors(const FeatureStates&, const GeometryTileLayer&, const ImagePositions&) {}
+    virtual bool updateVertexVectors(const FeatureStates&,


pls add a comment documenting the returned value

pozdnyakov · 2019-11-01T10:21:20Z

src/mbgl/renderer/paint_property_binder.hpp

+    virtual bool updateVertexVectors(const FeatureStates&,
+                                     const GeometryTileLayer&,
+                                     const ImagePositions&,
+                                     GeometryTile&) {


Let's not pass tile here, it's better to keep it decoupled from rendering

chloekraw · 2019-11-21T17:56:17Z

@jmalanen does this need a changelog, or is it all internal code so far that doesn't impact developers until we introduce new APIs?

jmalanen · 2019-11-22T08:47:48Z

@jmalanen does this need a changelog, or is it all internal code so far that doesn't impact developers until we introduce new APIs?

This is all internal code for now that doesn't impact developers.

chapmanjacobd · 2020-01-24T04:01:26Z

is there anything that an outside contributor can do to help push this along? I would like to use this feature in mapbox/mapbox-gl-js#7207

jplante · 2020-03-13T17:42:09Z

Our team is very eager for this release as well.

jmurphy-riskexec · 2020-03-13T17:45:11Z

I'm stuck on an old fork of GL JS waiting for this, I'd love to see this get moving again.

derrickrung · 2020-04-24T00:42:48Z

Thanks for all the great work. Super excited about this PR, our team has been looking forward to this for a while. Anything we can help do to get this moved along?

jmalanen · 2020-04-24T10:05:43Z

@chapmanjacobd @jmurphy-asurity @jplante @derrickrung

Which one of you are interested in the GL native version?

This is currently on hold, but if you are interested in GL JS please add your request to mapbox/mapbox-gl-js#7207

jmurphy-riskexec · 2020-04-24T13:58:48Z

Thanks for the update @jmalanen. I'm interested in both, but GL JS is more important to me. Why is this on hold? It seemed like it was nearly ready.

jmalanen force-pushed the jmalanen-feature branch 5 times, most recently from 5fe6a4a to d1ea075 Compare October 31, 2019 11:41

jmalanen mentioned this pull request Oct 31, 2019

Update fill-pattern feature state test mapbox/mapbox-gl-js#8927

Merged

jmalanen force-pushed the jmalanen-feature branch from d1ea075 to 17b4361 Compare October 31, 2019 13:03

jmalanen marked this pull request as ready for review October 31, 2019 13:05

jmalanen requested a review from 1ec5 as a code owner October 31, 2019 13:05

jmalanen requested review from a team, pozdnyakov, alexshalamov and tmpsantos October 31, 2019 13:05

jmalanen self-assigned this Oct 31, 2019

julianrex reviewed Oct 31, 2019

View reviewed changes

julianrex mentioned this pull request Oct 31, 2019

Update style generator mapbox/mapbox-gl-native-ios#46

Closed

alexshalamov reviewed Nov 1, 2019

View reviewed changes

pozdnyakov reviewed Nov 1, 2019

View reviewed changes

jmalanen force-pushed the jmalanen-feature branch 2 times, most recently from 0d3b0ab to b0426ff Compare November 1, 2019 16:02

alexshalamov mentioned this pull request Nov 7, 2019

Fix feature-state dependent styles for *-pattern properties #15895

Open

Juha Alanen added 3 commits November 20, 2019 15:25

[core] Upload buckets only when there are feature state changes

07b1b79

[core] Pass GeometryTile to paint property binder

2b5ffce

[core] Add getMissingImages API to GeometryTile

6300554

jmalanen force-pushed the jmalanen-feature branch from b0426ff to 51d2589 Compare November 21, 2019 10:28

Juha Alanen added 3 commits November 21, 2019 12:31

[core] Add feature state support to composite crossfade binder

18bf745

[glfw] Update feature state only when the layer is visible

2ae8e17

[render-test] Enable fill-pattern/update-feature-state test

a45b876

jmalanen force-pushed the jmalanen-feature branch from 51d2589 to a45b876 Compare November 21, 2019 10:34

asheemmamoowala mentioned this pull request Dec 11, 2019

Fix feature-state dependent styles for *-pattern properties mapbox/mapbox-gl-js#7207

Open

chloekraw added this to the release-unicorn milestone Dec 18, 2019

jmalanen removed request for tmpsantos and 1ec5 January 31, 2020 07:32

jmalanen closed this Jul 7, 2022

jmalanen deleted the jmalanen-feature branch July 7, 2022 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature state updates #15872

Feature state updates #15872

jmalanen commented Oct 30, 2019 •

edited

Loading

julianrex Oct 31, 2019

tobrun Oct 31, 2019

alexshalamov Nov 1, 2019

pozdnyakov Nov 1, 2019

jmalanen Nov 1, 2019

pozdnyakov Nov 1, 2019

jmalanen Nov 1, 2019

pozdnyakov Nov 1, 2019

jmalanen Nov 1, 2019

pozdnyakov Nov 1, 2019

pozdnyakov Nov 1, 2019

pozdnyakov Nov 1, 2019

pozdnyakov Nov 1, 2019

chloekraw commented Nov 21, 2019

jmalanen commented Nov 22, 2019

chapmanjacobd commented Jan 24, 2020

jplante commented Mar 13, 2020

jmurphy-riskexec commented Mar 13, 2020

derrickrung commented Apr 24, 2020 •

edited

Loading

jmalanen commented Apr 24, 2020

jmurphy-riskexec commented Apr 24, 2020

Feature state updates #15872

Feature state updates #15872

Conversation

jmalanen commented Oct 30, 2019 • 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

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

chloekraw commented Nov 21, 2019

jmalanen commented Nov 22, 2019

chapmanjacobd commented Jan 24, 2020

jplante commented Mar 13, 2020

jmurphy-riskexec commented Mar 13, 2020

derrickrung commented Apr 24, 2020 • edited Loading

jmalanen commented Apr 24, 2020

jmurphy-riskexec commented Apr 24, 2020

jmalanen commented Oct 30, 2019 •

edited

Loading

derrickrung commented Apr 24, 2020 •

edited

Loading