Fix LocationIndicator not updating image sizes when image manager content changesupstream/galinelle_locationIndicatorSizes

4 files changed, 133 insertions, 20 deletions

diff --git a/metrics/ignores/platform-linux.json b/metrics/ignores/platform-linux.json
index 29d06bfc3d..ec018eb805 100644
--- a/metrics/ignores/platform-linux.json
+++ b/metrics/ignores/platform-linux.json
@@ -21,5 +21,6 @@
     "location_indicator/tilted_texture_shift_top_left": "Would need a different baseline when ran on llvmpipe not supporting anisotropic filtering",
     "location_indicator/tilted_texture_shift_top_right": "Would need a different baseline when ran on llvmpipe not supporting anisotropic filtering",
     "location_indicator/two_textures": "Would need a different baseline when ran on llvmpipe not supporting anisotropic filtering",
-    "location_indicator/image_pixel_ratio": "Would need a different baseline when ran on llvmpipe not supporting anisotropic filtering"
+    "location_indicator/image_pixel_ratio": "Would need a different baseline when ran on llvmpipe not supporting anisotropic filtering",
+    "location_indicator/change_image": "Would need a different baseline when ran on llvmpipe not supporting anisotropic filtering"
 }
diff --git a/metrics/tests/location_indicator/change_image/expected.png b/metrics/tests/location_indicator/change_image/expected.png
new file mode 100644
index 0000000000..ffd0542078
--- /dev/null
+++ b/metrics/tests/location_indicator/change_image/expected.png
diff --git a/metrics/tests/location_indicator/change_image/style.json b/metrics/tests/location_indicator/change_image/style.json
new file mode 100644
index 0000000000..7aed158bc5
--- /dev/null
+++ b/metrics/tests/location_indicator/change_image/style.json
@@ -0,0 +1,104 @@
+{
+  "version": 8,
+  "metadata": {
+    "test": {
+      "width": 512,
+      "height": 256,
+      "operations": [
+        [
+          "addImage",
+          "puck_hat",
+          "puck_hat.png",
+          {"pixelRatio": 2.0}
+        ],
+        [
+          "addImage",
+          "puck",
+          "puck.png",
+          {"pixelRatio": 2.0}
+
+        ],
+        [
+          "addImage",
+          "puck_shadow",
+          "puck_shadow.png",
+          {"pixelRatio": 2.0}
+        ],
+        [ "sleep", 500 ],
+        [
+          "addImage",
+          "puck_hat",
+          "puck_hat.png",
+          {"pixelRatio": 4.0}
+        ]
+      ]
+    }
+  },
+  "sources": {
+    "geojson": {
+      "type": "geojson",
+      "data": {
+        "type": "FeatureCollection",
+        "features": [
+          {
+            "type": "Feature",
+            "properties": {
+            },
+            "geometry": {
+              "type": "Point",
+              "coordinates": [
+              139.766707,
+                35.693055
+
+              ]
+            }
+          }
+        ]
+      }
+    }
+  },
+  "center": [ 139.766707, 35.693055 ],
+  "zoom": 16,
+  "pitch" : 0,
+  "bearing" : 42,
+  "layers": [
+    {
+      "id": "background",
+      "type": "background",
+      "paint": {
+        "background-color": "lightseagreen"
+      }
+    },
+    {
+      "id": "puck123",
+      "type": "circle",
+      "source": "geojson",
+      "paint": {
+        "circle-radius": 40,
+        "circle-color" : "rgba(255,0,0,0.1)"
+      }
+    },
+    {
+      "id": "puck124",
+      "type": "location-indicator",
+      "layout" : {
+          "bearing-image" : "puck",
+          "top-image" : "puck_hat",
+          "shadow-image" : "puck_shadow"
+      },
+      "paint" : {
+          "bearing" : 45,
+          "perspective-compensation" : 1,
+          "image-tilt-displacement" : 6.5,
+          "location" : [ 35.693055, 139.766707, 0],
+          "accuracy-radius": 90.0,
+          "bearing-image-size" : 0.52,
+          "top-image-size" : 0.36,
+          "shadow-image-size" : 0.4,
+
+          "accuracy-radius-color": "rgba(255,0,0,0.2)",
+          "accuracy-radius-border-color": "rgba(255,0,255,0.6)"
+      }
+    }
+  ]
+}
diff --git a/src/mbgl/renderer/layers/render_location_indicator_layer.cpp b/src/mbgl/renderer/layers/render_location_indicator_layer.cpp
index f3d72f02d1..99eef054f9 100644
--- a/src/mbgl/renderer/layers/render_location_indicator_layer.cpp
+++ b/src/mbgl/renderer/layers/render_location_indicator_layer.cpp
@@ -272,12 +272,11 @@ public:
             Assign can be called any time. Conversely, upload must be called with a bound gl context.
         */
         void assign(const Immutable<style::Image::Impl>* img) {
-            if ((img && &img->get()->image == image) || (!img && !image)) return;
             imageDirty = true;
-            image = (img) ? &img->get()->image : nullptr;
+            image = (img) ? &(img->get()->image) : nullptr;
             width = height = 0;
             pixelRatio = 1.0f;
-            if (img) {
+            if (image) {
                 sharedImage = *img; // keep reference until uploaded
                 width = image->size.width;
                 height = image->size.height;
@@ -293,7 +292,7 @@ public:
             initialize();
 
             MBGL_CHECK_ERROR(glBindTexture(GL_TEXTURE_2D, texId));
-            if (!image || !image->valid()) {
+            if (!sharedImage || !image || !image->valid()) {
                 MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 0, 0, 0, GL_RGBA, GL_UNSIGNED_BYTE, nullptr));
             } else {
                 MBGL_CHECK_ERROR(glTexImage2D(GL_TEXTURE_2D,
@@ -399,9 +398,9 @@ public:
                    params.puckShadowScale != oldParams.puckShadowScale)
             bearingChanged = true; // changes puck geometry but not necessarily the location
         if (params.errorRadiusMeters != oldParams.errorRadiusMeters) radiusChanged = true;
-        setTextureFromImageID(params.puckImagePath, texPuck, params);
-        setTextureFromImageID(params.puckShadowImagePath, texShadow, params);
-        setTextureFromImageID(params.puckHatImagePath, texPuckHat, params);
+        bearingChanged |= setTextureFromImageID(params.puckImagePath, texPuck, params);
+        bearingChanged |= setTextureFromImageID(params.puckShadowImagePath, texShadow, params);
+        bearingChanged |= setTextureFromImageID(params.puckHatImagePath, texPuckHat, params);
 
         projectionCircle = params.projectionMatrix;
         const Point<double> positionMercator = project(params.puckPosition, *params.state);
@@ -514,7 +513,10 @@ protected:
         return vec2(posScreenDelta - posScreen).normalized();
     }
 
-    void updatePuck(const mbgl::LocationIndicatorRenderParameters& params) { return updatePuckPerspective(params); }
+    void updatePuck(const mbgl::LocationIndicatorRenderParameters& params) {
+        updatePuckPerspective(params);
+        bearingChanged = false;
+    }
 
     void updatePuckPerspective(const mbgl::LocationIndicatorRenderParameters& params) {
         const TransformState& s = *params.state;
@@ -572,8 +574,6 @@ protected:
             hatGeometry[i] =
                 vec2(hatOffset + (verticalShift * (tilt * params.puckLayersDisplacement * horizontalScaleFactor)));
         }
-
-        bearingChanged = false;
     }
 
     void drawRadius(const mbgl::LocationIndicatorRenderParameters& params) {
@@ -636,23 +636,31 @@ protected:
         return s.screenCoordinateToLatLng(flippedPoint, wrapMode);
     }
 
-    void setTextureFromImageID(const std::string& imagePath,
-                               std::shared_ptr<Texture>& tex,
+    bool setTextureFromImageID(const std::string& imagePath,
+                               std::shared_ptr<Texture>& texture,
                                const mbgl::LocationIndicatorRenderParameters& params) {
+        bool updated = false;
         if (textures.find(imagePath) == textures.end()) {
             std::shared_ptr<Texture> tx = std::make_shared<Texture>();
-            if (!imagePath.empty() && params.imageManager)
+            if (!imagePath.empty() && params.imageManager) {
                 tx->assign(params.imageManager->getSharedImage(imagePath));
-            else
+                updated = true;
+            } else {
                 tx->assign(nullptr);
+            }
             textures[imagePath] = tx;
+            texture = tx;
         } else {
-            const Immutable<style::Image::Impl>* ima = params.imageManager->getSharedImage(imagePath);
-            const mbgl::PremultipliedImage* img = (ima) ? &ima->get()->image : nullptr;
-            if (textures.at(imagePath)->image != img) // image for the ID might have changed.
-                textures.at(imagePath)->assign(ima);
+            const Immutable<style::Image::Impl>* sharedImage = params.imageManager->getSharedImage(imagePath);
+            const mbgl::PremultipliedImage* img = (sharedImage) ? &sharedImage->get()->image : nullptr;
+            std::shared_ptr<Texture>& tex = textures.at(imagePath);
+            if (tex->image != img) { // image for the ID might have changed.
+                tex->assign(sharedImage);
+                updated = true;
+            }
+            texture = tex;
         }
-        tex = textures.at(imagePath);
+        return updated;
     }
 
     std::map<std::string, std::shared_ptr<Texture>> textures;