Don't skip draw calls on zero-sized viewports.

If transform feedback is active, primitives still need to be rendered even
if no pixels are written to the framebuffer.

Instead of checking for active transform feedback, simply remove the draw
call skipping optimization since it is most likely an application mistake to
draw with a zero sized viewport and we shouldn't optimize for this case.

This change doesn't affect the clear calls because the viewport is set to the
framebuffer size which is non-zero.

BUG=angle:743

Change-Id: I04af9d6de5aad3040e3c6b3c24990e107e21ad36
Reviewed-on: https://chromium-review.googlesource.com/218508
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Shannon Woods <shannonwoods@chromium.org>
Tested-by: Geoff Lang <geofflang@chromium.org>

8 files changed, 128 insertions, 56 deletions

diff --git a/src/libGLESv2/Context.cpp b/src/libGLESv2/Context.cpp
index eb71e71c..f2852b06 100644
--- a/src/libGLESv2/Context.cpp
+++ b/src/libGLESv2/Context.cpp
@@ -1325,7 +1325,7 @@ bool Context::getIndexedQueryParameterInfo(GLenum target, GLenum *type, unsigned
 
 // Applies the render target surface, depth stencil surface, viewport rectangle and
 // scissor rectangle to the renderer
-bool Context::applyRenderTarget(GLenum drawMode, bool ignoreViewport)
+void Context::applyRenderTarget(GLenum drawMode, bool ignoreViewport)
 {
     Framebuffer *framebufferObject = mState.getDrawFramebuffer();
     ASSERT(framebufferObject && framebufferObject->completeness() == GL_FRAMEBUFFER_COMPLETE);
@@ -1334,15 +1334,10 @@ bool Context::applyRenderTarget(GLenum drawMode, bool ignoreViewport)
 
     float nearZ, farZ;
     mState.getDepthRange(&nearZ, &farZ);
-    if (!mRenderer->setViewport(mState.getViewport(), nearZ, farZ, drawMode, mState.getRasterizerState().frontFace,
-                                ignoreViewport))
-    {
-        return false;
-    }
+    mRenderer->setViewport(mState.getViewport(), nearZ, farZ, drawMode, mState.getRasterizerState().frontFace,
+                           ignoreViewport);
 
     mRenderer->setScissorRectangle(mState.getScissor(), mState.isScissorTestEnabled());
-
-    return true;
 }
 
 // Applies the fixed-function state (culling, depth test, alpha blending, stenciling, etc) to the Direct3D 9 device
@@ -1561,10 +1556,7 @@ Error Context::clear(GLbitfield mask)
 
     ClearParameters clearParams = mState.getClearParameters(mask);
 
-    if (!applyRenderTarget(GL_TRIANGLES, true))   // Clips the clear to the scissor rectangle but not the viewport
-    {
-        return Error(GL_NO_ERROR);
-    }
+    applyRenderTarget(GL_TRIANGLES, true);   // Clips the clear to the scissor rectangle but not the viewport
 
     return mRenderer->clear(clearParams, mState.getDrawFramebuffer());
 }
@@ -1595,10 +1587,7 @@ Error Context::clearBufferfv(GLenum buffer, int drawbuffer, const float *values)
         clearParams.depthClearValue = values[0];
     }
 
-    if (!applyRenderTarget(GL_TRIANGLES, true))   // Clips the clear to the scissor rectangle but not the viewport
-    {
-        return Error(GL_NO_ERROR);
-    }
+    applyRenderTarget(GL_TRIANGLES, true);   // Clips the clear to the scissor rectangle but not the viewport
 
     return mRenderer->clear(clearParams, mState.getDrawFramebuffer());
 }
@@ -1619,10 +1608,7 @@ Error Context::clearBufferuiv(GLenum buffer, int drawbuffer, const unsigned int
     clearParams.colorUIClearValue = ColorUI(values[0], values[1], values[2], values[3]);
     clearParams.colorClearType = GL_UNSIGNED_INT;
 
-    if (!applyRenderTarget(GL_TRIANGLES, true))   // Clips the clear to the scissor rectangle but not the viewport
-    {
-        return Error(GL_NO_ERROR);
-    }
+    applyRenderTarget(GL_TRIANGLES, true);   // Clips the clear to the scissor rectangle but not the viewport
 
     return mRenderer->clear(clearParams, mState.getDrawFramebuffer());
 }
