New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 623198 link

Starred by 4 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Vertical line corruption on tile borders

Project Member Reported by vmi...@chromium.org, Jun 24 2016

Issue description

Version: 53.0.2778.2 (also seen in 52.0.2743.49)
OS: Windows

What steps will reproduce the problem?
(1) Open http://jason-kenny.com/
(2) Scroll down to the GALLERY section of the page.
(3) Try adjusting page zoom scale.  This seems to change the results.

There are dark & light vertical lines appearing in the gradients on the left & right sides of the page.  They correspond to tile boundaries.

The pattern with one dark column and one light column looks like tiles are rendered overlapped +/- 1 pixel.
 
jason-kenny.png
4.2 MB View Download

Comment 1 by vmi...@chromium.org, Jun 24 2016

Labels: -OS-Windows OS-All
Reproduced on Mac & Linux also.

Comment 2 by danakj@chromium.org, Jun 24 2016

I wonder if the rastered tiles show this or just the compositor output. Page zoom shouldn't really change drawing.

Comment 3 by danakj@chromium.org, Jun 24 2016

Cc: chrishtr@chromium.org pdr@chromium.org

Comment 4 by vmi...@chromium.org, Jun 24 2016

> I wonder if the rastered tiles show this or just the compositor output. 
> Page zoom shouldn't really change drawing.

Kind of suspect compositor output more due to the double-blend effect.

The layer's x offset is shifting while zooming.  Could this be changing the tile's coordinate snapping?

Comment 5 by vmi...@chromium.org, Jun 24 2016

Confirmed it's a compositor drawing issue using VS graphics debugger.


pixels.png
978 KB View Download
overdraw.png
20.0 KB View Download

Comment 6 by enne@chromium.org, Jun 25 2016

My guess is that it's floating point related.  The quads are all at x=~17500000.

Comment 7 by enne@chromium.org, Jun 27 2016

If that's true, then a potential fix would be to adjust the draw transform in the shared quad state of PictureLayerImpl such that the first quad (or the top left of the visible content rect) is at the origin, rather than having a far away origin and quads at large offsets.
I can repro it on Android Nexus 5 and Galaxy S6 too. There are lot of white tiles too, while we pinch/zoom . The issue is not repro when we force gpu-raster on Android.

I can work on this, if no one has started yet.

Comment 9 by enne@chromium.org, Jun 28 2016

sohan: I don't think anybody's looking at fixing this, past doing some initial investigation.  If you want to take a look, that'd be great.  I'd be happy to review any patch.
The problem indeed looks to be the tile boundaries, the bigger sized gpu tiles hence doesnt have the artifacts. 
But its not only the left most tile/quad, but also the right most which has the issue. So, looks to be the border tiles are the problems.

From the content, its the navigation either side of the img which makes those tile,

<ul class="flex-direction-nav"><li><a class="flex-prev" href="#">Previous</a></li><li><a class="flex-next flex-disabled" href="#">Next</a></li></ul>

there is horiz navigation all throughout the page, so the lines appears at quite a few places.
the strange thing is, it is still only the img tiles which get affected, do we treat them any special while drawing quad (i think we just apply transform and gldraw for each quad)?

not sure whats the best soln here ? promote divs to layer if it overlaps with img ? make tiles more aligned to viewport ?




Comment 11 by enne@chromium.org, Jun 29 2016

sohan: see comment #6 and #7.

It's not the left-most or right-most.  You can change the tile size to 50 and you'll see lots of internal lines.
It didn't look like it was specific to image tiles.  The incorrect quads look like the picture quads in the "flex-prev" and "flex-next" boxes.  It's feint, but I can see the lines floating over text in the  "VIDEO & AUDIO" section.

Fundamentally I think the issue is we get adjacent quads with slightly different vertex coordinates after doing the transform math, as we don't use the same vertices for adjacent quads; we take a 1x1 rectangle and transform it into place.  The slight floating point math difference between left & right sides of the rectangle could account for the error, due to the huge coordinates.

A work around is maybe as enne suggested in #7, adjusting the transform and tile positions so that the coordinates aren't huge.

