New issue
Advanced search Search tips

Issue 847984 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 19
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

16.7%-20.6% regression in smoothness.key_silk_cases at 562134:562141

Project Member Reported by sullivan@chromium.org, May 30 2018

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, May 30 2018

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=847984

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=4ce8a983300b18bc0a4fff2dd1feb681e4199fa6b24f1642ae8cf1fe11385e95


Bot(s) for this bug's original alert(s):

android-nexus5
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, May 31 2018

Cc: bsalo...@google.com
Owner: bsalo...@google.com
Status: Assigned (was: Untriaged)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/12f71bc6240000

Add support for SkCanvas::kStrict_SrcRectConstraint to GrTextureOp. by bsalomon@google.com
https://skia.googlesource.com/skia/+/a0047bcff704a9121a6d82a6f97d6124463a2e54
16.79 → 20.06 (+3.263)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Comment 4 by ajuma@chromium.org, Jun 25 2018

Regressions in frame time and fps still haven't recovered. Does it make sense that the CL identified in #3 would increase frame time by 3ms/frame?
I've gotten to the bottom of this. The reason this regressed on the N5 but not other bots is due to a driver/GPU correctness workaround that was introduced to fix  Issue 801783 .

The anti-aliased image drawing code now evaluates 4 edge equations in the shader. Normally we do that in the vertex shader and interpolate the values to the fragment shader where we calculate a fractional coverage. On Adreno 3xx the fixed function interpolation is imprecise enough to cause gpu_tests.screenshot_sync_integration_test.ScreenshotSyncIntegrationTest.ScreenshotSync_GPURasterWithCanvas  to fail.

The workaround is to instead evaluation all four edge equations in at every pixel in the fragment shader. This extra fragment shader work is the cause of the performance regression.

From looking at the results of Skia tests on a N5, Moto-E, and Galaxy J5 with the workaround disabled I think we should just live with the inaccuracy and reclaim the performance. The worst case pixel diff in our tests is 16/255 and is not perceptible to me. Currently this test allows a pixel difference of 1. If I make that 16 locally then the test passes.

From what I can tell cc's GLRenderer does AA the same way and doesn't have this workaround. So I believe we're already accepting this level of inaccuracy at the edges of composited layers.

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/841c0dde5461c763fdd524c2078aafaf241444aa

commit 841c0dde5461c763fdd524c2078aafaf241444aa
Author: Brian Salomon <bsalomon@google.com>
Date: Tue Jul 10 12:51:17 2018

Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.

Skia currently has a driver workaround that performans antialiased
rectangle edge equation evalution in the fragment shader rather than
vertex shader on Adreno 3xx GPUs. This was done because the interpolation
of values computed in the vertex shader lacks enough precision to pass
these tests. However, doing so has a negative performance impact. Skia
tests run with the workaround removed show that in practice the visual
difference is insignificant. Moreover, cc::GLRenderer also incurs these
same errors as it uses the same antialiasing approach without the
workaround and has shipped on Adreno 3xx devices for years with no known
complaints about antialiasing quality/accuracy. We're better off
reclaiming the performance and accepting a small hit to antialiasing
accuracy.

