WebGL backbuffer creation on Mali-400 GPU

Source/platform/graphics/gpu/DrawingBuffer.cpp

Index: Source/platform/graphics/gpu/DrawingBuffer.cpp
diff --git a/Source/platform/graphics/gpu/DrawingBuffer.cpp b/Source/platform/graphics/gpu/DrawingBuffer.cpp
index aa528a3b761c82c8ed8dd9b4e8eaa86145009398..73fe9ee0feaf9f0c15b55bf5aef078177b982527 100644
--- a/Source/platform/graphics/gpu/DrawingBuffer.cpp
+++ b/Source/platform/graphics/gpu/DrawingBuffer.cpp
@@ -105,6 +105,7 @@ PassRefPtr<DrawingBuffer> DrawingBuffer::create(PassOwnPtr<WebGraphicsContext3D>
         extensionsUtil->ensureExtensionEnabled("GL_EXT_discard_framebuffer");
 
     RefPtr<DrawingBuffer> drawingBuffer = adoptRef(new DrawingBuffer(context, extensionsUtil.release(), multisampleSupported, packedDepthStencilSupported, discardFramebufferSupported, preserve, requestedAttributes));
+    drawingBuffer->validateBackbuffer();
     if (!drawingBuffer->initialize(size)) {
         drawingBuffer->beginDestruction();
         return PassRefPtr<DrawingBuffer>();
@@ -403,7 +404,9 @@ bool DrawingBuffer::initialize(const IntSize& size)
         return false;
     }
 
-    if (m_requestedAttributes.alpha) {
+    // Type of color format should be selected based on
+    // alpha attribute & texture format supported on GPU's
+    if (m_requestedAttributes.alpha || !isRGBTextureSupported()) {
         m_internalColorFormat = GL_RGBA;
         m_colorFormat = GL_RGBA;
         m_internalRenderbufferFormat = GL_RGBA8_OES;
@@ -763,8 +766,18 @@ bool DrawingBuffer::reset(const IntSize& newSize)
     }
 
     m_context->disable(GL_SCISSOR_TEST);
-    m_context->clearColor(0, 0, 0, 0);
-    m_context->colorMask(true, true, true, true);
+
+    if (!m_requestedAttributes.alpha && !isRGBTextureSupported()) {
+        // This case signifies that requested alpha attribute is false, even though
+        // DrawingBuffer choosen backbuffer as GL_RGBA format (due to GPU limitations).
+        // 1. So ensure alpha channel should be cleared to 1.0
+        // 2. colormask should be disabled (Turn off rendering to alpha)
+        m_context->clearColor(0, 0, 0, 1);
+        m_context->colorMask(true, true, true, false);

[Ken Russell (switch to Gerrit), 2015/04/06 21:43:18]

This won't work -- the color mask applies to clear operations too, so this code
will disable the needed clear of the alpha channel to 0xff. How much testing was
done on this patch? There should be WebGL conformance tests which would have
caught this bug. If not, we need to add them.

+    } else {
+        m_context->clearColor(0, 0, 0, 0);
+        m_context->colorMask(true, true, true, true);
+    }
 
     GLbitfield clearMask = GL_COLOR_BUFFER_BIT;
     if (m_actualAttributes.depth) {
@@ -987,4 +1000,24 @@ void DrawingBuffer::deleteChromiumImageForTexture(TextureInfo* info)
     }
 }
 
+void DrawingBuffer::validateBackbuffer()
+{
+    // One time validation is required for GL_RGB texture

[Ken Russell (switch to Gerrit), 2015/04/06 21:43:18]

I'm sorry, but I'm not comfortable doing this code unconditionally. First,
running this code will slow down WebGL context creation on all platforms.
Second, it's clear there are bugs in this code, and it's essential that this
workaround be done exactly on the platforms where it's intended and no others.

Please:

 - Introduce a new type like WebGpuBlacklist in WebKit/public/platform with a
getter in WebKit/public/platform/Platform.h. For the time being, this can have
just one virtual function named similarly to your new GPU driver bug workaround
in https://codereview.chromium.org/996163003/ , such as "disableGLRGBFormat"
 - Implement the getter and this type in
src/content/renderer/renderer_blink_platform_impl.{cc,h} and possibly in
src/gpu/blink/ (not sure where the best place for this code is -- look for
similarly structured classes). Base the result on the actual GPU driver bug
workarounds in use.
 - Perform the RGB -> RGBA back buffer emulation only when the driver bug
workaround is enabled.

This will require more CLs to be landed. First introduce the new type
(WebGpuBlacklist) on the Blink side. Then plumb it through to the Chromium side.
Finally, use the new type on the Blink side to implement the workaround.

[Ken Russell (switch to Gerrit), 2015/04/06 21:44:54]

Also -- on request from another engineer on Blink, please add a UseCounter (see
core/frame/UseCounter.h ) around the workaround code. We'll want to be able to
remove this workaround at some point in the future based on usage data.

+    // so that DrawingBuffer can choose format for the backbuffer correctly.
+    // DrawingBuffer tries to allocate GL_RGB texture in case of alpha attribute false
+    // and fails because GL_RGB not supported on specific GPU's (e.g Mali- 2/3/400 family).
+    Platform3DObject fbo = m_context->createFramebuffer();
+    m_context->bindFramebuffer(GL_FRAMEBUFFER, fbo);
+    unsigned offscreenRGBTexture = createColorTexture();
+    m_context->texImage2D(GL_TEXTURE_2D, 0, GL_RGB, 1, 1, 0, GL_RGB, GL_UNSIGNED_BYTE, 0);
+    m_context->framebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, offscreenRGBTexture, 0);
+    m_RGBTextureSupported = m_context->checkFramebufferStatus(GL_FRAMEBUFFER) == GL_FRAMEBUFFER_COMPLETE;
+
+    if (fbo)

[Ken Russell (switch to Gerrit), 2015/04/06 21:43:18]

Also m_context_>bindFramebuffer(GL_FRAMEBUFFER, 0); .

+        m_context->deleteFramebuffer(fbo);
+
+    if (offscreenRGBTexture)
+        m_context->deleteTexture(offscreenRGBTexture);
+}
+
 } // namespace blink