cc: optimize rtree loop

The rtree build_tree code is calculating two parameters num_strips
and num_tiles that it used to iterate through the list but both are
unnecessary.  The iterated parameters were not used, and the ceil
used on the sqrt insured that we go through every single branch.

This patch removes the extra variables and converts the two loops
going through strips and tiles, into one loop that goes through
branches from the input.

Bug: None
Change-Id: I156a1607d81010f00523f0a6c60e45dfe3a88a88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1612047Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: James Bankoski <jimbankoski@google.com>
Cr-Commit-Position: refs/heads/master@{#659749}

cc/base/rtree.h

@@ -230,60 +230,54 @@ auto RTree<T>::BuildRecursive(std::vector<Branch<T>>* branches, int level)
       remainder = kMinChildren - remainder;
   }
 
-  int num_strips = static_cast<int>(std::ceil(std::sqrt(num_branches)));
-  int num_tiles = static_cast<int>(
-      std::ceil(num_branches / static_cast<float>(num_strips)));
   size_t current_branch = 0;
 
   size_t new_branch_index = 0;
-  for (int i = 0; i < num_strips; ++i) {
-    // Might be worth sorting by X here too.
-    for (int j = 0; j < num_tiles && current_branch < branches->size(); ++j) {
-      int increment_by = kMaxChildren;
-      if (remainder != 0) {
-        // if need be, omit some nodes to make up for remainder
-        if (remainder <= kMaxChildren - kMinChildren) {
-          increment_by -= remainder;
-          remainder = 0;
-        } else {
-          increment_by = kMinChildren;
-          remainder -= kMaxChildren - kMinChildren;
-        }
+  while (current_branch < branches->size()) {
+    int increment_by = kMaxChildren;
+    if (remainder != 0) {
+      // if need be, omit some nodes to make up for remainder
+      if (remainder <= kMaxChildren - kMinChildren) {
+        increment_by -= remainder;
+        remainder = 0;
+      } else {
+        increment_by = kMinChildren;
+        remainder -= kMaxChildren - kMinChildren;
       }
-      Node<T>* node = AllocateNodeAtLevel(level);
-      node->num_children = 1;
-      node->children[0] = (*branches)[current_branch];
-
-      Branch<T> branch;
-      branch.bounds = (*branches)[current_branch].bounds;
-      branch.subtree = node;
+    }
+    Node<T>* node = AllocateNodeAtLevel(level);
+    node->num_children = 1;
+    node->children[0] = (*branches)[current_branch];
+
+    Branch<T> branch;
+    branch.bounds = (*branches)[current_branch].bounds;
+    branch.subtree = node;
+    ++current_branch;
+    int x = branch.bounds.x();
+    int y = branch.bounds.y();
+    int right = branch.bounds.right();
+    int bottom = branch.bounds.bottom();
+    for (int k = 1; k < increment_by && current_branch < branches->size();
+         ++k) {
+      // We use a custom union instead of gfx::Rect::Union here, since this
+      // bypasses some empty checks and extra setters, which improves
+      // performance.
+      auto& bounds = (*branches)[current_branch].bounds;
+      x = std::min(x, bounds.x());
+      y = std::min(y, bounds.y());
+      right = std::max(right, bounds.right());
+      bottom = std::max(bottom, bounds.bottom());
+
+      node->children[k] = (*branches)[current_branch];
+      ++node->num_children;
       ++current_branch;
-      int x = branch.bounds.x();
-      int y = branch.bounds.y();
-      int right = branch.bounds.right();
-      int bottom = branch.bounds.bottom();
-      for (int k = 1; k < increment_by && current_branch < branches->size();
-           ++k) {
-        // We use a custom union instead of gfx::Rect::Union here, since this
-        // bypasses some empty checks and extra setters, which improves
-        // performance.
-        auto& bounds = (*branches)[current_branch].bounds;
-        x = std::min(x, bounds.x());
-        y = std::min(y, bounds.y());
-        right = std::max(right, bounds.right());
-        bottom = std::max(bottom, bounds.bottom());
-
-        node->children[k] = (*branches)[current_branch];
-        ++node->num_children;
-        ++current_branch;
-      }
-      branch.bounds.SetRect(x, y, base::ClampSub(right, x),
-                            base::ClampSub(bottom, y));
-
-      DCHECK_LT(new_branch_index, current_branch);
-      (*branches)[new_branch_index] = std::move(branch);
-      ++new_branch_index;
     }
+    branch.bounds.SetRect(x, y, base::ClampSub(right, x),
+                          base::ClampSub(bottom, y));
+
+    DCHECK_LT(new_branch_index, current_branch);
+    (*branches)[new_branch_index] = std::move(branch);
+    ++new_branch_index;
   }
   branches->resize(new_branch_index);
   return BuildRecursive(branches, level + 1);