Bug:  847984 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: If1e62e0780a752179a75ddd57ccc7b24128c68ac
Reviewed-on: https://chromium-review.googlesource.com/1129041
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Cr-Commit-Position: refs/heads/master@{#573695}
[modify] https://crrev.com/841c0dde5461c763fdd524c2078aafaf241444aa/content/test/gpu/gpu_tests/screenshot_sync_integration_test.py

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 10

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

commit a7a278205bb040061cb4ba46839efe18635c7edc
Author: Brian Salomon <bsalomon@google.com>
Date: Tue Jul 10 13:52:04 2018

Remove interpolants are inaccurate workaround for Adreno 3xx.

The chrome screenshot unit test that led to adding this workaround has
been adjusted to avoid testing AA edges of rendered rectangles. We're
accepting the inaccuracy in favor of increased performance.

Chrome change: https://chromium-review.googlesource.com/c/chromium/src/+/1129041

Bug:  chromium:847984 
Change-Id: I9b714ade2a67e956ebb2773ebe3b8632dc3a50c6
Reviewed-on: https://skia-review.googlesource.com/140180
Commit-Queue: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

[modify] https://crrev.com/a7a278205bb040061cb4ba46839efe18635c7edc/src/gpu/GrShaderCaps.cpp
[modify] https://crrev.com/a7a278205bb040061cb4ba46839efe18635c7edc/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/a7a278205bb040061cb4ba46839efe18635c7edc/src/gpu/ops/GrTextureOp.cpp
[modify] https://crrev.com/a7a278205bb040061cb4ba46839efe18635c7edc/src/gpu/GrShaderCaps.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95287e9e54828d9e8144463dc25d2c458729010c

commit 95287e9e54828d9e8144463dc25d2c458729010c
Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 10 15:13:46 2018

Roll src/third_party/skia 6d98257725b5..a7a278205bb0 (3 commits)

https://skia.googlesource.com/skia.git/+log/6d98257725b5..a7a278205bb0


git log 6d98257725b5..a7a278205bb0 --date=short --no-merges --format='%ad %ae %s'
2018-07-10 bsalomon@google.com Remove interpolants are inaccurate workaround for Adreno 3xx.
2018-07-10 fmalita@chromium.org Enable Skottie on Android framework builds.
2018-07-10 fmalita@chromium.org [skottie] Apply fully opaque masks as clips


Created with:
  gclient setdep -r src/third_party/skia@a7a278205bb0

The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:847984 
TBR=robertphillips@chromium.org

Change-Id: I023a08a71bdc718b72194567bb18b5a493124479
Reviewed-on: https://chromium-review.googlesource.com/1131378
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#573731}
[modify] https://crrev.com/95287e9e54828d9e8144463dc25d2c458729010c/DEPS

Performance should return to previously levels or very close. I'll keep an eye on the graphs.
Unfortunately this broke screenshot_sync_tests on a couple of Android devices:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20FYI%20Release%20%28Nexus%209%29/5732
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Android%20FYI%20Release%20%28Nexus%206P%29/5476

It's surely just a logic error and possibly related to the DPI of the displays. Brian is it possible for you to triage this quickly? These tests are very important to preserve correctness.

Components: Internals>GPU>Testing
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a95d1589fd0a159f87f0f4805ff52dfd837492bd

commit a95d1589fd0a159f87f0f4805ff52dfd837492bd
Author: Brian Salomon <bsalomon@google.com>
Date: Tue Jul 10 20:32:20 2018

Revert "Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest."

This reverts commit 841c0dde5461c763fdd524c2078aafaf241444aa.

Reason for revert: Have to revert Chrome change

Original change's description:
> Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.
> 
> Skia currently has a driver workaround that performans antialiased
> rectangle edge equation evalution in the fragment shader rather than
> vertex shader on Adreno 3xx GPUs. This was done because the interpolation
> of values computed in the vertex shader lacks enough precision to pass
> these tests. However, doing so has a negative performance impact. Skia
> tests run with the workaround removed show that in practice the visual
> difference is insignificant. Moreover, cc::GLRenderer also incurs these
> same errors as it uses the same antialiasing approach without the
> workaround and has shipped on Adreno 3xx devices for years with no known
> complaints about antialiasing quality/accuracy. We're better off
> reclaiming the performance and accepting a small hit to antialiasing
> accuracy.
> 
> Bug:  847984 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: If1e62e0780a752179a75ddd57ccc7b24128c68ac
> Reviewed-on: https://chromium-review.googlesource.com/1129041
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Cr-Commit-Position: refs/heads/master@{#573695}

TBR=bsalomon@google.com,zmo@chromium.org,kbr@chromium.org

Change-Id: Ib787aba372282504f047c129a9f0b3750668d1b0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847984 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1132274
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Cr-Commit-Position: refs/heads/master@{#573888}
[modify] https://crrev.com/a95d1589fd0a159f87f0f4805ff52dfd837492bd/content/test/gpu/gpu_tests/screenshot_sync_integration_test.py

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 10

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

commit 1b112cc5604d294427c0d51f194ab7fb3067d9e2
Author: Brian Salomon <bsalomon@google.com>
Date: Tue Jul 10 20:34:26 2018

Revert "Remove interpolants are inaccurate workaround for Adreno 3xx."

This reverts commit a7a278205bb040061cb4ba46839efe18635c7edc.

Reason for revert: Chrome change had to be reverted because of new failures on Nexus 9 and 6P.

Original change's description:
> Remove interpolants are inaccurate workaround for Adreno 3xx.
> 
> The chrome screenshot unit test that led to adding this workaround has
> been adjusted to avoid testing AA edges of rendered rectangles. We're
> accepting the inaccuracy in favor of increased performance.
> 
> Chrome change: https://chromium-review.googlesource.com/c/chromium/src/+/1129041
> 
> Bug:  chromium:847984 
> Change-Id: I9b714ade2a67e956ebb2773ebe3b8632dc3a50c6
> Reviewed-on: https://skia-review.googlesource.com/140180
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Auto-Submit: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

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

Change-Id: Ic6b0e5a343556e59d144852f9fa5b561302f9781
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:847984 
Reviewed-on: https://skia-review.googlesource.com/140380
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

[modify] https://crrev.com/1b112cc5604d294427c0d51f194ab7fb3067d9e2/src/gpu/GrShaderCaps.cpp
[modify] https://crrev.com/1b112cc5604d294427c0d51f194ab7fb3067d9e2/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/1b112cc5604d294427c0d51f194ab7fb3067d9e2/src/gpu/ops/GrTextureOp.cpp
[modify] https://crrev.com/1b112cc5604d294427c0d51f194ab7fb3067d9e2/src/gpu/GrShaderCaps.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/775a3bbc9b1a2237d4f6acdd1e8d1febfb382c47

commit 775a3bbc9b1a2237d4f6acdd1e8d1febfb382c47
Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jul 11 01:36:39 2018

Roll src/third_party/skia c69c4410be7d..e8d674dd7cef (9 commits)

https://skia.googlesource.com/skia.git/+log/c69c4410be7d..e8d674dd7cef


git log c69c4410be7d..e8d674dd7cef --date=short --no-merges --format='%ad %ae %s'
2018-07-11 angle-skia-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com Roll third_party/externals/angle2 d8b1c5c5bba2..569b9cb983b1 (16 commits)
2018-07-10 bungeman@google.com Move SkRefSet / pipe code to smart pointer.
2018-07-10 herb@google.com Revert "Use new SkGlyphIDSet"
2018-07-10 herb@google.com Use new SkGlyphIDSet
2018-07-10 bungeman@google.com Move SkWriteBuffer to smart pointers.
2018-07-10 brianosman@google.com Revert "transform paint color to dst colorspace"
2018-07-10 jcgregorio@google.com Revert "added skeletal animation support to GPU backend"
2018-07-10 bsalomon@google.com Revert "Remove interpolants are inaccurate workaround for Adreno 3xx."
2018-07-10 ruiqimao@google.com added skeletal animation support to GPU backend


Created with:
  gclient setdep -r src/third_party/skia@e8d674dd7cef

The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:847984 
TBR=robertphillips@chromium.org

Change-Id: Ibd19c78f436bc166f9eed2742bfb01c416a9d029
Reviewed-on: https://chromium-review.googlesource.com/1132440
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#574033}
[modify] https://crrev.com/775a3bbc9b1a2237d4f6acdd1e8d1febfb382c47/DEPS

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23370775c415c21f4de1c7d97dd5645fe2b7e818

commit 23370775c415c21f4de1c7d97dd5645fe2b7e818
Author: Brian Salomon <bsalomon@google.com>
Date: Wed Jul 11 22:15:39 2018

Reland "Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest."

This is a reland of 841c0dde5461c763fdd524c2078aafaf241444aa

The updated change uses window.devicePixelRatio to determine the inset.
It also accounts for the absolute position used by the "WithDivs"
variations of this test. One additional device pixel of offset is used
to make this pass on mac_chromium_rel_ng. This is probably necessary
because either window.devicePixelRatio reports 1 incorrectly or the
edge antialiasing covers slightly more than one pixel. This wasn't
investigated.

Original change's description:
> Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.
>
> Skia currently has a driver workaround that performans antialiased
> rectangle edge equation evalution in the fragment shader rather than
> vertex shader on Adreno 3xx GPUs. This was done because the interpolation
> of values computed in the vertex shader lacks enough precision to pass
> these tests. However, doing so has a negative performance impact. Skia
> tests run with the workaround removed show that in practice the visual
> difference is insignificant. Moreover, cc::GLRenderer also incurs these
> same errors as it uses the same antialiasing approach without the
> workaround and has shipped on Adreno 3xx devices for years with no known
> complaints about antialiasing quality/accuracy. We're better off
> reclaiming the performance and accepting a small hit to antialiasing
> accuracy.
>
> Bug:  847984 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: If1e62e0780a752179a75ddd57ccc7b24128c68ac
> Reviewed-on: https://chromium-review.googlesource.com/1129041
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Cr-Commit-Position: refs/heads/master@{#573695}

Bug:  847984 
Change-Id: Ie5e685e8d5e0afbcf4c54ea3186ee68d238464ba
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1132574
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574369}
[modify] https://crrev.com/23370775c415c21f4de1c7d97dd5645fe2b7e818/content/test/gpu/gpu_tests/screenshot_sync_integration_test.py

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbaa2f11a4e9f5ef04eeb8cf0dc2acfdbaa79f56

commit cbaa2f11a4e9f5ef04eeb8cf0dc2acfdbaa79f56
Author: Brian Salomon <bsalomon@google.com>
Date: Thu Jul 12 03:18:56 2018

Revert "Reland "Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.""

This reverts commit 23370775c415c21f4de1c7d97dd5645fe2b7e818.

Reason for revert: still fails on Nexus 9

Original change's description:
> Reland "Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest."
> 
> This is a reland of 841c0dde5461c763fdd524c2078aafaf241444aa
> 
> The updated change uses window.devicePixelRatio to determine the inset.
> It also accounts for the absolute position used by the "WithDivs"
> variations of this test. One additional device pixel of offset is used
> to make this pass on mac_chromium_rel_ng. This is probably necessary
> because either window.devicePixelRatio reports 1 incorrectly or the
> edge antialiasing covers slightly more than one pixel. This wasn't
> investigated.
> 
> Original change's description:
> > Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.
> >
> > Skia currently has a driver workaround that performans antialiased
> > rectangle edge equation evalution in the fragment shader rather than
> > vertex shader on Adreno 3xx GPUs. This was done because the interpolation
> > of values computed in the vertex shader lacks enough precision to pass
> > these tests. However, doing so has a negative performance impact. Skia
> > tests run with the workaround removed show that in practice the visual
> > difference is insignificant. Moreover, cc::GLRenderer also incurs these
> > same errors as it uses the same antialiasing approach without the
> > workaround and has shipped on Adreno 3xx devices for years with no known
> > complaints about antialiasing quality/accuracy. We're better off
> > reclaiming the performance and accepting a small hit to antialiasing
> > accuracy.
> >
> > Bug:  847984 
> > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> > Change-Id: If1e62e0780a752179a75ddd57ccc7b24128c68ac
> > Reviewed-on: https://chromium-review.googlesource.com/1129041
> > Reviewed-by: Kenneth Russell <kbr@chromium.org>
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Cr-Commit-Position: refs/heads/master@{#573695}
> 
> Bug:  847984 
> Change-Id: Ie5e685e8d5e0afbcf4c54ea3186ee68d238464ba
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Reviewed-on: https://chromium-review.googlesource.com/1132574
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574369}

TBR=bsalomon@google.com,zmo@chromium.org,kbr@chromium.org

Change-Id: Idf353b940d79a85adba768e22593057d00310e47
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  847984 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1134120
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Cr-Commit-Position: refs/heads/master@{#574477}
[modify] https://crrev.com/cbaa2f11a4e9f5ef04eeb8cf0dc2acfdbaa79f56/content/test/gpu/gpu_tests/screenshot_sync_integration_test.py

Argh. If you hardcode a start_x of 10 (for example) can you get this revised test to pass on the Nexus 9? Would like to unblock you.

I don't have a N9 to test but I'm pretty sure the answer is yes. The failures were at x=5 so I'm pretty sure reducing start_x is what caused the bugs. Just pushed a new CL at https://chromium-review.googlesource.com/c/chromium/src/+/1138293
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7cd177f2d52db0565a1f92edf858ad6987fad96c

commit 7cd177f2d52db0565a1f92edf858ad6987fad96c
Author: Brian Salomon <bsalomon@google.com>
Date: Mon Jul 16 17:18:56 2018

Reland "Reland "Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.""

This is a reland of 23370775c415c21f4de1c7d97dd5645fe2b7e818

This reland undoes all the changes to start_x. start_X is now 10 as it was before
the original change.

Original change's description:
> Reland "Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest."
>
> This is a reland of 841c0dde5461c763fdd524c2078aafaf241444aa
>
> The updated change uses window.devicePixelRatio to determine the inset.
> It also accounts for the absolute position used by the "WithDivs"
> variations of this test. One additional device pixel of offset is used
> to make this pass on mac_chromium_rel_ng. This is probably necessary
> because either window.devicePixelRatio reports 1 incorrectly or the
> edge antialiasing covers slightly more than one pixel. This wasn't
> investigated.
>
> Original change's description:
> > Avoid testing antialiased boundary in ScreenshotSyncIntegrationTest.
> >
> > Skia currently has a driver workaround that performans antialiased
> > rectangle edge equation evalution in the fragment shader rather than
> > vertex shader on Adreno 3xx GPUs. This was done because the interpolation
> > of values computed in the vertex shader lacks enough precision to pass
> > these tests. However, doing so has a negative performance impact. Skia
> > tests run with the workaround removed show that in practice the visual
> > difference is insignificant. Moreover, cc::GLRenderer also incurs these
> > same errors as it uses the same antialiasing approach without the
> > workaround and has shipped on Adreno 3xx devices for years with no known
> > complaints about antialiasing quality/accuracy. We're better off
> > reclaiming the performance and accepting a small hit to antialiasing
> > accuracy.
> >
> > Bug:  847984 
> > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> > Change-Id: If1e62e0780a752179a75ddd57ccc7b24128c68ac
> > Reviewed-on: https://chromium-review.googlesource.com/1129041
> > Reviewed-by: Kenneth Russell <kbr@chromium.org>
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Cr-Commit-Position: refs/heads/master@{#573695}
>
> Bug:  847984 
> Change-Id: Ie5e685e8d5e0afbcf4c54ea3186ee68d238464ba
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Reviewed-on: https://chromium-review.googlesource.com/1132574
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#574369}

Bug:  847984 
Change-Id: I29a31001bd76436eba60df87ffe26fdb03d6d032
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1138293
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575322}
[modify] https://crrev.com/7cd177f2d52db0565a1f92edf858ad6987fad96c/content/test/gpu/gpu_tests/screenshot_sync_integration_test.py

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 17

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

commit 9b4bd599015445dbbfb85af3159404c1c5c21bc4
Author: Brian Salomon <bsalomon@google.com>
Date: Tue Jul 17 13:30:20 2018

Reland "Remove interpolants are inaccurate workaround for Adreno 3xx."

This is a reland of a7a278205bb040061cb4ba46839efe18635c7edc

Clean reland. Change to Chrome screenshot unit test has stuck. The previous
revert of that was the reason this had to be reverted.

Original change's description:
> Remove interpolants are inaccurate workaround for Adreno 3xx.
>
> The chrome screenshot unit test that led to adding this workaround has
> been adjusted to avoid testing AA edges of rendered rectangles. We're
> accepting the inaccuracy in favor of increased performance.
>
> Chrome change: https://chromium-review.googlesource.com/c/chromium/src/+/1129041
>
> Bug:  chromium:847984 
> Change-Id: I9b714ade2a67e956ebb2773ebe3b8632dc3a50c6
> Reviewed-on: https://skia-review.googlesource.com/140180
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Auto-Submit: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug:  chromium:847984 
Change-Id: I2dc1195f07bb27015b0a7a0fa6263d0e60a32a15
Reviewed-on: https://skia-review.googlesource.com/141761
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

[modify] https://crrev.com/9b4bd599015445dbbfb85af3159404c1c5c21bc4/src/gpu/GrShaderCaps.cpp
[modify] https://crrev.com/9b4bd599015445dbbfb85af3159404c1c5c21bc4/src/gpu/gl/GrGLCaps.cpp
[modify] https://crrev.com/9b4bd599015445dbbfb85af3159404c1c5c21bc4/src/gpu/ops/GrTextureOp.cpp
[modify] https://crrev.com/9b4bd599015445dbbfb85af3159404c1c5c21bc4/src/gpu/GrShaderCaps.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c034c9ed8e0fe5c0adb7f189fcffd7c6bf9082e1

commit c034c9ed8e0fe5c0adb7f189fcffd7c6bf9082e1
Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 17 18:29:24 2018

Roll src/third_party/skia 94fee93c9b23..b609e6dc6a02 (6 commits)

https://skia.googlesource.com/skia.git/+log/94fee93c9b23..b609e6dc6a02


git log 94fee93c9b23..b609e6dc6a02 --date=short --no-merges --format='%ad %ae %s'
2018-07-17 ruiqimao@google.com added byte and ubyte types to SKSL
2018-07-17 bsalomon@google.com Remove crash when shader linking fails.
2018-07-17 brianosman@google.com Remove SkTransferFunctionBehavior
2018-07-17 bsalomon@google.com Reland "Remove interpolants are inaccurate workaround for Adreno 3xx."
2018-07-17 caryclark@skia.org polish picture and rrect docs
2018-07-17 robertphillips@google.com Revert "Reduce arbitrary opList splitting when sorting"


Created with:
  gclient setdep -r src/third_party/skia@b609e6dc6a02

The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:861956 , chromium:847984 
TBR=bungeman@chromium.org

Change-Id: Ic674df2072110895987466c86baa4f96ab849cee
Reviewed-on: https://chromium-review.googlesource.com/1140413
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#575728}
[modify] https://crrev.com/c034c9ed8e0fe5c0adb7f189fcffd7c6bf9082e1/DEPS

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c034c9ed8e0fe5c0adb7f189fcffd7c6bf9082e1

commit c034c9ed8e0fe5c0adb7f189fcffd7c6bf9082e1
Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 17 18:29:24 2018

Roll src/third_party/skia 94fee93c9b23..b609e6dc6a02 (6 commits)

https://skia.googlesource.com/skia.git/+log/94fee93c9b23..b609e6dc6a02


git log 94fee93c9b23..b609e6dc6a02 --date=short --no-merges --format='%ad %ae %s'
2018-07-17 ruiqimao@google.com added byte and ubyte types to SKSL
2018-07-17 bsalomon@google.com Remove crash when shader linking fails.
2018-07-17 brianosman@google.com Remove SkTransferFunctionBehavior
2018-07-17 bsalomon@google.com Reland "Remove interpolants are inaccurate workaround for Adreno 3xx."
2018-07-17 caryclark@skia.org polish picture and rrect docs
2018-07-17 robertphillips@google.com Revert "Reduce arbitrary opList splitting when sorting"


Created with:
  gclient setdep -r src/third_party/skia@b609e6dc6a02

The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:861956 , chromium:847984 
TBR=bungeman@chromium.org

Change-Id: Ic674df2072110895987466c86baa4f96ab849cee
Reviewed-on: https://chromium-review.googlesource.com/1140413
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#575728}
[modify] https://crrev.com/c034c9ed8e0fe5c0adb7f189fcffd7c6bf9082e1/DEPS

Status: Fixed (was: Assigned)
Locally the performance has returned after then change in #22. AFAICT the bots aren't running these tests since 2018-06-30.

Sign in to add a comment