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

Issue 843290 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 844897
issue 848084



Sign in to add a comment

Draw and swap after primary surface is available

Project Member Reported by samans@chromium.org, May 15 2018

Issue description

If fallback SurfaceId was used during aggregation and a newer Surface
becomes available later on, make sure we mark the display as damaged
to force a draw and swap.
 

Comment 1 by samans@chromium.org, May 15 2018

Owner: samans@chromium.org
Status: assigned (was: Untriaged)

Comment 2 by samans@chromium.org, May 17 2018

Description: Show this description

Comment 3 by samans@chromium.org, May 17 2018

Summary: Draw and swap after primary surface is available (was: Fix Display::SurfaceDamaged)
Project Member

Comment 4 by bugdroid1@chromium.org, May 19 2018

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

commit bdc9b10b813d29194165c72c0587ed1e5d556654
Author: Saman Sami <samans@chromium.org>
Date: Sat May 19 22:04:13 2018

viz: Draw and swap after primary surface is available

If fallback SurfaceId was used during aggregation and a newer Surface
becomes available later on, make sure we mark the display as damaged
to force a draw and swap.

Bug:  843290 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ie0d46b547ab0c983c21c0fe003af3fbf90759aba
Reviewed-on: https://chromium-review.googlesource.com/1062585
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560181}
[modify] https://crrev.com/bdc9b10b813d29194165c72c0587ed1e5d556654/components/viz/service/display/display.cc
[modify] https://crrev.com/bdc9b10b813d29194165c72c0587ed1e5d556654/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/bdc9b10b813d29194165c72c0587ed1e5d556654/components/viz/service/display/surface_aggregator.h
[modify] https://crrev.com/bdc9b10b813d29194165c72c0587ed1e5d556654/components/viz/service/display/surface_aggregator_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 21 2018

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

commit 9921a22f4af4a87db15b22996df66b19b79b328f
Author: James Darpinian <jdarpinian@chromium.org>
Date: Mon May 21 02:29:01 2018

Revert "viz: Draw and swap after primary surface is available"

This reverts commit bdc9b10b813d29194165c72c0587ed1e5d556654.
Originally reviewed at: https://chromium-review.googlesource.com/c/chromium/src/+/1062585

Suspected of breaking WebGL conformance tests on FYI waterfall:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20FYI%20Release%20(AMD)/1241
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20FYI%20Release%20(NVIDIA)/1013
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20FYI%20x64%20Release%20(NVIDIA)/1071
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Release%20(Intel)/3022
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac%20FYI%20Experimental%20Retina%20Release%20(AMD)/1728

Bug:  844897 , 843290 
No-Try: true
Tbr: fsamuel@chromium.org
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I7994694e057a08f2a27d4aedcfa369fd350f32b2
Reviewed-on: https://chromium-review.googlesource.com/1067142
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560218}
[modify] https://crrev.com/9921a22f4af4a87db15b22996df66b19b79b328f/components/viz/service/display/display.cc
[modify] https://crrev.com/9921a22f4af4a87db15b22996df66b19b79b328f/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/9921a22f4af4a87db15b22996df66b19b79b328f/components/viz/service/display/surface_aggregator.h
[modify] https://crrev.com/9921a22f4af4a87db15b22996df66b19b79b328f/components/viz/service/display/surface_aggregator_unittest.cc

Comment 6 by kbr@chromium.org, May 22 2018

Cc: kbr@chromium.org
Components: Internals>Services>Viz
The flaky test failures that were introduced by this CL are concerning because they're very difficult to track down after the fact. https://chromium-review.googlesource.com/1069598 in progress running more GPU tests against Viz changes.

Comment 7 by kbr@chromium.org, May 22 2018

Blockedon: 844897
Project Member

Comment 8 by bugdroid1@chromium.org, May 22 2018

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

commit 3cb37960cf3a2e83edaf6f460e2316278eefb3c3
Author: Kenneth Russell <kbr@chromium.org>
Date: Tue May 22 21:57:16 2018

Run WebGL 2.0 conformance tests against Viz changes.

This will increase the likelihood that intermittent failures caused by
changes in this directory will be caught on the commit queue.

Bug:  843290 
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: Ie4d230c0290706c956d88f483b82b98f3a62f4bf
Reviewed-on: https://chromium-review.googlesource.com/1069598
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560799}
[modify] https://crrev.com/3cb37960cf3a2e83edaf6f460e2316278eefb3c3/components/viz/PRESUBMIT.py

Comment 9 by kbr@chromium.org, May 31 2018

Blockedon: 848084
Project Member

Comment 10 by bugdroid1@chromium.org, May 31 2018

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

commit 8099105ecb00185a0825f989d44c109ba52c4c44
Author: Kenneth Russell <kbr@chromium.org>
Date: Thu May 31 01:01:06 2018

Revert "Run WebGL 2.0 conformance tests against Viz changes."

This reverts commit 3cb37960cf3a2e83edaf6f460e2316278eefb3c3.

Reason for revert: there are a high volume of changes in this subdirectory right now, causing overloading of the GPU bots.

Bug: 848084,  843290 
No-Try: True
Tbr: fsamuel@chromium.org
Tbr: danakj@chromium.org
Tbr: samans@chromium.org
Tbr: sadrul@chromium.org

Original change's description:
> Run WebGL 2.0 conformance tests against Viz changes.
> 
> This will increase the likelihood that intermittent failures caused by
> changes in this directory will be caught on the commit queue.
> 
> Bug:  843290 
> 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: Ie4d230c0290706c956d88f483b82b98f3a62f4bf
> Reviewed-on: https://chromium-review.googlesource.com/1069598
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Commit-Queue: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#560799}

TBR=kbr@chromium.org,fsamuel@chromium.org,samans@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  843290 
Change-Id: Id5dddfac8e7cdb271628de739e8179459b368c49
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/1080075
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Commit-Queue: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563098}
[modify] https://crrev.com/8099105ecb00185a0825f989d44c109ba52c4c44/components/viz/PRESUBMIT.py

Cc: fsam...@chromium.org
So I think the problem is that SurfaceModified in SurfaceManager::SurfaceActivated is returning true so surface->RunDrawCallback() is not immediately called. The expectation is that the draw callback will be run on the next DrawAndSwap, however the test is timing out so I'm assuming that's not happening. I need to get myself more familiar with the display scheduler before I can fix this.
Blocking: 854798
Labels: -Pri-3 Pri-1
Bumping to P1. Any update on this, Saman?
I'm actively working on it, but I haven't figured out a fix yet.
Blocking: -854798
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 4

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

commit 902374da5c238be851606c5683991deec1f2bc86
Author: Saman Sami <samans@chromium.org>
Date: Wed Jul 04 23:04:45 2018

Reland "viz: Draw and swap after primary surface is available"

Relands bdc9b10b but also ensures every all CompositorFrame acks are
satisfied on the next draw even if the CompositorFrame is not drawn.

If fallback SurfaceId was used during aggregation and a newer Surface
becomes available later on, make sure we mark the display as damaged
to force a draw and swap.

Bug:  843290 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ifa913bdc0112f28c8bae083858d834a6aa27e0a0
Reviewed-on: https://chromium-review.googlesource.com/1120435
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572673}
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/common/surfaces/local_surface_id.cc
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/common/surfaces/local_surface_id.h
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/service/display/display.cc
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/service/display/display.h
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/service/display/surface_aggregator.h
[modify] https://crrev.com/902374da5c238be851606c5683991deec1f2bc86/components/viz/service/display/surface_aggregator_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment