New issue
Advanced search Search tips

Issue 894806 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Flaky-Test: SkiaOutputSurfaceImplTest.CopyOutOutside

Blocking:
issue 644851
issue 899750



Sign in to add a comment

Add unittests for SkiaOutputSufaceImpl

Project Member Reported by penghuang@chromium.org, Oct 12

Issue description

Add unittests for SkiaOutputSufaceImpl
 
Description: Show this description
Blocking: 898270
Labels: vulkanize
Labels: -vulkanize Proj-Vulkanize
Blocking: -898270 899750
Summary: Add unittests for SkiaOutputSufaceImpl (was: Add unittests for SiaOutputSufaceImpl)
Blocking: 644851
Hi, peng, I am willing to help on this unit test. Are U OK with this?
Owner: xing...@intel.com
That will be great. Thanks.
Status: Started (was: Assigned)
As suggested by peng, more cases are required:

Maybe add a test for Reshape() method. However you can add it in a followup CL.

It could be similar to SubmitPaint,
Something line:

TestReshape() {
 Paint something,
 Change to a new size;
 Paint something.
 ...
};

And maybe test for Paint render pass as well.


https://chromium-review.googlesource.com/c/chromium/src/+/1375393/8/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc#213

Project Member

Comment 10 by bugdroid1@chromium.org, Dec 17

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

commit c5679d550b2e60bbfe7be9f900daec5f672b9391
Author: Xu Xing <xing.xu@intel.com>
Date: Mon Dec 17 03:16:42 2018

Add SkiaOutputSurfaceImpl unittest

Bug: 894806
Change-Id: I3158c75458ceee8c2502ed5bf09a56b6f3996524
Reviewed-on: https://chromium-review.googlesource.com/c/1375393
Commit-Queue: Xing Xu <xing.xu@intel.com>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617041}
[modify] https://crrev.com/c5679d550b2e60bbfe7be9f900daec5f672b9391/components/viz/service/BUILD.gn
[add] https://crrev.com/c5679d550b2e60bbfe7be9f900daec5f672b9391/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 17

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

commit c5fa8e36423a8ebf133fc882932e5d06d75e431c
Author: Xing Xu <xing.xu@intel.com>
Date: Mon Dec 17 05:58:41 2018

Revert "Add SkiaOutputSurfaceImpl unittest"

This reverts commit c5679d550b2e60bbfe7be9f900daec5f672b9391.

Reason for revert: The CheckSyncTokenOnGpuThread is mistakenly checked and maybe flaky.

Original change's description:
> Add SkiaOutputSurfaceImpl unittest
> 
> Bug: 894806
> Change-Id: I3158c75458ceee8c2502ed5bf09a56b6f3996524
> Reviewed-on: https://chromium-review.googlesource.com/c/1375393
> Commit-Queue: Xing Xu <xing.xu@intel.com>
> Reviewed-by: Peng Huang <penghuang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#617041}

TBR=penghuang@chromium.org,weiliangc@chromium.org,xing.xu@intel.com

Change-Id: I48ea9387a0ee929b2b1ff7fb999aad8ba669b0da
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 894806
Reviewed-on: https://chromium-review.googlesource.com/c/1379695
Reviewed-by: Xing Xu <xing.xu@intel.com>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#617053}
[modify] https://crrev.com/c5fa8e36423a8ebf133fc882932e5d06d75e431c/components/viz/service/BUILD.gn
[delete] https://crrev.com/cbe2bdf3057e4a56bdbbb31e3862157056672ec2/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 19

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

commit 9e87502f43cfb9a3e5fcd9554eb7b8b049eb47d4
Author: Xu Xing <xing.xu@intel.com>
Date: Wed Dec 19 13:20:31 2018

Reland "Add SkiaOutputSurfaceImpl unittest"

Main change:
Post CheckSyncTokenOnGpuThread by gpu::Scheduler::ScheduleTask.

Reason:
SubmitPaint doesn't guarantee that SyncToken is released, so
if we check SyncToken on GpuThread directly, it may be flaky. Instead,
gpu::Scheduler::ScheduleTask will wait on the SyncToken and ensures that
the SyncToken is released.

Original CL: https://chromium-review.googlesource.com/c/1375393

Bug: 894806
Change-Id: Id9eeac79196664b8b56da625f6f4fbd56a969554
Reviewed-on: https://chromium-review.googlesource.com/c/1379701
Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#617813}
[modify] https://crrev.com/9e87502f43cfb9a3e5fcd9554eb7b8b049eb47d4/components/viz/service/BUILD.gn
[add] https://crrev.com/9e87502f43cfb9a3e5fcd9554eb7b8b049eb47d4/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 4

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

