From b83030aa9bb1a8d9f14ae8160698c1ee4d5a4c72 Mon Sep 17 00:00:00 2001
From: Mikhail Pozdnyakov <mikhail.pozdnyakov@mapbox.com>
Date: Fri, 18 Jan 2019 13:12:04 +0200
Subject: [core] Remove tile sorting from the clip and mask algorithms

The tile sorting can be now removed from the algorithms, which
calculate tile mask and clip ids, because their client code
provides tiles being already sorted (in `TilePyramid`).

This patch brings significant improvements to the Tile-related
performance tests results, for example the `TileMaskGeneration`
benchmark test runs 33 times faster with these changes applied.
---
 test/algorithm/generate_clip_ids.test.cpp | 21 +++++++++++++++------
 test/algorithm/update_tile_masks.test.cpp |  5 ++++-
 2 files changed, 19 insertions(+), 7 deletions(-)

(limited to 'test/algorithm')

diff --git a/test/algorithm/generate_clip_ids.test.cpp b/test/algorithm/generate_clip_ids.test.cpp
index f01c2da80a..d918514e51 100644
--- a/test/algorithm/generate_clip_ids.test.cpp
+++ b/test/algorithm/generate_clip_ids.test.cpp
@@ -24,6 +24,15 @@ struct Renderable {
     }
 };
 
+namespace {
+auto makeSorted(std::vector<Renderable>& renderables) {
+    std::vector<std::reference_wrapper<Renderable>> sorted(renderables.begin(), renderables.end());
+    std::sort(sorted.begin(), sorted.end(),
+                  [](const Renderable& a, const Renderable& b){ return a.id < b.id; });
+    return sorted;
+}
+}  // namespace
+
 ::std::ostream& operator<<(::std::ostream& os, const Renderable& rhs) {
     return os << "Renderable{ " << rhs.id <<  ", " << rhs.clip << " }";
 }
@@ -71,7 +80,7 @@ TEST(GenerateClipIDs, ParentAndFourChildrenNegative) {
     };
 
     algorithm::ClipIDGenerator generator;
-    generator.update<Renderable>({ renderables.begin(), renderables.end() });
+    generator.update<Renderable>(makeSorted(renderables));
 
     EXPECT_EQ(decltype(renderables)({
                   Renderable{ UnwrappedTileID{ 1, -2, 0 }, ClipID{ "00000111", "00000010" } },
@@ -102,7 +111,7 @@ TEST(GenerateClipIDs, NegativeParentAndMissingLevel) {
     };
 
     algorithm::ClipIDGenerator generator;
-    generator.update<Renderable>({ renderables.begin(), renderables.end() });
+    generator.update<Renderable>(makeSorted(renderables));
 
     EXPECT_EQ(decltype(renderables)({
                   Renderable{ UnwrappedTileID{ 1, -1, 0 }, ClipID{ "00000111", "00000001" } },
@@ -186,7 +195,7 @@ TEST(GenerateClipIDs, MultipleLevels) {
     };
 
     algorithm::ClipIDGenerator generator;
-    generator.update<Renderable>({ renderables.begin(), renderables.end() });
+    generator.update<Renderable>(makeSorted(renderables));
     ASSERT_EQ(decltype(renderables)({
                   Renderable{ UnwrappedTileID{ 2, 0, 0 }, ClipID{ "00001111", "00000001" } },
                   Renderable{ UnwrappedTileID{ 3, 0, 0 }, ClipID{ "00001111", "00000011" } },
@@ -237,7 +246,7 @@ TEST(GenerateClipIDs, Bug206) {
     };
 
     algorithm::ClipIDGenerator generator;
-    generator.update<Renderable>({ renderables.begin(), renderables.end() });
+    generator.update<Renderable>(makeSorted(renderables));
     EXPECT_EQ(
         decltype(renderables)({
             Renderable{ UnwrappedTileID{ 10, 162, 395 }, ClipID{ "00001111", "00000001" } },
@@ -350,8 +359,8 @@ TEST(GenerateClipIDs, SomeUnclippedTiles) {
     };
 
     algorithm::ClipIDGenerator generator;
-    generator.update<Renderable>({ renderables1.begin(), renderables1.end() });
-    generator.update<Renderable>({ renderables2.begin(), renderables2.end() });
+    generator.update<Renderable>(makeSorted(renderables1));
+    generator.update<Renderable>(makeSorted(renderables2));
     EXPECT_EQ(decltype(renderables1)({
                 Renderable { UnwrappedTileID { 7, 36, 49 }, ClipID {"00000011","00000010"} },
                 Renderable { UnwrappedTileID { 7, 36, 48 }, ClipID {"00000011","00000001"} },
diff --git a/test/algorithm/update_tile_masks.test.cpp b/test/algorithm/update_tile_masks.test.cpp
index 3c698eb0cd..381a6628cf 100644
--- a/test/algorithm/update_tile_masks.test.cpp
+++ b/test/algorithm/update_tile_masks.test.cpp
@@ -45,7 +45,10 @@ void validate(const std::vector<MaskedRenderable> expected) {
     std::vector<MaskedRenderable> actual = expected;
     std::for_each(actual.begin(), actual.end(),
                   [](auto& renderable) { renderable.mask.clear(); });
-    algorithm::updateTileMasks<MaskedRenderable>({ actual.begin(), actual.end() });
+    std::vector<std::reference_wrapper<MaskedRenderable>> sorted(actual.begin(), actual.end());
+    std::sort(sorted.begin(), sorted.end(),
+                  [](const MaskedRenderable& a, const MaskedRenderable& b){ return a.id < b.id; });
+    algorithm::updateTileMasks<MaskedRenderable>(std::move(sorted));
     EXPECT_EQ(expected, actual);
 }
 
-- 
cgit v1.2.1