@@ -1653,10 +1639,7 @@ Error Context::clearBufferiv(GLenum buffer, int drawbuffer, const int *values)
         clearParams.stencilClearValue = values[1];
     }
 
-    if (!applyRenderTarget(GL_TRIANGLES, true))   // Clips the clear to the scissor rectangle but not the viewport
-    {
-        return Error(GL_NO_ERROR);
-    }
+    applyRenderTarget(GL_TRIANGLES, true);   // Clips the clear to the scissor rectangle but not the viewport
 
     return mRenderer->clear(clearParams, mState.getDrawFramebuffer());
 }
@@ -1675,10 +1658,7 @@ Error Context::clearBufferfi(GLenum buffer, int drawbuffer, float depth, int ste
     clearParams.clearStencil = true;
     clearParams.stencilClearValue = stencil;
 
-    if (!applyRenderTarget(GL_TRIANGLES, true))   // Clips the clear to the scissor rectangle but not the viewport
-    {
-        return Error(GL_NO_ERROR);
-    }
+    applyRenderTarget(GL_TRIANGLES, true);   // Clips the clear to the scissor rectangle but not the viewport
 
     return mRenderer->clear(clearParams, mState.getDrawFramebuffer());
 }
@@ -1721,11 +1701,7 @@ void Context::drawArrays(GLenum mode, GLint first, GLsizei count, GLsizei instan
         return;
     }
 
-    if (!applyRenderTarget(mode, false))
-    {
-        return;
-    }
-
+    applyRenderTarget(mode, false);
     applyState(mode);
 
     GLenum err = mRenderer->applyVertexBuffer(programBinary, mState.getVertexArray()->getVertexAttributes(), mState.getVertexAttribCurrentValues(), first, count, instances);
@@ -1787,11 +1763,7 @@ void Context::drawElements(GLenum mode, GLsizei count, GLenum type,
         return;
     }
 
-    if (!applyRenderTarget(mode, false))
-    {
-        return;
-    }
-
+    applyRenderTarget(mode, false);
     applyState(mode);
 
     VertexArray *vao = mState.getVertexArray();
diff --git a/src/libGLESv2/Context.h b/src/libGLESv2/Context.h
index 7c5494b4..5cf4746e 100644
--- a/src/libGLESv2/Context.h
+++ b/src/libGLESv2/Context.h
@@ -234,7 +234,7 @@ class Context
     // TODO: std::array may become unavailable using older versions of GCC
     typedef std::array<unsigned int, IMPLEMENTATION_MAX_FRAMEBUFFER_ATTACHMENTS> FramebufferTextureSerialArray;
 
-    bool applyRenderTarget(GLenum drawMode, bool ignoreViewport);
+    void applyRenderTarget(GLenum drawMode, bool ignoreViewport);
     void applyState(GLenum drawMode);
     void applyShaders(ProgramBinary *programBinary, bool transformFeedbackActive);
     void applyTextures(SamplerType shaderType, Texture *textures[], TextureType *textureTypes, SamplerState *samplers,
diff --git a/src/libGLESv2/renderer/Renderer.h b/src/libGLESv2/renderer/Renderer.h
index 2d45cdbf..390cdfb7 100644
--- a/src/libGLESv2/renderer/Renderer.h
+++ b/src/libGLESv2/renderer/Renderer.h
@@ -125,7 +125,7 @@ class Renderer
                                       int stencilBackRef, bool frontFaceCCW) = 0;
 
     virtual void setScissorRectangle(const gl::Rectangle &scissor, bool enabled) = 0;
-    virtual bool setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
+    virtual void setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
                              bool ignoreViewport) = 0;
 
     virtual bool applyRenderTarget(gl::Framebuffer *frameBuffer) = 0;
diff --git a/src/libGLESv2/renderer/d3d/d3d11/Renderer11.cpp b/src/libGLESv2/renderer/d3d/d3d11/Renderer11.cpp
index 753a4cae..eb13343b 100644
--- a/src/libGLESv2/renderer/d3d/d3d11/Renderer11.cpp
+++ b/src/libGLESv2/renderer/d3d/d3d11/Renderer11.cpp
@@ -697,7 +697,7 @@ void Renderer11::setScissorRectangle(const gl::Rectangle &scissor, bool enabled)
     mForceSetScissor = false;
 }
 
-bool Renderer11::setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace, 
+void Renderer11::setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
                              bool ignoreViewport)
 {
     gl::Rectangle actualViewport = viewport;
@@ -724,11 +724,6 @@ bool Renderer11::setViewport(const gl::Rectangle &viewport, float zNear, float z
     dxViewport.MinDepth = actualZNear;
     dxViewport.MaxDepth = actualZFar;
 
-    if (dxViewport.Width <= 0 || dxViewport.Height <= 0)
-    {
-        return false;   // Nothing to render
-    }
-
     bool viewportChanged = mForceSetViewport || memcmp(&actualViewport, &mCurViewport, sizeof(gl::Rectangle)) != 0 ||
                            actualZNear != mCurNear || actualZFar != mCurFar;
 
@@ -758,7 +753,6 @@ bool Renderer11::setViewport(const gl::Rectangle &viewport, float zNear, float z
     }
 
     mForceSetViewport = false;
-    return true;
 }
 
 bool Renderer11::applyPrimitiveType(GLenum mode, GLsizei count)
diff --git a/src/libGLESv2/renderer/d3d/d3d11/Renderer11.h b/src/libGLESv2/renderer/d3d/d3d11/Renderer11.h
index ae6d2845..2d9e3034 100644
--- a/src/libGLESv2/renderer/d3d/d3d11/Renderer11.h
+++ b/src/libGLESv2/renderer/d3d/d3d11/Renderer11.h
@@ -72,7 +72,7 @@ class Renderer11 : public Renderer
                                       int stencilBackRef, bool frontFaceCCW);
 
     virtual void setScissorRectangle(const gl::Rectangle &scissor, bool enabled);
-    virtual bool setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
+    virtual void setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
                              bool ignoreViewport);
 
     virtual bool applyPrimitiveType(GLenum mode, GLsizei count);
diff --git a/src/libGLESv2/renderer/d3d/d3d9/Renderer9.cpp b/src/libGLESv2/renderer/d3d/d3d9/Renderer9.cpp
index cabec08d..e82971db 100644
--- a/src/libGLESv2/renderer/d3d/d3d9/Renderer9.cpp
+++ b/src/libGLESv2/renderer/d3d/d3d9/Renderer9.cpp
@@ -964,7 +964,7 @@ void Renderer9::setScissorRectangle(const gl::Rectangle &scissor, bool enabled)
     mForceSetScissor = false;
 }
 
-bool Renderer9::setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
+void Renderer9::setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
                             bool ignoreViewport)
 {
     gl::Rectangle actualViewport = viewport;
@@ -988,11 +988,6 @@ bool Renderer9::setViewport(const gl::Rectangle &viewport, float zNear, float zF
     dxViewport.MinZ = actualZNear;
     dxViewport.MaxZ = actualZFar;
 
-    if (dxViewport.Width <= 0 || dxViewport.Height <= 0)
-    {
-        return false;   // Nothing to render
-    }
-
     float depthFront = !gl::IsTriangleMode(drawMode) ? 0.0f : (frontFace == GL_CCW ? 1.0f : -1.0f);
 
     bool viewportChanged = mForceSetViewport || memcmp(&actualViewport, &mCurViewport, sizeof(gl::Rectangle)) != 0 ||
@@ -1045,7 +1040,6 @@ bool Renderer9::setViewport(const gl::Rectangle &viewport, float zNear, float zF
     }
 
     mForceSetViewport = false;
-    return true;
 }
 
 bool Renderer9::applyPrimitiveType(GLenum mode, GLsizei count)
diff --git a/src/libGLESv2/renderer/d3d/d3d9/Renderer9.h b/src/libGLESv2/renderer/d3d/d3d9/Renderer9.h
index 6dfad26f..e7dab81a 100644
--- a/src/libGLESv2/renderer/d3d/d3d9/Renderer9.h
+++ b/src/libGLESv2/renderer/d3d/d3d9/Renderer9.h
@@ -73,7 +73,7 @@ class Renderer9 : public Renderer
                                       int stencilBackRef, bool frontFaceCCW);
 
     virtual void setScissorRectangle(const gl::Rectangle &scissor, bool enabled);