commit 8f13336d43ca10ee10bd22ad02aab11c7d799128
Author: Xu Xing <xing.xu@intel.com>
Date: Fri Jan 04 01:05:40 2019

Add reshape and copy outside case for SkiaOutputSurfaceImplTest

Bug: 894806
Change-Id: Id76b1a229b9583e7d7304b4f5262e2b8d9bbbbb3
Reviewed-on: https://chromium-review.googlesource.com/c/1388020
Commit-Queue: Xing Xu <xing.xu@intel.com>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619841}
[modify] https://crrev.com/8f13336d43ca10ee10bd22ad02aab11c7d799128/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Jan 4

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

commit 4c0d4779099e40a82e26e8d6e7517365f48f69c0
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Jan 04 03:17:24 2019

Revert "Add reshape and copy outside case for SkiaOutputSurfaceImplTest"

This reverts commit 8f13336d43ca10ee10bd22ad02aab11c7d799128.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 619841 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vOGYxMzMzNmQ0M2NhMTBlZTEwYmQyMmFkMDJhYWIxMWM3ZDc5OTEyOAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.mac/Mac10.13%20Tests/8493

Sample Failed Step: viz_unittests on (none) GPU on Mac

Sample Flaky Test: SkiaOutputSurfaceImplTest.CopyOutOutside

Original change's description:
> Add reshape and copy outside case for SkiaOutputSurfaceImplTest
> 
> Bug: 894806
> Change-Id: Id76b1a229b9583e7d7304b4f5262e2b8d9bbbbb3
> Reviewed-on: https://chromium-review.googlesource.com/c/1388020
> Commit-Queue: Xing Xu <xing.xu@intel.com>
> Reviewed-by: Peng Huang <penghuang@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619841}

Change-Id: Ic8105579dffefcf7f13cfb12e30e350c6ffee6e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 894806,  919012 
Reviewed-on: https://chromium-review.googlesource.com/c/1395719
Cr-Commit-Position: refs/heads/master@{#619874}
[modify] https://crrev.com/4c0d4779099e40a82e26e8d6e7517365f48f69c0/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

 Issue 919012  has been merged into this issue.
Project Member

Comment 16 by bugdroid1@chromium.org, Yesterday (29 hours ago)

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

commit bf925d474e3b4b33acff76dd5aeccedf9c9c81dd
Author: Xu Xing <xing.xu@intel.com>
Date: Tue Jan 22 03:32:26 2019

Reland "Add reshape and copy outside case for SkiaOutputSurfaceImplTest"

Original CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1388020

Bug: 894806
Change-Id: Iefa89bc1ebba9c8146bc7a8bc26647635e61ed01
Reviewed-on: https://chromium-review.googlesource.com/c/1397425
Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: Xing Xu <xing.xu@intel.com>
Cr-Commit-Position: refs/heads/master@{#624719}
[modify] https://crrev.com/bf925d474e3b4b33acff76dd5aeccedf9c9c81dd/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Yesterday (27 hours ago)

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

commit 10f34c9d903c89c4960f540162b75138e2c5d4df
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Jan 22 04:58:25 2019

Revert "Reland "Add reshape and copy outside case for SkiaOutputSurfaceImplTest""

This reverts commit bf925d474e3b4b33acff76dd5aeccedf9c9c81dd.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 624719 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2JmOTI1ZDQ3NGUzYjRiMzNhY2ZmNzZkZDVhZWNjZWRmOWM5YzgxZGQM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.win/Win%207%20Tests%20x64%20%281%29/47326

Sample Failed Step: viz_unittests

Original change's description:
> Reland "Add reshape and copy outside case for SkiaOutputSurfaceImplTest"
> 
> Original CL:
> https://chromium-review.googlesource.com/c/chromium/src/+/1388020
> 
> Bug: 894806
> Change-Id: Iefa89bc1ebba9c8146bc7a8bc26647635e61ed01
> Reviewed-on: https://chromium-review.googlesource.com/c/1397425
> Reviewed-by: Peng Huang <penghuang@chromium.org>
> Commit-Queue: Xing Xu <xing.xu@intel.com>
> Cr-Commit-Position: refs/heads/master@{#624719}

Change-Id: Iaa8605c9c561e6b144c25de190403b92623162f4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 894806
Reviewed-on: https://chromium-review.googlesource.com/c/1426477
Cr-Commit-Position: refs/heads/master@{#624726}
[modify] https://crrev.com/10f34c9d903c89c4960f540162b75138e2c5d4df/components/viz/service/display_embedder/skia_output_surface_impl_unittest.cc

Sign in to add a comment