The full solution might be to change our cc::GLRenderer vertex shaders so we do the exact same math for adjacent quad vertices.  More involved but I think possible.

enne: I'm curious what makes the layer's origin so distant?  The content of the layer seems to be entirely near the viewport (float:left, float:right content).  It looks like a squashing result?

  tilingSize: {height: 2402, width: 20000716}
  visibleRect: Rect(19997846, 2, 2870, 1486)

Comment 13 by enne@chromium.org, Jun 29 2016

I'm not sure what makes the layer's origin so distant, but I think we should just make this case work properly in the renderer regardless.
This (ugly) patch renders correctly by anchoring the transform offset to the first visible quad in a layer.  @sohan maybe you can productize something similar?  https://codereview.chromium.org/2109603005
Cc: vollick@chromium.org esprehn@chromium.org
chrishtr@ or vollick@ could you take a quick look at why we're getting 20 million pixel wide layers for the "flex-prev" & "flex-next" elements?

I think we want to handle drawing big layers anyway, but it seems like a bug around paint or squashing.  All of the layer's content has a 20 million pixel translaiton.  esprehn@ told me he recently saw cracks between tiles on other content as well.
thanks Victor, enne@. I will check in those lines.

Victor, in android, i can still see the artifacts after applying your patch, i am attaching the screen-shot. I will check more.
screenshot-12147dec9d40592d-20160630T114404.png
1.6 MB View Download
Thanks Sohan.  I could still reproduce the lines on Android too.  I'm not sure if that's because the work around isn't working as expected, or it's just other FP math issues.

I uploaded another patch that fixes the bugs for me on desktop but not Android https://codereview.chromium.org/2109333005/.  Anyway, hope these point you in the right direction.

Thanks for the help Victor.

In AppendQuads, the quad rects are originally in this form,

geometry_rect = 14692796,2 305x509
opaque_rect = 0,0 0x0
visible_geometry_rect = 14692797,2 304x509

I wonder whether the 1px difference between the occluded and un-occluded geometry_rect can also be an issue ?
If i make them same, i cant see the artifacts in android though.

Looks like on pinch-zoom or scale change, the occlusion calculations (visible_geometry_rect) is causing the problem, floating points and partial pixels rounding etc.
I think that's the reason the problem is not manifested on loading the website, but rather after we change the scale.

enne@, is there any known issue about occlusion after scaling?

Or you think its better to change quad origin to solve the issue ?

Should be a hack, but can we skip getting visible_geometry_rect (GetUnoccludedContentRect) for quads far away from origin, and use geometry_rect instead.
https://cs.chromium.org/chromium/src/cc/layers/picture_layer_impl.cc?q=picture_layer_im&sq=package:chromium&l=289

For the far-away quads they are mostly same as geometry_rect with a 1px difference.
But, then the question is how far do we consider as far away quad ?

enne@ wdyt ?
Issue 575800 may be related; when we do anti-aliasing on the edges of separate layers which line up at a non-pixel coordinate you can see through the gap between them.

Comment 22 by enne@chromium.org, Jul 6 2016

sohan: I don't think that sounds like a workable solution.  If the error is in calculating visible geometry rect, then that should just be fixed directly.
enne@, following is the break down of how visible geometry rect adds the extra pixels.

[INFO:picture_layer_impl.cc(296)] AppendQuads  - geometry_rect = 14692796,1 305x510
[INFO:math_util.cc(182)] MapEnclosingClippedRect  - mapped_rect = -1.000000,609.122070 305.000000x510.000000
[INFO:math_util.cc(191)] MapEnclosingClippedRect  - mapped_enclosed_rect = -1,609 305x511
[INFO:occlusion.cc(85)] GetUnoccludedRectInTargetSurface  - unoccluded_rect_in_target_surface = -1,609 305x511
[INFO:occlusion.cc(60)] GetUnoccludedContentRect  - draw_transform_ = 
 [ +1.0000 +0.0000 +0.0000 -14692796.0000  
   +0.0000 +1.0000 +0.0000 +608.1221  
   +0.0000 +0.0000 +1.0000 +0.0000  
   +0.0000 +0.0000 +0.0000 +1.0000 ]
 
