Texture Draw Calls Coalescing

cc/gl_renderer.cc

Index: cc/gl_renderer.cc
diff --git a/cc/gl_renderer.cc b/cc/gl_renderer.cc
index 94f8d83124f945b5118af3f5b1d07a7bbcdc5fba..3dc21ee18b2dc10a78ac98d6de48599429146bf4 100644
--- a/cc/gl_renderer.cc
+++ b/cc/gl_renderer.cc
@@ -59,6 +59,58 @@ bool needsIOSurfaceReadbackWorkaround()
 
 } // anonymous namespace
 
+// A basic vector class, used for vertex transforms and OpenGL vector upload
+struct Vector {

[jamesr, 2012/11/28 00:00:37]

cc/math_util.h has a HomogeneousCoordinate that's a 4-vector.  Shawn - do you
think we should extend this?

If we don't use the w component we could just use gfx::Vector3dF which defines
addition and scalar multiplication already

[whunt, 2012/11/28 00:29:08]

In truth we no longer need any math because I'm not skinning on the host any
more because VBO upload is slow, I just need a container that stores 4 floats
(which gfx::Vector3dF is not).  When we were skinning on the host we did need
the w component.

On 2012/11/28 00:00:37, jamesr wrote:

[shawnsingh, 2012/11/28 19:44:16]

I think we should do the following:
(1) merge this Vector class with HomogeneousCoordinate.

(2) move all this math to MathUtil.*

(3) add one trivial unit test to make sure that the Matrix * Vector multiply
doesn't accidentally regress / transpose if someone else modifies it

+    float x, y, z, w;
+    Vector() {}
+    Vector(float x, float y, float z = 0.0f, float w = 0.0f)
+        : x(x), y(y), z(z), w(w) { }
+};
+
+// A basic matrix class, used for vertex transforms and OpenGL matrix upload
+struct Matrix {

[danakj, 2012/11/27 23:47:47]

Is there some reason to not just use gfx::Transform?

[whunt, 2012/11/28 00:29:08]

This only works if gfx::Transform is of float type (it is packed and organized
in the correct format).  It cannot be guaranteed that gfx::Transform is not in
double format and thus will not upload. 

On 2012/11/27 23:47:47, danakj wrote:

[danakj, 2012/11/28 00:43:58]

Oh okay, thanks for the explanation. Let's call it FloatMatrix..or..something
that makes its purpose/difference clear.

+    Vector x, y, z, w;
+    Matrix() {}
+    Matrix(const Vector& x, const Vector& y, const Vector& z, const Vector& w)
+        : x(x), y(y), z(z), w(w) { }
+};
+
+// An alternate vector constructor that defaults to w = 1.0f rather than w = 0.0f
+inline Vector const Point(float x, float y, float z = 0.0f, float w = 1.0f) { return Vector(x, y, z, w); }
+
+// The most basic math required to implement vertex transforms
+inline Vector const operator +(const Vector& a, const Vector& b) { return Vector(a.x + b.x, a.y + b.y, a.z + b.z, a.w + b.w); }
+inline Vector const operator *(const Vector& a, float b) { return Vector(a.x * b, a.y * b, a.z * b, a.w * b); }
+inline Vector const operator *(const Matrix& m, const Vector& v) { return m.x * v.x + m.y * v.y + m.z * v.z + m.w * v.w; }
+
+// A cache for storing textured quads to be drawn.  Stores the minimum required
+// data to tell if two back to back draws only differ in their transform. Quads
+// that only differ by transform may be coalesced into a single draw call.
+struct TexturedQuadDrawCache

[jamesr, 2012/11/28 00:00:37]

Could we put this in its own file?  Adding a new file is fairly simple - just
add the file and add it to the appropriate part of cc/cc.gyp - you don't have to
muck about with individual build systems or anything like that

[whunt, 2012/11/28 00:29:08]

Sure, no problem.

On 2012/11/28 00:00:37, jamesr wrote:

