WebGL backbuffer creation on Mali-400 GPU

Source/platform/graphics/gpu/DrawingBuffer.cpp

 /*
  * Copyright (c) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 87 matching lines @@
             extensionsUtil->ensureExtensionEnabled("GL_EXT_multisampled_render_to_texture");
     }
     bool packedDepthStencilSupported = extensionsUtil->supportsExtension("GL_OES_packed_depth_stencil");
     if (packedDepthStencilSupported)
         extensionsUtil->ensureExtensionEnabled("GL_OES_packed_depth_stencil");
     bool discardFramebufferSupported = extensionsUtil->supportsExtension("GL_EXT_discard_framebuffer");
     if (discardFramebufferSupported)
         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>();
     }
     return drawingBuffer.release();
 }
 
 DrawingBuffer::DrawingBuffer(PassOwnPtr<WebGraphicsContext3D> context,
     PassOwnPtr<Extensions3DUtil> extensionsUtil,
     bool multisampleExtensionSupported,
@@ skipping 278 matching lines @@
     ASSERT_NOT_REACHED();
 }
 
 bool DrawingBuffer::initialize(const IntSize& size)
 {
     if (m_context->isContextLost()) {
         // Need to try to restore the context again later.
         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;
     } else {
         m_internalColorFormat = GL_RGB;
         m_colorFormat = GL_RGB;
         m_internalRenderbufferFormat = GL_RGB8_OES;
     }
 
     m_context->getIntegerv(GL_MAX_TEXTURE_SIZE, &m_maxTextureSize);
@@ skipping 339 matching lines @@
             break;
         } while (!adjustedSize.isEmpty());
 
         setSize(adjustedSize);
 
         if (adjustedSize.isEmpty())
             return false;
     }
 
     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) {
         m_context->clearDepth(1.0f);
         clearMask |= GL_DEPTH_BUFFER_BIT;
         m_context->depthMask(true);
     }
     if (m_actualAttributes.stencil) {
         m_context->clearStencil(0);
         clearMask |= GL_STENCIL_BUFFER_BIT;
@@ skipping 202 matching lines @@
 
 void DrawingBuffer::deleteChromiumImageForTexture(TextureInfo* info)
 {
     if (info->imageId) {
         m_context->releaseTexImage2DCHROMIUM(GL_TEXTURE_2D, info->imageId);
         m_context->destroyImageCHROMIUM(info->imageId);
         info->imageId = 0;
     }
 }
 
+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