cc: Use the correct internal format for glCopyTexImage2D calls.

This uses the correct format for glCopyTexImage2D calls from the
default framebuffer (for the root renderpass). This function is used
for the implementation of backdrop filters.

Currently it uses GL_RGBA, but it is not valid to add a channel, so
if the framebuffer is only RGB, then we must use GL_RGB or it will fail.

This is demonstrated by the backdrop filter layout tests once they
use a cc::Display for doing GL drawing, instead of the current method
with a MailboxOutputSurface, which happens to provide an RGBA
framebuffer.

The layout tests are:
css3/filters/backdrop-filter-rendering.html
css3/filters/backdrop-filter-rendering-no-background.html

This also changes the ContextProvider used by cc pixel tests
to drop the alpha. With that, a number of pixel tests require
new baselines, as the background becomes black instead of
transparent, and some tests use background filters or leave some
pixels undrawn.

With the change to the pixel tests, but without the change in
GLRenderer, many cc pixel tests fail showing black for any
background filter, as exhibited by bug 612238, along with the
error "GL ERROR :GL_INVALID_OPERATION : glCopyTexImage2D:
incompatible format".

R=fsamuel@chromium.org, piman@chromium.org, sievers
BUG=311404, 612238
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2089753003
Cr-Commit-Position: refs/heads/master@{#401438}

android_webview/browser/aw_render_thread_context_provider.cc

@@ -75,6 +75,11 @@ AwRenderThreadContextProvider::~AwRenderThreadContextProvider() {
     gr_context_->releaseResourcesAndAbandonContext();
 }
 
+uint32_t AwRenderThreadContextProvider::GetCopyTextureInternalFormat() {
+  // The attributes used in the constructor included an alpha channel.
+  return GL_RGBA;
+}
+
 bool AwRenderThreadContextProvider::BindToCurrentThread() {
   // This is called on the thread the context will be used.
   DCHECK(main_thread_checker_.CalledOnValidThread());

android_webview/browser/aw_render_thread_context_provider.h

@@ -32,6 +32,10 @@ class AwRenderThreadContextProvider : public cc::ContextProvider {
       scoped_refptr<gl::GLSurface> surface,
       scoped_refptr<gpu::InProcessCommandBuffer::Service> service);
 
+  // Gives the GL internal format that should be used for calling CopyTexImage2D
+  // on the default framebuffer.
+  uint32_t GetCopyTextureInternalFormat();
+
  private:
   AwRenderThreadContextProvider(
       scoped_refptr<gl::GLSurface> surface,

android_webview/browser/parent_output_surface.cc

@@ -4,13 +4,14 @@
 
 #include "android_webview/browser/parent_output_surface.h"
 
+#include "android_webview/browser/aw_render_thread_context_provider.h"
 #include "cc/output/output_surface_client.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 
 namespace android_webview {
 
 ParentOutputSurface::ParentOutputSurface(
-    scoped_refptr<cc::ContextProvider> context_provider)
+    scoped_refptr<AwRenderThreadContextProvider> context_provider)
     : cc::OutputSurface(std::move(context_provider), nullptr, nullptr) {
   stencil_state_.stencil_test_enabled = false;
 }
@@ -54,6 +55,11 @@ void ParentOutputSurface::ApplyExternalStencil() {
                         stencil_state_.stencil_back_z_pass_op);
 }
 
+uint32_t ParentOutputSurface::GetFramebufferCopyTextureFormat() {
+  auto* gl = static_cast<AwRenderThreadContextProvider*>(context_provider());
+  return gl->GetCopyTextureInternalFormat();
+}
+
 void ParentOutputSurface::SetGLState(const ScopedAppGLStateRestore& gl_state) {
   stencil_state_ = gl_state.stencil_state();
   SetExternalStencilTest(stencil_state_.stencil_test_enabled);

android_webview/browser/parent_output_surface.h

@@ -10,11 +10,12 @@
 #include "cc/output/output_surface.h"
 
 namespace android_webview {
+class AwRenderThreadContextProvider;
 
 class ParentOutputSurface : NON_EXPORTED_BASE(public cc::OutputSurface) {
  public:
   explicit ParentOutputSurface(
-      scoped_refptr<cc::ContextProvider> context_provider);
+      scoped_refptr<AwRenderThreadContextProvider> context_provider);
   ~ParentOutputSurface() override;
 
   // OutputSurface overrides.
@@ -24,6 +25,7 @@ class ParentOutputSurface : NON_EXPORTED_BASE(public cc::OutputSurface) {
                bool has_alpha) override;
   void SwapBuffers(cc::CompositorFrame* frame) override;
   void ApplyExternalStencil() override;
+  uint32_t GetFramebufferCopyTextureFormat() override;
 
   void SetGLState(const ScopedAppGLStateRestore& gl_state);
 

blimp/client/feature/compositor/blimp_context_provider.cc

@@ -124,5 +124,11 @@ void BlimpContextProvider::OnLostContext() {
     gr_context_->OnLostContext();
 }
 
+uint32_t BlimpContextProvider::GetCopyTextureInternalFormat() {
+  // The attributes used to create the context in the constructor specify
+  // an alpha channel.
+  return GL_RGBA;
+}
+
 }  // namespace client
 }  // namespace blimp

blimp/client/feature/compositor/blimp_context_provider.h

@@ -43,6 +43,10 @@ class BlimpContextProvider : public cc::ContextProvider {
   void SetLostContextCallback(
       const LostContextCallback& lost_context_callback) override;
 
+  // Gives the GL internal format that should be used for calling CopyTexImage2D
+  // on the default framebuffer.
+  uint32_t GetCopyTextureInternalFormat();
+
  protected:
   BlimpContextProvider(
       gfx::AcceleratedWidget widget,

blimp/client/feature/compositor/blimp_output_surface.cc

@@ -6,14 +6,15 @@
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "blimp/client/feature/compositor/blimp_context_provider.h"
 #include "cc/output/output_surface_client.h"
 #include "gpu/command_buffer/client/context_support.h"
 
 namespace blimp {
 namespace client {
 
 BlimpOutputSurface::BlimpOutputSurface(
-    scoped_refptr<cc::ContextProvider> context_provider)
+    scoped_refptr<BlimpContextProvider> context_provider)
     : cc::OutputSurface(std::move(context_provider), nullptr, nullptr) {}
 
 BlimpOutputSurface::~BlimpOutputSurface() {}
@@ -25,5 +26,10 @@ void BlimpOutputSurface::SwapBuffers(cc::CompositorFrame* frame) {
   cc::OutputSurface::PostSwapBuffersComplete();
 }
 
+uint32_t BlimpOutputSurface::GetFramebufferCopyTextureFormat() {
+  auto* gl = static_cast<BlimpContextProvider*>(context_provider());
+  return gl->GetCopyTextureInternalFormat();
+}
+
 }  // namespace client
 }  // namespace blimp

blimp/client/feature/compositor/blimp_output_surface.h

@@ -13,16 +13,18 @@
 
 namespace blimp {
 namespace client {
+class BlimpContextProvider;
 
 // Minimal implementation of cc::OutputSurface.
 class BlimpOutputSurface : public cc::OutputSurface {
  public:
   explicit BlimpOutputSurface(
-      scoped_refptr<cc::ContextProvider> context_provider);
+      scoped_refptr<BlimpContextProvider> context_provider);
   ~BlimpOutputSurface() override;
 
   // OutputSurface implementation.
   void SwapBuffers(cc::CompositorFrame* frame) override;
+  uint32_t GetFramebufferCopyTextureFormat() override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BlimpOutputSurface);

cc/output/gl_renderer.cc

@@ -833,7 +833,7 @@ std::unique_ptr<ScopedResource> GLRenderer::GetBackdropTexture(
   {
     ResourceProvider::ScopedWriteLockGL lock(resource_provider_,
                                              device_background_texture->id());
-    GetFramebufferTexture(lock.texture_id(), RGBA_8888, bounding_rect);
+    GetFramebufferTexture(lock.texture_id(), bounding_rect);
   }
   return device_background_texture;
 }
@@ -2869,7 +2869,7 @@ void GLRenderer::GetFramebufferPixelsAsync(
       texture_id =
           gl_->CreateAndConsumeTextureCHROMIUM(GL_TEXTURE_2D, mailbox.name);
     }
-    GetFramebufferTexture(texture_id, RGBA_8888, window_rect);
+    GetFramebufferTexture(texture_id, window_rect);
 
     const GLuint64 fence_sync = gl_->InsertFenceSyncCHROMIUM();
     gl_->ShallowFlushCHROMIUM();
@@ -3009,17 +3009,26 @@ void GLRenderer::FinishedReadback(unsigned source_buffer,
 }
 
 void GLRenderer::GetFramebufferTexture(unsigned texture_id,
-                                       ResourceFormat texture_format,
                                        const gfx::Rect& window_rect) {
   DCHECK(texture_id);
   DCHECK_GE(window_rect.x(), 0);
   DCHECK_GE(window_rect.y(), 0);
   DCHECK_LE(window_rect.right(), current_surface_size_.width());
   DCHECK_LE(window_rect.bottom(), current_surface_size_.height());
 
+  // If copying a non-root renderpass then use the format of the bound
+  // texture. Otherwise, we use the format of the default framebuffer.
+  GLenum format = current_framebuffer_lock_
+                      ? GLCopyTextureInternalFormat(current_framebuffer_format_)
+                      : output_surface_->GetFramebufferCopyTextureFormat();
+  // Verify the format is valid for GLES2's glCopyTexImage2D.
+  DCHECK(format == GL_ALPHA || format == GL_LUMINANCE ||
+         format == GL_LUMINANCE_ALPHA || format == GL_RGB || format == GL_RGBA)
+      << format;
+
   gl_->BindTexture(GL_TEXTURE_2D, texture_id);
-  gl_->CopyTexImage2D(GL_TEXTURE_2D, 0, GLDataFormat(texture_format),
-                      window_rect.x(), window_rect.y(), window_rect.width(),
+  gl_->CopyTexImage2D(GL_TEXTURE_2D, 0, format, window_rect.x(),
+                      window_rect.y(), window_rect.width(),
                       window_rect.height(), 0);
   gl_->BindTexture(GL_TEXTURE_2D, 0);
 }
@@ -3049,6 +3058,7 @@ bool GLRenderer::BindFramebufferToTexture(DrawingFrame* frame,
   current_framebuffer_lock_ =
       base::WrapUnique(new ResourceProvider::ScopedWriteLockGL(
           resource_provider_, texture->id()));
+  current_framebuffer_format_ = texture->format();
   unsigned texture_id = current_framebuffer_lock_->texture_id();
   gl_->FramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
                             texture_id, 0);

cc/output/gl_renderer.h

@@ -92,7 +92,6 @@ class CC_EXPORT GLRenderer : public DirectRenderer {
                                  const gfx::Rect& rect,
                                  std::unique_ptr<CopyOutputRequest> request);
   void GetFramebufferTexture(unsigned texture_id,
-                             ResourceFormat texture_format,
                              const gfx::Rect& device_rect);
   void ReleaseRenderPassTextures();
   enum BoundGeometry { NO_BINDING, SHARED_BINDING, CLIPPED_BINDING };
@@ -511,6 +510,8 @@ class CC_EXPORT GLRenderer : public DirectRenderer {
 
   std::unique_ptr<ResourceProvider::ScopedWriteLockGL>
       current_framebuffer_lock_;
+  // This is valid when current_framebuffer_lock_ is not null.
+  ResourceFormat current_framebuffer_format_;
 
   class SyncQuery;
   std::deque<std::unique_ptr<SyncQuery>> pending_sync_queries_;

cc/output/gl_renderer_unittest.cc

@@ -1392,7 +1392,7 @@ TEST_F(GLRendererTest, DrawFramePreservesFramebuffer) {
   gpu::gles2::GLES2Interface* gl =
       output_surface->context_provider()->ContextGL();
   gl->GenFramebuffers(1, &fbo);
-  output_surface->set_framebuffer(fbo);
+  output_surface->set_framebuffer(fbo, GL_RGB);
 
   renderer.DecideRenderPassAllocationsForFrame(render_passes_in_draw_order_);
   renderer.DrawFrame(&render_passes_in_draw_order_, 1.f, device_viewport_rect,
@@ -1779,6 +1779,7 @@ class MockOutputSurface : public OutputSurface {
   MOCK_METHOD3(Reshape,
                void(const gfx::Size& size, float scale_factor, bool has_alpha));
   MOCK_METHOD0(BindFramebuffer, void());
+  MOCK_METHOD0(GetFramebufferCopyTextureFormat, GLenum());
   MOCK_METHOD1(SwapBuffers, void(CompositorFrame* frame));
 };
 

cc/output/output_surface.h

@@ -132,6 +132,9 @@ class CC_EXPORT OutputSurface : public base::trace_event::MemoryDumpProvider {
   virtual void ForceReclaimResources() {}
 
   virtual void BindFramebuffer();
+  // Gives the GL internal format that should be used for calling CopyTexImage2D
+  // when the framebuffer is bound via BindFramebuffer().
+  virtual uint32_t GetFramebufferCopyTextureFormat() = 0;
 
   // The implementation may destroy or steal the contents of the CompositorFrame
   // passed in (though it will not take ownership of the CompositorFrame

cc/output/output_surface_unittest.cc

@@ -22,11 +22,12 @@ namespace {
 
 class TestOutputSurface : public OutputSurface {
  public:
-  explicit TestOutputSurface(scoped_refptr<ContextProvider> context_provider)
+  explicit TestOutputSurface(
+      scoped_refptr<TestContextProvider> context_provider)
       : OutputSurface(std::move(context_provider), nullptr, nullptr) {}
 
-  TestOutputSurface(scoped_refptr<ContextProvider> context_provider,
-                    scoped_refptr<ContextProvider> worker_context_provider)
+  TestOutputSurface(scoped_refptr<TestContextProvider> context_provider,
+                    scoped_refptr<TestContextProvider> worker_context_provider)
       : OutputSurface(std::move(context_provider),
                       std::move(worker_context_provider),
                       nullptr) {}
@@ -35,7 +36,7 @@ class TestOutputSurface : public OutputSurface {
       std::unique_ptr<SoftwareOutputDevice> software_device)
       : OutputSurface(nullptr, nullptr, std::move(software_device)) {}
 
-  TestOutputSurface(scoped_refptr<ContextProvider> context_provider,
+  TestOutputSurface(scoped_refptr<TestContextProvider> context_provider,
                     std::unique_ptr<SoftwareOutputDevice> software_device)
       : OutputSurface(std::move(context_provider),
                       nullptr,
@@ -45,6 +46,10 @@ class TestOutputSurface : public OutputSurface {
     client_->DidSwapBuffers();
     client_->DidSwapBuffersComplete();
   }
+  uint32_t GetFramebufferCopyTextureFormat() override {
+    // TestContextProvider has no real framebuffer, just use RGB.
+    return GL_RGB;
+  }
 
   void DidSwapBuffersForTesting() { client_->DidSwapBuffers(); }
 

cc/output/overlay_unittest.cc

@@ -133,8 +133,9 @@ size_t DefaultOverlayProcessor::GetStrategyCount() {
 
 class OverlayOutputSurface : public OutputSurface {
  public:
-  explicit OverlayOutputSurface(scoped_refptr<ContextProvider> context_provider)
-      : OutputSurface(context_provider, nullptr, nullptr) {
+  explicit OverlayOutputSurface(
+      scoped_refptr<TestContextProvider> context_provider)
+      : OutputSurface(std::move(context_provider), nullptr, nullptr) {
     surface_size_ = kDisplaySize;
     device_scale_factor_ = 1;
     is_displayed_as_overlay_plane_ = true;
@@ -149,6 +150,10 @@ class OverlayOutputSurface : public OutputSurface {
     OutputSurface::BindFramebuffer();
     bind_framebuffer_count_ += 1;
   }
+  uint32_t GetFramebufferCopyTextureFormat() override {
+    // TestContextProvider has no real framebuffer, just use RGB.
+    return GL_RGB;
+  }
   void SwapBuffers(CompositorFrame* frame) override {
     client_->DidSwapBuffers();
   }

cc/output/renderer_unittest.cc

@@ -21,8 +21,9 @@ class TestOutputSurface : public OutputSurface {
   explicit TestOutputSurface(scoped_refptr<ContextProvider> context_provider);
   ~TestOutputSurface() override;
 
-  // OutputSurface implementation
+  // OutputSurface implementation.
   void SwapBuffers(CompositorFrame* frame) override;
+  uint32_t GetFramebufferCopyTextureFormat() override { return GL_RGB; }
 };
 
 TestOutputSurface::TestOutputSurface(

cc/resources/resource_format.cc

@@ -63,14 +63,39 @@ GLenum GLDataFormat(ResourceFormat format) {
   };
   static_assert(arraysize(format_gl_data_format) == (RESOURCE_FORMAT_MAX + 1),
                 "format_gl_data_format does not handle all cases.");
-
   return format_gl_data_format[format];
 }
 
 GLenum GLInternalFormat(ResourceFormat format) {
+  // In GLES2, the internal format must match the texture format. (It no longer
+  // is true in GLES3, however it still holds for the BGRA extension.)
   return GLDataFormat(format);
 }
 
+GLenum GLCopyTextureInternalFormat(ResourceFormat format) {
+  // In GLES2, valid formats for glCopyTexImage2D are: GL_ALPHA, GL_LUMINANCE,
+  // GL_LUMINANCE_ALPHA, GL_RGB, or GL_RGBA.
+  // Extensions typically used for glTexImage2D do not also work for
+  // glCopyTexImage2D. For instance GL_BGRA_EXT may not be used for
+  // anything but gl(Sub)TexImage2D:
+  // https://www.khronos.org/registry/gles/extensions/EXT/EXT_texture_format_BGRA8888.txt
+  DCHECK_LE(format, RESOURCE_FORMAT_MAX);
+  static const GLenum format_gl_data_format[] = {
+      GL_RGBA,       // RGBA_8888
+      GL_RGBA,       // RGBA_4444
+      GL_RGBA,       // BGRA_8888
+      GL_ALPHA,      // ALPHA_8
+      GL_LUMINANCE,  // LUMINANCE_8
+      GL_RGB,        // RGB_565
+      GL_RGB,        // ETC1
+      GL_LUMINANCE,  // RED_8
+      GL_LUMINANCE,  // LUMINANCE_F16
+  };
+  static_assert(arraysize(format_gl_data_format) == (RESOURCE_FORMAT_MAX + 1),
+                "format_gl_data_format does not handle all cases.");
+  return format_gl_data_format[format];
+}
+
 gfx::BufferFormat BufferFormat(ResourceFormat format) {
   switch (format) {
     case BGRA_8888:

cc/resources/resource_format.h

@@ -34,6 +34,7 @@ CC_EXPORT int BitsPerPixel(ResourceFormat format);
 CC_EXPORT GLenum GLDataType(ResourceFormat format);
 CC_EXPORT GLenum GLDataFormat(ResourceFormat format);
 CC_EXPORT GLenum GLInternalFormat(ResourceFormat format);
+CC_EXPORT GLenum GLCopyTextureInternalFormat(ResourceFormat format);
 CC_EXPORT gfx::BufferFormat BufferFormat(ResourceFormat format);
 
 bool IsResourceFormatCompressed(ResourceFormat format);

cc/surfaces/surface_display_output_surface.cc

@@ -124,6 +124,17 @@ void SurfaceDisplayOutputSurface::DetachFromClient() {
   OutputSurface::DetachFromClient();
 }
 
+void SurfaceDisplayOutputSurface::BindFramebuffer() {
+  // This is a delegating output surface, no framebuffer/direct drawing support.
+  NOTREACHED();
+}
+
+uint32_t SurfaceDisplayOutputSurface::GetFramebufferCopyTextureFormat() {
+  // This is a delegating output surface, no framebuffer/direct drawing support.
+  NOTREACHED();
+  return 0;
+}
+
 void SurfaceDisplayOutputSurface::ReturnResources(
     const ReturnedResourceArray& resources) {
   CompositorFrameAck ack;

cc/surfaces/surface_display_output_surface.h

@@ -45,6 +45,8 @@ class CC_SURFACES_EXPORT SurfaceDisplayOutputSurface
   bool BindToClient(OutputSurfaceClient* client) override;
   void ForceReclaimResources() override;
   void DetachFromClient() override;
+  void BindFramebuffer() override;
+  uint32_t GetFramebufferCopyTextureFormat() override;
 
   // SurfaceFactoryClient implementation.
   void ReturnResources(const ReturnedResourceArray& resources) override;