Version: ToT
OS: All
What steps will reproduce the problem?
(1) Go to http://jsbin.com/dezalef/
What is the expected output?
The top and bottom pattern should look identical.
Ideally there should be no seams at tile boundary, but a separate issue with AA fragment shader will cause seams for quads that reveal layer edges.
What do you see instead?
There are visible seams even between internal tiles.
Detailed analysis from original thread https://codereview.chromium.org/2175553002/#msg41 pasted below:
On 2016/08/29 20:21:19, enne wrote:
https://codereview.chromium.org/2175553002/diff/60001/cc/tiles/picture_layer_...
> > cc/tiles/picture_layer_tiling.cc:468:
> tile_bounds_in_dest_space.Offset(-tiling_->contents_translation());
> > On 2016/08/22 22:57:39, enne wrote:
> > > I'm not 100% convinced that this satisfies the assumptions being made by
> > > coverage iterator, in particular that it will always fill the destination
> space
> > > perfectly. Can you add some PictureLayerTilingSetTestWithResources
> unittests
> > > that include translation offsets as well at different scales?
> >
> > It will because the pre-rounding tile_bounds_in_dest_space already fill the
> dest space. With ToEnclosingRect, some pixels in the dest space will be
covered
> by more than one tiles.
> >
> > In fact, some rounding math here is already questionable prior to my CL. I
> don't think we can simply take the enclosing rect for geometry rect. I will
> follow up with more numerical analysis.
>
> What is questionable about it? I'll be interested to hear your analysis.
Assuming no MSAA, the sample point of each pixel will be the center of that
pixel. For a tile of 256x256, the top-left sample point has coordinate of (0.5,
0.5). Likewise, (255.5, 255.5) for the bottom-right.
In other words, the texture sampling extent for a tile is only
tile_size.Inset(0.5). Sample points outside of that range will be wrapped
depending on the GL wrap mode. This is undesired for tiling except for the layer
edges.
Now look at our 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_);
Is it possible for the geometry rect to cover a point that will result in an
invalid sample point? The answer is yes, consider the configuration:
layer_size = 1000x1000
layer_transform = = draw_transform = translate2d(0.6px, 0.6px)
ideal_scale = 1
dest_scale = 1 + ε
Consider an ideal tiling of
contents_scale = 1
The content_rect of the top-left tile would be
(l=0, t=0, w=255, h=255)
The rect scaled to the dest space became (l=0, t=0, w=255+ε, h=255+ε)
Taking the enclosing rect,
current_geometry_rect = (l=0, t=0, w=256, h=256)
Now as we draw the quad, the quad will cover (l=0.6, t=0.6, w=256-ε, h=256-ε),
which include the screen pixel at (256.5, 256.5). Mapping it back to the texture
space the sampling point would be (255.9, 255.9), that exceeded the valid
texture range.
We don't see this drawing artifacts often because the condition is very rare as
most of the time the ideal tiling is the tiling with maximum scale. I think it
can only happen during a pinch zoom out, and only on tiles that are
unfortunately rounded outward by more than 0.5px.
Anyway, bad math is bad math and we can certainly do it more correctly.
The right way to calculate geometry rect is to take the inset of the tile size:
gfx::RectF tile_sample_extent = tiling_data.TileBoundsWithBorder().Inset(0.5);
gfx::RectF unrounded_geometry_rect = gfx::ScaleRect(tile_sample_extent,
content_to_dest_scale);
These are exact rect that contains neither false positive or false negative.
And I would recommend to do coverage in float region instead of integer region.
I don't think float region would be much slower than integer region as there is
no arithmetic involved, only comparison. FP comparison should be as fast as
integers as long as no special numbers are involved. Doing FP region would allow
us to use a tighter overlap between tiles (1 instead of 2px), that saves about
0.7% of texture memory.
If we have to stay with integer region, then we'll need to round inwards instead
of outward:
current_geometry_rect_ = gfx::ToEnclosedRect(unrounded_geometry_rect);
Now the question becomes whether the rounded geometry rect will cover the dest
space. The answer is yes with the following proof:
1. content_to_dest_scale >= 1 // because dest scale is
max scale
2. right_edge_of_tile = left_edge_of_next_tile + 2 // 2px overlap between
tiles
3. right_edge_of_sample_extent = right_edge_of_tile - 0.5
4. left_edge_of_next_sample_extent = left_edge_of_next_tile + 0.5
5. right_edge_of_geometry_rect = floor(right_edge_of_sample_extent *
content_to_dest_scale)
6. left_edge_of_next_geometry_rect = ceil(left_edge_of_next_sample_extent *
content_to_dest_scale)
7. right_edge_of_geometry_rect
= floor(right_edge_of_sample_extent * content_to_dest_scale)
= floor((left_edge_of_next_sample_extent + 1) * content_to_dest_scale) // plug
2,3,4
= floor(left_edge_of_next_sample_extent * content_to_dest_scale +
content_to_dest_scale)
>= floor(left_edge_of_next_sample_extent * content_to_dest_scale + 1) //
because 1
>= ceil(left_edge_of_next_sample_extent * content_to_dest_scale)
= left_edge_of_next_geometry_rect
Q.E.D.
Comment 1 by bugdroid1@chromium.org
, Sep 21 2016