[INFO:math_util.cc(44)] ProjectHomogeneousPoint  - p.x = -1 p.y = 609 z = -0 w = 1
[INFO:math_util.cc(44)] ProjectHomogeneousPoint  - p.x = 304 p.y = 609 z = -0 w = 1
[INFO:math_util.cc(44)] ProjectHomogeneousPoint  - p.x = 304 p.y = 1120 z = -0 w = 1
[INFO:math_util.cc(44)] ProjectHomogeneousPoint  - p.x = -1 p.y = 1120 z = -0 w = 1
[INFO:math_util.cc(241)] ProjectEnclosingClippedRect  - projected_rect = 14692797.000000,0.877930 305.000000x511.000061
[INFO:math_util.cc(246)] ProjectEnclosingClippedRect  - proj_enclosed_rect = 14692797,0 305x512

[INFO:occlusion.cc(73)] GetUnoccludedContentRect  - unoccluded_rect = 14692797,0 305x512
[INFO:occlusion.cc(75)] GetUnoccludedContentRect  - unoccluded_rect_after_intersection = 14692797,1 304x510

[INFO:picture_layer_impl.cc(300)] AppendQuads  - visible_geometry_rect = 14692797,1 304x510

The ToEnclosingRect() func does add them in both mapped and projected rect.
Also, the projection shifts the origin.

Not sure, what should be the right way to proceed ? 
this math utils apis are pretty stable otherwise, right ? should we plan to change them to solve the corner cases ?

Comment 24 by enne@chromium.org, Jul 7 2016

If the issue is the that the visible geometry rect is wrong, then what about doing GetUnoccludedContentRect of the entire visible rect for the layer at the top of AppendQuads, and then doing an intersection of that with the geometry rect to get the visible rect per tile?

That way even if the entire visible rect is off-by-one, there will not be inter-tile lines.
That works great! I cant see the lines anymore on Android.
Unfortunately i cant repro the original problem in my linux build to verify there.
I have uploaded a rough patch for it, https://codereview.chromium.org/2131143002/ , can you give your opinion on it?

Comment 26 by enne@chromium.org, Jul 12 2016

I don't think this is the right fix.  If I replace the visible_geometry_rect with geometry_rect in PictureLayerImpl, I still see lines on desktop.  That leads me to believe that maybe there are multiple problems or maybe the problem is in the gl renderer.  Can you investigate more thoroughly?
Hmm.ok.I will check more.
Do you think we should combine Victors' idea/patch for shifting the origin for distant quads and this visible_geometry_rect patch ?

Or check in gl renderer if it can be fixed cleanly there ? thanks.

Comment 28 by enne@chromium.org, Jul 13 2016

Let me know what you find.
Sorry enne@, i couldnt find much problem in gl renderer quad drawing, except for what Victor already pointed in his CL.
I tried modifying the quad coordinates, but it didnt solve the issue on android.
Anything which comes to your mind ?
Repro video on android, chrome public apk on ToT.
jason_kenny_whiteline.zip
5.7 MB Download

Comment 31 by enne@chromium.org, Jul 19 2016

What device are you testing on? It doesn't repro for me on a Nexus 4, even with the page in desktop mode.
This is on Galaxy S6. I will re-check on a Nexus5.
I suspect Enne is right that there are multiple problems, so it may take both the fix for visibile_geometry_rect and a fix to offset the coordinates like crrev.com/2109603005.

The latter I think we could just go ahead and make the change.  Rather than use the coordinate of a visible tile, could we compute the viewport's origin in layer space and use that as the quad origin?
Components: -Internals>Compositing Internals>Compositing>Quads
Project Member

Comment 35 by sheriffbot@chromium.org, Sep 8 2017

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 36 by enne@chromium.org, Sep 8 2017

Status: Available (was: Untriaged)
I think this is worth fixing still.

Comment 37 by enne@chromium.org, Sep 8 2017

Cc: flackr@chromium.org
 Issue 636587  has been merged into this issue.

Comment 38 by enne@chromium.org, Sep 8 2017

Cc: weiliangc@chromium.org yiyix@chromium.org
Project Member

Comment 39 by sheriffbot@chromium.org, Sep 10

Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)

Sign in to add a comment