Improve PictureLayerTiling::CoverageIterator to handle rounding more precisely

cc/tiles/picture_layer_tiling.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/tiles/picture_layer_tiling.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 387 matching lines @@
       left_(0),
       top_(0),
       right_(-1),
       bottom_(-1) {
   DCHECK(tiling_);
   if (dest_rect_.IsEmpty())
     return;
 
   dest_to_content_scale_ = tiling_->contents_scale_ / dest_scale;
 
-  gfx::Rect content_rect =
-      gfx::ScaleToEnclosingRect(dest_rect_,
-                                dest_to_content_scale_,
-                                dest_to_content_scale_);
+  SkRect content_rect;

[enne (OOO), 2016/09/02 19:04:54]

What's with SkRect here?

[trchen, 2016/09/02 23:06:54]

LTRB rect is also preferred here. I'll covert it to explicit computation as
well.

+  SkMatrix::MakeScale(dest_to_content_scale_)
+      .mapRectScaleTranslate(&content_rect, gfx::RectToSkRect(dest_rect_));
+  // Shrink rect by 1/512 to avoid generating empty tiles due to FP error.

[enne (OOO), 2016/09/02 19:04:54]

Sorry, but I'm not sure I follow this.

[trchen, 2016/09/02 23:06:54]

It is for avoid hitting DCHECK on line #487.

Although we didn't take enclosing rect on line #410, FP error can still expand
content_rect by 0.5 ulp. Also on line #470, the texel_extent can be shrunk by
0.5 ulp. This can cause a situation that at line #420~#429 we thought a tile
barely intersected with dest rect then later at line #487 found otherwise.

The constant 1/512px is picked by the fact that even if we missed a pixel for
shrinking, the pixel value is guaranteed to be transparent after bilinear
filtering.

+  float epsilon = 1.f / 512.f;
+  content_rect.inset(epsilon, epsilon);
   // IndexFromSrcCoord clamps to valid tile ranges, so it's necessary to
   // check for non-intersection first.
-  content_rect.Intersect(gfx::Rect(tiling_->tiling_size()));
-  if (content_rect.IsEmpty())
+  if (!content_rect.intersect(
+          gfx::RectToSkRect(gfx::Rect(tiling_->tiling_size()))))
     return;
 
-  left_ = tiling_->tiling_data_.TileXIndexFromSrcCoord(content_rect.x());
-  top_ = tiling_->tiling_data_.TileYIndexFromSrcCoord(content_rect.y());
-  right_ = tiling_->tiling_data_.TileXIndexFromSrcCoord(
-      content_rect.right() - 1);
-  bottom_ = tiling_->tiling_data_.TileYIndexFromSrcCoord(
-      content_rect.bottom() - 1);
+  left_ = tiling_->tiling_data_.LastBorderTileXIndexFromSrcCoord(

[enne (OOO), 2016/09/02 19:04:54]

Can you leave some comments about where the 0.5 is coming from here?

[trchen, 2016/09/02 23:06:54]

Acknowledged.

It is difference between sample point location and its integer index.

+      floorf(content_rect.left() - 0.5f));
+  top_ = tiling_->tiling_data_.LastBorderTileYIndexFromSrcCoord(
+      floorf(content_rect.top() - 0.5f));
+  right_ = std::max(tiling_->tiling_data_.FirstBorderTileXIndexFromSrcCoord(
+                        ceilf(content_rect.right() - 0.5f)),
+                    left_);
+  bottom_ = std::max(tiling_->tiling_data_.FirstBorderTileYIndexFromSrcCoord(
+                         ceilf(content_rect.bottom() - 0.5f)),
+                     top_);
 
   tile_i_ = left_ - 1;
   tile_j_ = top_;
   ++(*this);
 }
 
 PictureLayerTiling::CoverageIterator::~CoverageIterator() {
 }
 
 PictureLayerTiling::CoverageIterator&
@@ skipping 14 matching lines @@
     }
   }
 
   current_tile_ = tiling_->TileAt(tile_i_, tile_j_);
 
   // Calculate the current geometry rect.  Due to floating point rounding
   // and ToEnclosingRect, tiles might overlap in destination space on the
   // edges.
   gfx::Rect last_geometry_rect = current_geometry_rect_;
 
-  gfx::Rect content_rect = tiling_->tiling_data_.TileBounds(tile_i_, tile_j_);
-
-  current_geometry_rect_ =
-      gfx::ScaleToEnclosingRect(content_rect, 1 / dest_to_content_scale_);
+  // Important: To ensure no seams between tiles due to FP error, intermediate

[enne (OOO), 2016/09/02 19:04:54]

I would rather do the computations using gfx::Rect where you can (and explicit
computation where you can't) rather than depending on SkRect implementation
details.

[trchen, 2016/09/02 23:06:54]

Acknowledged.

+  // computation needs to be done in SkRect instead of gfx::RectF. It is
+  // because SkRect stores LTRB internally instead of XYWH.
+  SkRect texel_extent =
+      gfx::RectFToSkRect(tiling_->tiling_data_.TexelExtent(tile_i_, tile_j_));
+  SkMatrix::MakeScale(1 / dest_to_content_scale_)
+      .mapRectScaleTranslate(&texel_extent, texel_extent);
+  SkIRect geometry_rect;
+  texel_extent.roundIn(&geometry_rect);
+  // Extend external edges. Sample coordinates beyond the extent of texels
+  // will be clamped, but will be assigned transparency by AA fragment shaders.
+  {
+    const TilingData& data = tiling_->tiling_data_;
+    int l = tile_i_ ? geometry_rect.left() : 0;
+    int t = tile_j_ ? geometry_rect.top() : 0;
+    int r = (tile_i_ == data.num_tiles_x() - 1) ? data.tiling_size().width()

[enne (OOO), 2016/09/02 19:04:54]

Why do you need this conditional here?

[trchen, 2016/09/02 23:06:54]

Extend the right edge to layer bound like the left/top edge did.

It is due to the fact that the texel extent only covers up to
tiling_size().width() - 0.5, but we still want the quad to cover the whole layer
rect, as that's what the PictureLayerTilingSet and quad system expects.

BTW I'm thinking an alternative layer AA scheme using GL_CLAMP_TO_BORDER with
transparent border. We can discuss about that next time we meet.

+                                                : geometry_rect.right();
+    int b = (tile_j_ == data.num_tiles_y() - 1) ? data.tiling_size().height()
+                                                : geometry_rect.bottom();
+    current_geometry_rect_ = gfx::Rect(l, t, r - l, b - t);
+  }
 
   current_geometry_rect_.Intersect(dest_rect_);
   DCHECK(!current_geometry_rect_.IsEmpty());
 
   if (first_time)
     return *this;
 
   // Iteration happens left->right, top->bottom.  Running off the bottom-right
   // edge is handled by the intersection above with dest_rect_.  Here we make
   // sure that the new current geometry rect doesn't overlap with the last.
@@ skipping 407 matching lines @@
 size_t PictureLayerTiling::GPUMemoryUsageInBytes() const {
   size_t amount = 0;
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
     const Tile* tile = it->second.get();
     amount += tile->GPUMemoryUsageInBytes();
   }
   return amount;
 }
 
 }  // namespace cc