commit 974a1503273543719df2a3a18a967a4a785d49f1
author Jiawei Shao <jiawei.shao@intel.com> Wed May 08 23:59:03 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Wed May 08 23:59:03 2019 +0000
tree 48a902b5bdcc790e8af201c152a55f2bae09995b
parent f112ceca341d06a00089f3a71bfa18ab64d15287

OpenGL: Set default values for dynamic states before clear

This patch fixes a bug on the OpenGL backend by setting default values
for dynamic states (blend color, viewport, scissor rectangle, etc) before
clearing the attachments of the render pass so that the full area of the
attachments can be cleared correctly.

BUG=dawn:126, dawn:137, dawn:141
TEST=dawn_end2end_tests

Change-Id: I9ddf74a67413ed51f83a69a3dc2c2377a0622cac
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/7080
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/opengl/CommandBufferGL.cpp

diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index 03f2e11..686f9db 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -680,6 +680,13 @@
             }
         }
 
+        // Set defaults for dynamic state before executing clears and commands.
+        PersistentPipelineState persistentPipelineState;
+        persistentPipelineState.SetDefaultState();
+        glBlendColor(0, 0, 0, 0);
+        glViewport(0, 0, renderPass->width, renderPass->height);
+        glScissor(0, 0, renderPass->width, renderPass->height);
+
         // Clear framebuffer attachments as needed
         {
             for (uint32_t i : IterateBitSet(renderPass->colorAttachmentsSet)) {
@@ -725,17 +732,9 @@
         RenderPipeline* lastPipeline = nullptr;
         uint64_t indexBufferBaseOffset = 0;
 
-        PersistentPipelineState persistentPipelineState;
-
         PushConstantTracker pushConstants;
         InputBufferTracker inputBuffers;
 
-        // Set defaults for dynamic state
-        persistentPipelineState.SetDefaultState();
-        glBlendColor(0, 0, 0, 0);
-        glViewport(0, 0, renderPass->width, renderPass->height);
-        glScissor(0, 0, renderPass->width, renderPass->height);
-
         Command type;
         while (mCommands.NextCommandId(&type)) {
             switch (type) {

src/tests/end2end/RenderPassLoadOpTests.cpp

diff --git a/src/tests/end2end/RenderPassLoadOpTests.cpp b/src/tests/end2end/RenderPassLoadOpTests.cpp
index aadd9bc..5915b31 100644
--- a/src/tests/end2end/RenderPassLoadOpTests.cpp
+++ b/src/tests/end2end/RenderPassLoadOpTests.cpp
@@ -112,12 +112,6 @@
 
 // Tests clearing, loading, and drawing into color attachments
 TEST_P(RenderPassLoadOpTests, ColorClearThenLoadAndDraw) {
-    // TODO(jiawei.shao@intel.com): investigate why the test is flaky on Linux Intel OpenGL drivers.
-    // Currently we cannot collect PCI IDs from OpenGL drivers, we have to skip it on all OpenGL
-    // backends.
-    // Tracking bug: https://bugs.chromium.org/p/dawn/issues/detail?id=126
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsLinux());
-
     // Part 1: clear once, check to make sure it's cleared
     utils::ComboRenderPassDescriptor renderPassClearZero({renderTargetView});
     auto commandsClearZeroEncoder = device.CreateCommandEncoder();