Dense image maps

metrics/android-render-test-runner/render-tests/icon-image/icon-sdf-non-sdf-one-layer/metrics.json

@@ -19,8 +19,8 @@
             13,
             1,
             [
-                26720,
-                26720
+                26600,
+                26600
             ],
             [
                 46,
@@ -32,4 +32,4 @@
             ]
         ]
     ]
-}
+}

platform/android/DEVELOPING.md

@@ -55,7 +55,7 @@ The instrumentation tests are running on AWS Device Farm. To see the results and
 
 https://us-west-2.console.aws.amazon.com/devicefarm/home?region=us-east-1#/mobile/projects/20687d72-0e46-403e-8f03-0941850665bc/runs
 
-You can log with the `maplibre` alias, with `maplibre` as username and `maplibre` as password (this is a read-only account).
+You can log in with the `maplibre` alias, with `maplibre` as username and `maplibre` as password (this is a read-only account).
 
 ## Kotlin
 
@@ -75,4 +75,4 @@ To format all Kotlin source files, use:
 
 ```
 $ ./gradlew formatKotlin
-```
+```

render-test/manifest_parser.cpp

@@ -287,7 +287,31 @@ std::optional<Manifest> ManifestParser::parseManifest(const std::string& manifes
             testId = testId.substr(rootLength + 1, testId.length() - rootLength - 1);
 
             std::vector<mbgl::filesystem::path> expectedMetricPaths{expectedMetricPath};
-#if defined(__APPLE__)
+#if defined(__ANDROID__)
+            // todo: use `Context.getExternalFilesDir()` or similar via JNI to select an appropriate destination
+            const auto locations = std::vector<std::string>{
+                "/sdcard",
+                "/storage/emulated/0",
+                "/storage/self/primary",
+            };
+            static bool reportedOnce = false;
+            for (const auto& location : locations) {
+                // Checking `mbgl::filesystem::status` doesn't accurately reflect whether we can create subdirectories,
+                // so just try it. (See `TestRunner::checkProbingResults`)
+                try {
+                    const auto baselinesPath = location + "/baselines";
+                    mbgl::filesystem::create_directories(baselinesPath);
+                    expectedMetricPaths.emplace_back(baselinesPath);
+                    break;
+                } catch (mbgl::filesystem::filesystem_error& ex) {
+                    if (!reportedOnce) {
+                        mbgl::Log::Warning(mbgl::Event::Android, "Not a writable directory: " + std::string(ex.what()));
+                    }
+                }
+            }
+            // Only log on the first case
+            reportedOnce = true;
+#elif defined(__APPLE__)
             expectedMetricPaths.emplace_back(manifest.manifestPath + "/baselines/");
 #endif
             testPaths.emplace_back(testPath,

render-test/runner.cpp

@@ -11,6 +11,7 @@
 #include <mbgl/util/chrono.hpp>
 #include <mbgl/util/image.hpp>
 #include <mbgl/util/io.hpp>
+#include <mbgl/util/logging.hpp>
 #include <mbgl/util/projection.hpp>
 #include <mbgl/util/run_loop.hpp>
 #include <mbgl/util/string.hpp>
@@ -285,9 +286,16 @@ void TestRunner::checkProbingResults(TestMetadata& resultMetadata) {
     if (resultMetadata.metrics.isEmpty()) return;
     const auto writeMetrics = [&resultMetadata](const mbgl::filesystem::path& path,
                                                 const std::string& message = std::string()) {
-        mbgl::filesystem::create_directories(path);
-        mbgl::util::write_file(path / "metrics.json", serializeMetrics(resultMetadata.metrics));
-        resultMetadata.errorMessage += message;
+        try {
+            mbgl::filesystem::create_directories(path);
+            mbgl::util::write_file(path / "metrics.json", serializeMetrics(resultMetadata.metrics));
+            resultMetadata.errorMessage += message;
+        } catch (mbgl::filesystem::filesystem_error& ex) {
+            const auto msg = "Failed to write metrics. path='" + path.string() + "' err='" + ex.what() +
+                             "' msg=" + message;
+            resultMetadata.errorMessage += msg;
+            Log::Warning(Event::General, msg);
+        }
     };
 
     const std::vector<mbgl::filesystem::path>& expectedMetrics = resultMetadata.paths.expectedMetrics;

src/mbgl/layout/layout.hpp

@@ -25,9 +25,9 @@ class Layout {
                               bool,
                               const CanonicalTileID&) = 0;
 
-    virtual void prepareSymbols(const GlyphMap&, const GlyphPositions&, const ImageMap&, const ImagePositions&) {};
+    virtual void prepareSymbols(const GlyphMap&, const GlyphPositions&, const ImageMap&, const ImagePositions&) {}
 
-    virtual bool hasSymbolInstances() const { return true; };
+    virtual bool hasSymbolInstances() const { return true; }
 
     virtual bool hasDependencies() const = 0;
 };

src/mbgl/renderer/image_atlas.cpp

@@ -46,15 +46,18 @@ namespace {
 void populateImagePatches(ImagePositions& imagePositions,
                           const ImageManager& imageManager,
                           std::vector<ImagePatch>& /*out*/ patches) {
+    if (imagePositions.empty()) {
+        imagePositions.reserve(imageManager.updatedImageVersions.size());
+    }
     for (auto& updatedImageVersion : imageManager.updatedImageVersions) {
         const std::string& name = updatedImageVersion.first;
         const uint32_t version = updatedImageVersion.second;
-        auto it = imagePositions.find(updatedImageVersion.first);
+        const auto it = imagePositions.find(updatedImageVersion.first);
         if (it != imagePositions.end()) {
             auto& position = it->second;
             if (position.version == version) continue;
 
-            auto updatedImage = imageManager.getSharedImage(name);
+            const auto updatedImage = imageManager.getSharedImage(name);
             if (updatedImage == nullptr) continue;
 
             patches.emplace_back(*updatedImage, position.paddedRect);
@@ -72,28 +75,30 @@ std::vector<ImagePatch> ImageAtlas::getImagePatchesAndUpdateVersions(const Image
     return imagePatches;
 }
 
-ImageAtlas makeImageAtlas(const ImageMap& icons,
-                          const ImageMap& patterns,
-                          const std::unordered_map<std::string, uint32_t>& versionMap) {
+ImageAtlas makeImageAtlas(const ImageMap& icons, const ImageMap& patterns, const ImageVersionMap& versionMap) {
     ImageAtlas result;
 
     mapbox::ShelfPack::ShelfPackOptions options;
     options.autoResize = true;
     mapbox::ShelfPack pack(0, 0, options);
 
+    result.iconPositions.reserve(icons.size());
+
     for (const auto& entry : icons) {
         const style::Image::Impl& image = *entry.second;
         const mapbox::Bin& bin = _packImage(pack, image, result, ImageType::Icon);
-        auto it = versionMap.find(entry.first);
-        auto version = it != versionMap.end() ? it->second : 0;
+        const auto it = versionMap.find(entry.first);
+        const auto version = it != versionMap.end() ? it->second : 0;
         result.iconPositions.emplace(image.id, ImagePosition{bin, image, version});
     }
 
+    result.patternPositions.reserve(patterns.size());
+
     for (const auto& entry : patterns) {
         const style::Image::Impl& image = *entry.second;
         const mapbox::Bin& bin = _packImage(pack, image, result, ImageType::Pattern);
-        auto it = versionMap.find(entry.first);
-        auto version = it != versionMap.end() ? it->second : 0;
+        const auto it = versionMap.find(entry.first);
+        const auto version = it != versionMap.end() ? it->second : 0;
         result.patternPositions.emplace(image.id, ImagePosition{bin, image, version});
     }
 

src/mbgl/renderer/image_atlas.hpp

@@ -52,7 +52,7 @@ class ImagePosition {
     }
 };
 
-using ImagePositions = std::map<std::string, ImagePosition>;
+using ImagePositions = mbgl::unordered_map<std::string, ImagePosition>;
 
 class ImagePatch {
 public:
@@ -73,8 +73,6 @@ class ImageAtlas {
     std::vector<ImagePatch> getImagePatchesAndUpdateVersions(const ImageManager&);
 };
 
-ImageAtlas makeImageAtlas(const ImageMap&,
-                          const ImageMap&,
-                          const std::unordered_map<std::string, uint32_t>& versionMap);
+ImageAtlas makeImageAtlas(const ImageMap&, const ImageMap&, const ImageVersionMap& versionMap);
 
 } // namespace mbgl

src/mbgl/renderer/image_manager.cpp

@@ -285,6 +285,10 @@ void ImageManager::notify(ImageRequestor& requestor, const ImageRequestPair& pai
     ImageMap patternMap;
     ImageVersionMap versionMap;
 
+    iconMap.reserve(pair.first.size());
+    patternMap.reserve(pair.first.size());
+    versionMap.reserve(pair.first.size());
+
     for (const auto& dependency : pair.first) {
         auto it = images.find(dependency.first);
         if (it != images.end()) {

src/mbgl/renderer/image_manager.hpp

@@ -5,6 +5,7 @@
 
 #include <map>
 #include <mutex>
+#include <set>
 #include <string>
 
 namespace mbgl {

src/mbgl/style/image_impl.hpp

@@ -1,10 +1,9 @@
 #pragma once
 
 #include <mbgl/style/image.hpp>
+#include <mbgl/util/containers.hpp>
 
 #include <string>
-#include <unordered_map>
-#include <set>
 #include <optional>
 
 namespace mbgl {
@@ -45,10 +44,10 @@ enum class ImageType : bool {
     Pattern
 };
 
-using ImageMap = std::unordered_map<std::string, Immutable<style::Image::Impl>>;
-using ImageDependencies = std::unordered_map<std::string, ImageType>;
+using ImageMap = mbgl::unordered_map<std::string, Immutable<style::Image::Impl>>;
+using ImageDependencies = mbgl::unordered_map<std::string, ImageType>;
 using ImageRequestPair = std::pair<ImageDependencies, uint64_t>;
-using ImageVersionMap = std::unordered_map<std::string, uint32_t>;
+using ImageVersionMap = mbgl::unordered_map<std::string, uint32_t>;
 inline bool operator<(const Immutable<mbgl::style::Image::Impl>& a, const Immutable<mbgl::style::Image::Impl>& b) {
     return a->id < b->id;
 }

test/renderer/image_manager.test.cpp

@@ -86,12 +86,12 @@ class StubImageRequestor : public ImageRequestor {
 
     void onImagesAvailable(ImageMap icons,
                            ImageMap patterns,
-                           std::unordered_map<std::string, uint32_t> versionMap,
+                           ImageVersionMap versionMap,
                            uint64_t imageCorrelationID_) final {
         if (imagesAvailable && imageCorrelationID == imageCorrelationID_) imagesAvailable(icons, patterns, versionMap);
     }
 
-    std::function<void(ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>)> imagesAvailable;
+    std::function<void(ImageMap, ImageMap, ImageVersionMap)> imagesAvailable;
     uint64_t imageCorrelationID = 0;
 };
 
@@ -105,7 +105,7 @@ TEST(ImageManager, NotifiesRequestorWhenSpriteIsLoaded) {
     ImageManagerObserver observer;
     imageManager.setObserver(&observer);
 
-    requestor.imagesAvailable = [&](ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>) {
+    requestor.imagesAvailable = [&](ImageMap, ImageMap, ImageVersionMap) {
         notified = true;
     };
 
@@ -131,7 +131,7 @@ TEST(ImageManager, NotifiesRequestorImmediatelyIfDependenciesAreSatisfied) {
     StubImageRequestor requestor(imageManagerPtr);
     bool notified = false;
 
-    requestor.imagesAvailable = [&](ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>) {
+    requestor.imagesAvailable = [&](ImageMap, ImageMap, ImageVersionMap) {
         notified = true;
     };
 
@@ -173,7 +173,7 @@ TEST(ImageManager, OnStyleImageMissingBeforeSpriteLoaded) {
 
     bool notified = false;
 
-    requestor.imagesAvailable = [&](ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>) {
+    requestor.imagesAvailable = [&](ImageMap, ImageMap, ImageVersionMap) {
         notified = true;
     };
 
@@ -229,7 +229,7 @@ TEST(ImageManager, OnStyleImageMissingAfterSpriteLoaded) {
 
     bool notified = false;
 
-    requestor.imagesAvailable = [&](ImageMap, ImageMap, std::unordered_map<std::string, uint32_t>) {
+    requestor.imagesAvailable = [&](ImageMap, ImageMap, ImageVersionMap) {
         notified = true;
     };