+{
+    // Values tracked to determine if textured quads may be coalesced
+    int programID;
+    int resourceID;
+    float alpha;
+    bool usePremultipliedAlpha;
+    bool needsBlending;
+   
+    // Information about the program binding that is required to draw
+    int alphaLocation;
+    int uvXformLocation;
+    int matrixLocation;
+   
+    // A cache for the coalesced quad data
+    std::vector<Vector> uvXformData;
+    std::vector<Matrix> matrixData;
+   
+    TexturedQuadDrawCache()
+        : programID(0)
+        , resourceID(0)

[jamesr, 2012/11/28 00:00:37]

could you initialize all scalars and move the constructor to the top of the
struct declaration?

[whunt, 2012/11/28 00:29:08]

Constructor moved.

About initialization.  I *can* initialize these values.  However the intended
use of this struct is to assume that if programID is 0 then all fields are
undefined.  The "appropriate" way to implement this would be to have a get
method for each field that throws an exception if programID is 0 and have a
setter that sets everything at the same time.

As it stands nothing is ever read to or written from unless programID is
non-zero.  I can initialize them as a matter of course but it's not really the
intended behavior.

On 2012/11/28 00:00:37, jamesr wrote:

+    {
+    }
+};
+
 scoped_ptr<GLRenderer> GLRenderer::create(RendererClient* client, ResourceProvider* resourceProvider)
 {
     scoped_ptr<GLRenderer> renderer(make_scoped_ptr(new GLRenderer(client, resourceProvider)));
@@ -79,6 +131,7 @@ GLRenderer::GLRenderer(RendererClient* client, ResourceProvider* resourceProvide
     , m_isUsingBindUniform(false)
     , m_visible(true)
     , m_isScissorEnabled(false)
+    , m_drawCache(new TexturedQuadDrawCache())

[jamesr, 2012/11/28 00:00:37]

if we had TextureQuadDrawCache in its own header, we could just have a member of
type TexturedQuadDrawCache instead of a pointer and cut down on a bit of
boilerplate and a heap allocation

[whunt, 2012/11/28 00:29:08]

This is true, but that would make everything that depended on gl_renderer.h
*also* depend on the new file (we would have to include the new file in
gl_renderer.h rather than in gl_renderer.cc.  This both increases build times
and breaks encapsulation.

I can see it both ways.

On 2012/11/28 00:00:37, jamesr wrote:

 {
     DCHECK(m_context);
 }
@@ -243,6 +296,8 @@ void GLRenderer::beginDrawingFrame(DrawingFrame& frame)
     GLC(m_context, m_context->enable(GL_BLEND));
     GLC(m_context, m_context->blendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA));
     GLC(context(), context()->activeTexture(GL_TEXTURE0));
+    m_blendShadow = true;
+    m_programShadow = 0;
 }
 
 void GLRenderer::doNoOp()
@@ -254,11 +309,11 @@ void GLRenderer::doNoOp()
 void GLRenderer::drawQuad(DrawingFrame& frame, const DrawQuad* quad)
 {
     DCHECK(quad->rect.Contains(quad->visible_rect));
-
-    if (quad->ShouldDrawWithBlending())
-        GLC(m_context, m_context->enable(GL_BLEND));
-    else
-        GLC(m_context, m_context->disable(GL_BLEND));
+    if (quad->material != DrawQuad::TEXTURE_CONTENT)
+    {
+        drawTextureQuadFlush();
+        setBlendEnabled(quad->ShouldDrawWithBlending());
+    }
 
     switch (quad->material) {
     case DrawQuad::INVALID:
@@ -283,7 +338,7 @@ void GLRenderer::drawQuad(DrawingFrame& frame, const DrawQuad* quad)
         drawStreamVideoQuad(frame, StreamVideoDrawQuad::MaterialCast(quad));
         break;
     case DrawQuad::TEXTURE_CONTENT:
-        drawTextureQuad(frame, TextureDrawQuad::MaterialCast(quad));
+        drawTextureQuadPush(frame, TextureDrawQuad::MaterialCast(quad));
         break;
     case DrawQuad::TILED_CONTENT:
         drawTileQuad(frame, TileDrawQuad::MaterialCast(quad));
@@ -298,7 +353,7 @@ void GLRenderer::drawCheckerboardQuad(const DrawingFrame& frame, const Checkerbo
 {
     const TileCheckerboardProgram* program = tileCheckerboardProgram();
     DCHECK(program && (program->initialized() || isContextLost()));
-    GLC(context(), context()->useProgram(program->program()));
+    setUseProgram(program->program());
 
     SkColor color = quad->color;
     GLC(context(), context()->uniform4f(program->fragmentShader().colorLocation(), SkColorGetR(color) / 255.0, SkColorGetG(color) / 255.0, SkColorGetB(color) / 255.0, 1));
@@ -324,7 +379,7 @@ void GLRenderer::drawDebugBorderQuad(const DrawingFrame& frame, const DebugBorde
     static float glMatrix[16];
     const SolidColorProgram* program = solidColorProgram();
     DCHECK(program && (program->initialized() || isContextLost()));
-    GLC(context(), context()->useProgram(program->program()));
+    setUseProgram(program->program());
 
     // Use the full quadRect for debug quads to not move the edges based on partial swaps.
     const gfx::Rect& layerRect = quad->rect;
@@ -342,7 +397,7 @@ void GLRenderer::drawDebugBorderQuad(const DrawingFrame& frame, const DebugBorde
     GLC(context(), context()->lineWidth(quad->width));
 
     // The indices for the line are stored in the same array as the triangle indices.
-    GLC(context(), context()->drawElements(GL_LINE_LOOP, 4, GL_UNSIGNED_SHORT, 6 * sizeof(unsigned short)));
+    GLC(context(), context()->drawElements(GL_LINE_LOOP, 4, GL_UNSIGNED_SHORT, 0));
 }
 
 static WebGraphicsContext3D* getFilterContext(bool hasImplThread)
@@ -587,7 +642,7 @@ void GLRenderer::drawRenderPassQuad(DrawingFrame& frame, const RenderPassDrawQua
     int shaderAlphaLocation = -1;
     if (useAA && maskTextureId) {
         const RenderPassMaskProgramAA* program = renderPassMaskProgramAA();
-        GLC(context(), context()->useProgram(program->program()));
+        setUseProgram(program->program());
         GLC(context(), context()->uniform1i(program->fragmentShader().samplerLocation(), 0));
 
         shaderQuadLocation = program->vertexShader().pointLocation();
@@ -599,7 +654,7 @@ void GLRenderer::drawRenderPassQuad(DrawingFrame& frame, const RenderPassDrawQua
         shaderAlphaLocation = program->fragmentShader().alphaLocation();
     } else if (!useAA && maskTextureId) {
         const RenderPassMaskProgram* program = renderPassMaskProgram();
-        GLC(context(), context()->useProgram(program->program()));
+        setUseProgram(program->program());
         GLC(context(), context()->uniform1i(program->fragmentShader().samplerLocation(), 0));
 
         shaderMaskSamplerLocation = program->fragmentShader().maskSamplerLocation();
@@ -609,7 +664,7 @@ void GLRenderer::drawRenderPassQuad(DrawingFrame& frame, const RenderPassDrawQua
         shaderAlphaLocation = program->fragmentShader().alphaLocation();
     } else if (useAA && !maskTextureId) {
         const RenderPassProgramAA* program = renderPassProgramAA();
-        GLC(context(), context()->useProgram(program->program()));
+        setUseProgram(program->program());
         GLC(context(), context()->uniform1i(program->fragmentShader().samplerLocation(), 0));
 
         shaderQuadLocation = program->vertexShader().pointLocation();
@@ -618,7 +673,7 @@ void GLRenderer::drawRenderPassQuad(DrawingFrame& frame, const RenderPassDrawQua
         shaderAlphaLocation = program->fragmentShader().alphaLocation();
     } else {
         const RenderPassProgram* program = renderPassProgram();
-        GLC(context(), context()->useProgram(program->program()));
+        setUseProgram(program->program());
         GLC(context(), context()->uniform1i(program->fragmentShader().samplerLocation(), 0));
 
         shaderMatrixLocation = program->vertexShader().matrixLocation();
@@ -660,7 +715,7 @@ void GLRenderer::drawRenderPassQuad(DrawingFrame& frame, const RenderPassDrawQua
 void GLRenderer::drawSolidColorQuad(const DrawingFrame& frame, const SolidColorDrawQuad* quad)
 {
     const SolidColorProgram* program = solidColorProgram();
-    GLC(context(), context()->useProgram(program->program()));
+    setUseProgram(program->program());
 
     SkColor color = quad->color;
     float opacity = quad->opacity();
@@ -778,7 +833,7 @@ void GLRenderer::drawTileQuad(const DrawingFrame& frame, const TileDrawQuad* qua
         }
     }
 
-    GLC(context(), context()->useProgram(uniforms.program));
+    setUseProgram(uniforms.program);
     GLC(context(), context()->uniform1i(uniforms.samplerLocation, 0));
     ResourceProvider::ScopedReadLockGL quadResourceLock(m_resourceProvider, quad->resource_id);
     GLC(context(), context()->bindTexture(GL_TEXTURE_2D, quadResourceLock.textureId()));
@@ -895,7 +950,7 @@ void GLRenderer::drawYUVVideoQuad(const DrawingFrame& frame, const YUVVideoDrawQ
     GLC(context(), context()->activeTexture(GL_TEXTURE3));
     GLC(context(), context()->bindTexture(GL_TEXTURE_2D, vPlaneLock.textureId()));
 
-    GLC(context(), context()->useProgram(program->program()));
+    setUseProgram(program->program());
 
     GLC(context(), context()->uniform2f(program->vertexShader().texScaleLocation(), quad->tex_scale.width(), quad->tex_scale.height()));
     GLC(context(), context()->uniform1i(program->fragmentShader().yTextureLocation(), 1));
@@ -938,7 +993,7 @@ void GLRenderer::drawStreamVideoQuad(const DrawingFrame& frame, const StreamVide
     DCHECK(m_capabilities.usingEglImage);
 
     const VideoStreamTextureProgram* program = videoStreamTextureProgram();
-    GLC(context(), context()->useProgram(program->program()));
+    setUseProgram(program->program());
 
     toGLMatrix(&glMatrix[0], quad->matrix);
     GLC(context(), context()->uniformMatrix4fv(program->vertexShader().texMatrixLocation(), 1, false, glMatrix));
@@ -977,6 +1032,101 @@ struct TexTransformTextureProgramBinding : TextureProgramBinding {
     int texTransformLocation;
 };
 
+void GLRenderer::drawTextureQuadFlush()
+{
+    // Check to see if we have anything to draw
+    if (m_drawCache->programID == 0)
+      return;
+
+    // Set the correct blending mode
+    setBlendEnabled(m_drawCache->needsBlending);
+
+    // Bind the program to the GL state
+    setUseProgram(m_drawCache->programID);
+
+    // Bind the texture to slot 0
+    GLC(context(), context()->activeTexture(GL_TEXTURE0));
+    ResourceProvider::ScopedReadLockGL lockedQuad(m_resourceProvider, m_drawCache->resourceID);
+    GLC(context(), context()->bindTexture(GL_TEXTURE_2D, lockedQuad.textureId()));
+
+    // set up premultiplied alpha
+    if (!m_drawCache->usePremultipliedAlpha) {
+      // As it turns out, the premultiplied alpha blending function (ONE, ONE_MINUS_SRC_ALPHA)
+        // will never cause the alpha channel to be set to anything less than 1.0 if it is
+        // initialized to that value! Therefore, premultipliedAlpha being false is the first
+        // situation we can generally see an alpha channel less than 1.0 coming out of the
+        // compositor. This is causing platform differences in some layout tests (see
+        // https://bugs.webkit.org/show_bug.cgi?id=82412), so in this situation, use a separate
+        // blend function for the alpha channel to avoid modifying it. Don't use colorMask for this
+        // as it has performance implications on some platforms.
+        GLC(context(), context()->blendFuncSeparate(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA, GL_ZERO, GL_ONE));
+    }
+
+    // Set the shader opacity
+    setShaderOpacity(m_drawCache->alpha, m_drawCache->alphaLocation);
+
+    // Upload the tranforms for both points and uvs
+    GLC(m_context, m_context->uniformMatrix4fv((int)m_drawCache->matrixLocation, (int)m_drawCache->matrixData.size(), false, (float*)&m_drawCache->matrixData.front()));
+    GLC(m_context, m_context->uniform4fv((int)m_drawCache->uvXformLocation, (int)m_drawCache->uvXformData.size(), (float*)&m_drawCache->uvXformData.front()));
+
+    // Draw the quads!
+    GLC(m_context, m_context->drawElements(GL_TRIANGLES, 6 * m_drawCache->matrixData.size(), GL_UNSIGNED_SHORT, 0));
+
+    // Clean up after ourselves (reset state set above)
+     if (!m_drawCache->usePremultipliedAlpha)
+        GLC(m_context, m_context->blendFunc(GL_ONE, GL_ONE_MINUS_SRC_ALPHA));
+
+    // Clear the cache
+    m_drawCache->programID = 0;
+    m_drawCache->uvXformData.resize(0);
+    m_drawCache->matrixData.resize(0);
+}
+
+void GLRenderer::drawTextureQuadPush(const DrawingFrame& frame, const TextureDrawQuad* quad)

[shawnsingh, 2012/11/28 19:44:16]

The name at a glance doesn't make me think about the implications...  could we
rename it to something like  "enqueueTextureQuad"

[whunt, 2012/11/28 22:01:15]

I like that suggestion. Done.

On 2012/11/28 19:44:16, shawnsingh wrote:

+{
+    // Choose the correcte texture program binding
+    TexTransformTextureProgramBinding binding;
+    if (quad->flipped)
+        binding.set(textureProgramFlip(), context());
+    else
+        binding.set(textureProgram(), context());
+
+    int resourceID = quad->resource_id;
+
+    if (m_drawCache->programID             != binding.programId              ||
+        m_drawCache->resourceID            != resourceID                     ||
+        m_drawCache->alpha                 != quad->opacity()                ||
+        m_drawCache->usePremultipliedAlpha != quad->premultiplied_alpha      ||
+        m_drawCache->needsBlending         != quad->ShouldDrawWithBlending() ||
+        m_drawCache->matrixData.size() >= 8)
+    {
+        drawTextureQuadFlush();
+        m_drawCache->programID = binding.programId;
+        m_drawCache->resourceID = resourceID;
+        m_drawCache->alpha = quad->opacity();
+        m_drawCache->usePremultipliedAlpha = quad->premultiplied_alpha;
+        m_drawCache->needsBlending = quad->ShouldDrawWithBlending();
+
+        m_drawCache->alphaLocation = binding.alphaLocation;
+        m_drawCache->uvXformLocation = binding.texTransformLocation;
+        m_drawCache->matrixLocation = binding.matrixLocation;
+    }
+
+    // Generate the uv-transform
+    const gfx::RectF& uvRect = quad->uv_rect;
+    Vector uv(uvRect.x(), uvRect.y(), uvRect.width(), uvRect.height());
+    m_drawCache->uvXformData.push_back(uv);
+
+    // Generate the transform matrix
+    gfx::Transform quadRectMatrix;
+    quadRectTransform(&quadRectMatrix, quad->quadTransform(), quad->rect);
+    quadRectMatrix = frame.projectionMatrix * quadRectMatrix;
+
+    Matrix m;
+    quadRectMatrix.matrix().asColMajorf((float*)&m);
+    m_drawCache->matrixData.push_back(m);
+}
+
 void GLRenderer::drawTextureQuad(const DrawingFrame& frame, const TextureDrawQuad* quad)
 {
     TexTransformTextureProgramBinding binding;
@@ -984,7 +1134,7 @@ void GLRenderer::drawTextureQuad(const DrawingFrame& frame, const TextureDrawQua
         binding.set(textureProgramFlip(), context());
     else
         binding.set(textureProgram(), context());
-    GLC(context(), context()->useProgram(binding.programId));
+    setUseProgram(binding.programId);
     GLC(context(), context()->uniform1i(binding.samplerLocation, 0));
     const gfx::RectF& uvRect = quad->uv_rect;
     GLC(context(), context()->uniform4f(binding.texTransformLocation, uvRect.x(), uvRect.y(), uvRect.width(), uvRect.height()));
@@ -1016,7 +1166,7 @@ void GLRenderer::drawIOSurfaceQuad(const DrawingFrame& frame, const IOSurfaceDra
     TexTransformTextureProgramBinding binding;
     binding.set(textureIOSurfaceProgram(), context());
 
-    GLC(context(), context()->useProgram(binding.programId));
+    setUseProgram(binding.programId);
     GLC(context(), context()->uniform1i(binding.samplerLocation, 0));
     if (quad->orientation == IOSurfaceDrawQuad::FLIPPED)
         GLC(context(), context()->uniform4f(binding.texTransformLocation, 0, quad->io_surface_size.height(), quad->io_surface_size.width(), quad->io_surface_size.height() * -1.0));
@@ -1037,6 +1187,12 @@ void GLRenderer::finishDrawingFrame(DrawingFrame& frame)
     m_swapBufferRect.Union(gfx::ToEnclosingRect(frame.rootDamageRect));
 
     GLC(m_context, m_context->disable(GL_BLEND));
+    m_blendShadow = false;
+}
+
+void GLRenderer::finishDrawingQuadList()
+{
+    drawTextureQuadFlush();
 }
 
 bool GLRenderer::flippedFramebuffer() const
@@ -1049,6 +1205,7 @@ void GLRenderer::ensureScissorTestEnabled()
     if (m_isScissorEnabled)
         return;
 
+    drawTextureQuadFlush();
     GLC(m_context, m_context->enable(GL_SCISSOR_TEST));
     m_isScissorEnabled = true;
 }
@@ -1058,6 +1215,7 @@ void GLRenderer::ensureScissorTestDisabled()
     if (!m_isScissorEnabled)
         return;
 
+    drawTextureQuadFlush();
     GLC(m_context, m_context->disable(GL_SCISSOR_TEST));
     m_isScissorEnabled = false;
 }
@@ -1090,6 +1248,27 @@ void GLRenderer::setShaderOpacity(float opacity, int alphaLocation)
         GLC(m_context, m_context->uniform1f(alphaLocation, opacity));
 }
 
+void GLRenderer::setBlendEnabled(bool enabled)
+{
+    if (enabled != m_blendShadow)
+    {
+        if (enabled)
+            GLC(m_context, m_context->enable(GL_BLEND));
+        else
+            GLC(m_context, m_context->disable(GL_BLEND));
+        m_blendShadow = enabled;
+    }
+}
+
+void GLRenderer::setUseProgram(unsigned program)
+{
+    if (program != m_programShadow)
+    {
+        GLC(m_context, m_context->useProgram(program));
+        m_programShadow = program;
+    }
+}
+
 void GLRenderer::drawQuadGeometry(const DrawingFrame& frame, const gfx::Transform& drawTransform, const gfx::RectF& quadRect, int matrixLocation)
 {
     gfx::Transform quadRectMatrix;
@@ -1107,7 +1286,7 @@ void GLRenderer::copyTextureToFramebuffer(const DrawingFrame& frame, int texture
 
     GLC(context(), context()->bindTexture(GL_TEXTURE_2D, textureId));
 
-    GLC(context(), context()->useProgram(program->program()));
+    setUseProgram(program->program());
     GLC(context(), context()->uniform1i(program->fragmentShader().samplerLocation(), 0));
     setShaderOpacity(1, program->fragmentShader().alphaLocation());
     drawQuadGeometry(frame, drawMatrix, rect, program->vertexShader().matrixLocation());
@@ -1358,6 +1537,7 @@ void GLRenderer::setScissorTestRect(const gfx::Rect& scissorRect)
         return;
 
     m_scissorRect = scissorRect;
+    drawTextureQuadFlush();
     GLC(m_context, m_context->scissor(scissorRect.x(), scissorRect.y(), scissorRect.width(), scissorRect.height()));
 }
 
@@ -1527,6 +1707,7 @@ const GLRenderer::TextureProgram* GLRenderer::textureProgram()
     if (!m_textureProgram->initialized()) {
         TRACE_EVENT0("cc", "GLRenderer::textureProgram::initialize");
         m_textureProgram->initialize(m_context, m_isUsingBindUniform);
+        GLC(context(), context()->uniform1i(m_textureProgram.get()->fragmentShader().samplerLocation(), 0));
     }
     return m_textureProgram.get();
 }
@@ -1538,6 +1719,7 @@ const GLRenderer::TextureProgramFlip* GLRenderer::textureProgramFlip()
     if (!m_textureProgramFlip->initialized()) {
         TRACE_EVENT0("cc", "GLRenderer::textureProgramFlip::initialize");
         m_textureProgramFlip->initialize(m_context, m_isUsingBindUniform);
+        GLC(context(), context()->uniform1i(m_textureProgramFlip.get()->fragmentShader().samplerLocation(), 0));
     }
     return m_textureProgramFlip.get();
 }