-    virtual bool setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
+    virtual void setViewport(const gl::Rectangle &viewport, float zNear, float zFar, GLenum drawMode, GLenum frontFace,
                              bool ignoreViewport);
 
     virtual bool applyRenderTarget(gl::Framebuffer *frameBuffer);
diff --git a/tests/angle_tests/TransformFeedbackTest.cpp b/tests/angle_tests/TransformFeedbackTest.cpp
new file mode 100644
index 00000000..8bdb4e71
--- /dev/null
+++ b/tests/angle_tests/TransformFeedbackTest.cpp
@@ -0,0 +1,112 @@
+#include "ANGLETest.h"
+
+class TransformFeedbackTest : public ANGLETest
+{
+  protected:
+      TransformFeedbackTest()
+    {
+        setWindowWidth(128);
+        setWindowHeight(128);
+        setConfigRedBits(8);
+        setConfigGreenBits(8);
+        setConfigBlueBits(8);
+        setConfigAlphaBits(8);
+        setClientVersion(3);
+    }
+
+    virtual void SetUp()
+    {
+        ANGLETest::SetUp();
+
+        const std::string vertexShaderSource = SHADER_SOURCE
+        (
+            precision highp float;
+            attribute vec4 position;
+
+            void main()
+            {
+                gl_Position = position;
+            }
+        );
+
+        const std::string fragmentShaderSource = SHADER_SOURCE
+        (
+            precision highp float;
+
+            void main()
+            {
+                gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
+            }
+        );
+
+        mProgram = CompileProgram(vertexShaderSource, fragmentShaderSource);
+        if (mProgram == 0)
+        {
+            FAIL() << "shader compilation failed.";
+        }
+
+        glGenBuffers(1, &mTransformFeedbackBuffer);
+        mTransformFeedbackBufferSize = 1 << 24; // ~16MB
+        glBindBuffer(GL_TRANSFORM_FEEDBACK_BUFFER, mTransformFeedbackBuffer);
+        glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, mTransformFeedbackBufferSize, NULL, GL_STATIC_DRAW);
+
+        ASSERT_GL_NO_ERROR();
+    }
+
+    virtual void TearDown()
+    {
+        glDeleteProgram(mProgram);
+        glDeleteBuffers(1, &mTransformFeedbackBuffer);
+        ANGLETest::TearDown();
+    }
+
+    GLuint mProgram;
+
+    size_t mTransformFeedbackBufferSize;
+    GLuint mTransformFeedbackBuffer;
+};
+
+TEST_F(TransformFeedbackTest, ZeroSizedViewport)
+{
+    // Set the program's transform feedback varyings (just gl_Position)
+    const GLchar* transformFeedbackVaryings[] =
+    {
+        "gl_Position"
+    };
+    glTransformFeedbackVaryings(mProgram, ArraySize(transformFeedbackVaryings), transformFeedbackVaryings, GL_INTERLEAVED_ATTRIBS);
+    glLinkProgram(mProgram);
+
+    // Re-link the program
+    GLint linkStatus;
+    glGetProgramiv(mProgram, GL_LINK_STATUS, &linkStatus);
+    ASSERT_NE(linkStatus, 0);
+
+    glUseProgram(mProgram);
+
+    // Bind the buffer for transform feedback output and start transform feedback
+    glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
+    glBeginTransformFeedback(GL_TRIANGLES);
+
+    // Create a query to check how many primitives were written
+    GLuint primitivesWrittenQuery = 0;
+    glGenQueries(1, &primitivesWrittenQuery);
+    glBeginQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN, primitivesWrittenQuery);
+
+    // Set a viewport that would result in no pixels being written to the framebuffer and draw
+    // a quad
+    glViewport(0, 0, 0, 0);
+
+    drawQuad(mProgram, "position", 0.5f);
+
+    // End the query and transform feedkback
+    glEndQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN);
+    glEndTransformFeedback();
+
+    // Check how many primitives were written and verify that some were written even if
+    // no pixels were rendered
+    GLuint primitivesWritten = 0;
+    glGetQueryObjectuiv(primitivesWrittenQuery, GL_QUERY_RESULT_EXT, &primitivesWritten);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_EQ(primitivesWritten, 2);
+}