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

Issue 732341 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Pixels are corrupted in help and NTP pages

Project Member Reported by sc00335...@techmahindra.com, Jun 12 2017

Issue description

Chrome Version: 60.0.3112.30 
OS: Ubuntu 14.04

Pre-Condition: Enable GPU rasterization flag from chrome://flags

What steps will reproduce the problem?
(1)Launch chrome and enable above flag >> Go to NTP or hit F1 and check font 

Expected: Font should be clear and readable.
Actual: Instead weird font is seen.

This is a regression issue broken in M60.

Good Build: 60.0.3104.0 dev
Bad Build: 60.0.3105.0 dev

NOTE: Issue is not seen on windows.

 
Expected_rasterization.png
144 KB View Download
Actual_rasterization.png
141 KB View Download
Actual_font.png
123 KB View Download

Comment 1 by bsalo...@google.com, Jun 12 2017

Cc: egdaniel@chromium.org
Status: Untriaged (was: Unconfirmed)
Unable to reproduce this issue on Mac OS 10.12 using chrome dev #60.0.3112.30. Observed similar behavior on Ubuntu 14.04 as well. This issue might be specific to GPU.

Sindhu@ Please provide bisect information.
Labels: -Needs-Bisect hasbisect
Owner: egdaniel@chromium.org
Status: Assigned (was: Untriaged)
This issue is seen for many pages, Compose button of Gmail , Account info page etc.. 

You are probably looking for a change made after 473360 (known good), but no later than 473367 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/cdcd0b7cbe1630435abab4a03ca0543752138b56..e9cc6305714c1a585bc5cf51d289fdf000070ea0

Suspecting https://skia.googlesource.com/skia.git/+/d547fb602d3fd970882c3fa39464ec48bc6c4d5d from skia roll

@egdaniel:Please confirm the issue and help in re-assigning if it is not related to your change.
Screenshot from 2017-06-12 17:27:41.png
140 KB View Download
Attaching chrome://gpu of system where it reproduces..


chrome___gpu.pdf
95.4 KB Download
The attached chrome_gpu says rasterization is using software. When the bug occurs is this the case or did you just have gpu-rasterization disabled when you took the screen shot?
Also this does not repo on ubuntu with nvidia gpu. I'll have to find an ubuntu machine with intel gpu to build chrome on and test there.

Comment 7 by bsalo...@google.com, Jun 12 2017

Greg, just noting that the Skia revision in use is from the M60 branch, here:

https://skia.googlesource.com/skia/+/chrome/m60
With responce to comment#4: Attached wrong GPU.

Now attaching GPU details when GPU Rasterization is enabled.

Issue is seen in some more places like print preview [buttons,links] as well.
chrome___gpu.pdf
97.8 KB Download
Screenshot from 2017-06-13 12:31:38.png
256 KB View Download
Actual_print gpu.png
127 KB View Download

Comment 9 by ericrk@chromium.org, Jun 13 2017

Just to make sure we're tracking down the right thing - GPU rasterization is disabled on Linux by default - sc00335628@, had you enabled GPU rasterization yourself?

Not saying we shouldn't treat this as high priority - we plan to turn GPU raster on for Linux soon - but just want to make sure this doesn't happen with SW raster as well.
So I tried repoing the bug on Fedora with an Intel GPU, but I was not able to repo the bug. My driver is newer than the one reported here (13.0.2 vs 11.0.2)  which may be the cause. Since I haven't been able to actual get this to repo I can't claim whether or not this a specific hardware issue, a driver issue, OS issue, or bug in Skia.

However, I did put together a CL (https://skia-review.googlesource.com/c/19703/), which should get us a least back to where we were before m60. sc00335628@ can you try patching in that change and see if it fixes the bug?
With respect to comment#9: Yes i enabled GPU Rasterization from chrome://flags
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/b54bdef86eb5cf63b94588afaa9197f49374a5f5

commit b54bdef86eb5cf63b94588afaa9197f49374a5f5
Author: Greg Daniel <egdaniel@google.com>
Date: Wed Jun 14 15:13:58 2017

Go back to using dual source blending for lcd src-over even with non-opaque color

This is change is currently still safe since earlier in Skia we are still requiring
the dst to be opaque. The change is a workaround to spots where trying to read the
dst to do in shader blending is failing for some reason. This also should give back
a little performance since doing dual source blending should be better than shader
blends.

Bug:  chromium:732341 
Change-Id: I795f8a520f87f3fbf5d63a9509fbd9f394ea2b29
Reviewed-on: https://skia-review.googlesource.com/19703
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>

[modify] https://crrev.com/b54bdef86eb5cf63b94588afaa9197f49374a5f5/src/gpu/effects/GrPorterDuffXferProcessor.cpp
[modify] https://crrev.com/b54bdef86eb5cf63b94588afaa9197f49374a5f5/tests/GrPorterDuffTest.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/7d6fe0b9964d139de64ca88c46d87eba41c7f84e

commit 7d6fe0b9964d139de64ca88c46d87eba41c7f84e
Author: Greg Daniel <egdaniel@google.com>
Date: Wed Jun 14 15:55:50 2017

Revert "Go back to using dual source blending for lcd src-over even with non-opaque color"

This reverts commit b54bdef86eb5cf63b94588afaa9197f49374a5f5.

Reason for revert: breaking some bots
Original change's description:
> Go back to using dual source blending for lcd src-over even with non-opaque color
> 
> This is change is currently still safe since earlier in Skia we are still requiring
> the dst to be opaque. The change is a workaround to spots where trying to read the
> dst to do in shader blending is failing for some reason. This also should give back
> a little performance since doing dual source blending should be better than shader
> blends.
> 
> Bug:  chromium:732341 
> Change-Id: I795f8a520f87f3fbf5d63a9509fbd9f394ea2b29
> Reviewed-on: https://skia-review.googlesource.com/19703
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>

TBR=egdaniel@google.com,bsalomon@google.com

Change-Id: Ibb9bc1ef4ec5967dabcd62c81f62c0989c14fbb8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:732341 
Reviewed-on: https://skia-review.googlesource.com/19815
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>

[modify] https://crrev.com/7d6fe0b9964d139de64ca88c46d87eba41c7f84e/src/gpu/effects/GrPorterDuffXferProcessor.cpp
[modify] https://crrev.com/7d6fe0b9964d139de64ca88c46d87eba41c7f84e/tests/GrPorterDuffTest.cpp

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 14 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/d7b1159d787de27d75032ec213903a03962ec839

commit d7b1159d787de27d75032ec213903a03962ec839
Author: Greg Daniel <egdaniel@google.com>
Date: Wed Jun 14 18:40:25 2017

Revert "Revert "Go back to using dual source blending for lcd src-over even with non-opaque color""

This reverts commit 7d6fe0b9964d139de64ca88c46d87eba41c7f84e.

Reason for revert: Relanding with fix

Original change's description:
> Revert "Go back to using dual source blending for lcd src-over even with non-opaque color"
> 
> This reverts commit b54bdef86eb5cf63b94588afaa9197f49374a5f5.
> 
> Reason for revert: breaking some bots
> Original change's description:
> > Go back to using dual source blending for lcd src-over even with non-opaque color
> > 
> > This is change is currently still safe since earlier in Skia we are still requiring
> > the dst to be opaque. The change is a workaround to spots where trying to read the
> > dst to do in shader blending is failing for some reason. This also should give back
> > a little performance since doing dual source blending should be better than shader
> > blends.
> > 
> > Bug:  chromium:732341 
> > Change-Id: I795f8a520f87f3fbf5d63a9509fbd9f394ea2b29
> > Reviewed-on: https://skia-review.googlesource.com/19703
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Commit-Queue: Greg Daniel <egdaniel@google.com>
> 
> TBR=egdaniel@google.com,bsalomon@google.com
> 
> Change-Id: Ibb9bc1ef4ec5967dabcd62c81f62c0989c14fbb8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  chromium:732341 
> Reviewed-on: https://skia-review.googlesource.com/19815
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>

TBR=egdaniel@google.com,bsalomon@google.com
Bug:  chromium:732341 

Change-Id: I7481755a9aa64364371d8149af4458fc2c15c8aa
Reviewed-on: https://skia-review.googlesource.com/19840
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>

[modify] https://crrev.com/d7b1159d787de27d75032ec213903a03962ec839/src/gpu/effects/GrPorterDuffXferProcessor.cpp
[modify] https://crrev.com/d7b1159d787de27d75032ec213903a03962ec839/tests/GrPorterDuffTest.cpp

Status: Fixed (was: Assigned)
Should be fixed on ToT.
Labels: Merge-Request-60
Do we need a merge to M60 here?  I'm seeing the issue on the beta channel.
Project Member

Comment 17 by sheriffbot@chromium.org, Jul 11 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Approved-60
Approving merge to M60. 
So just want to double check before we Merge to M60. From what was described in the bug this is an issue on just Linux with GPU rasterization enabled. AFAIK we don't default to GPU on Linux so a user would manually have to switch it. If this is the case do we still want to cherry pick?
Project Member

Comment 20 by sheriffbot@chromium.org, Jul 17 2017

Cc: abdulsyed@chromium.org fmalita@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -M-60
Thanks Greg, I hadn't realized this is Linux/GPU-only.

If that's the case, then I agree -- the m60 merge is not high priority.  I'd say we should only merge if there aren't conflicts and the change is low-risk.
Labels: -Merge-Approved-60 Merge-Rejected-60
Thanks for more information - I think per comment 19 and 21, let's wait until M61. 
Cc: bunge...@chromium.org derat@chromium.org
 Issue 751120  has been merged into this issue.

Comment 24 by derat@chromium.org, Aug 17 2017

Cc: drott@chromium.org susanjuniab@chromium.org
 Issue 754243  has been merged into this issue.

Comment 25 by derat@chromium.org, Aug 17 2017

Cc: krajshree@chromium.org vmi...@chromium.org ericrk@chromium.org
 Issue 750494  has been merged into this issue.

Sign in to add a comment