From 4d8dc295cccc146320088799dd9af2f90d71232d Mon Sep 17 00:00:00 2001 From: erikchen Date: Thu, 24 Mar 2016 21:37:08 -0700 Subject: Clean up calls to CreateGpuMemoryBufferImageCHROMIUM. This CL should not induce any behavior changes. IOSurfaces expect a format of GL_BGRA, and an internal format of GL_RGBA. Callers of CreateGpuMemoryBufferImageCHROMIUM were incorrectly passing in GL_BGRA as |internalformat|, and the implementation of CreateGpuMemoryBufferImageCHROMIUM was incorrectly allowing this. This CL updates callsites to pass in GL_RGBA, and updates the implementation of CreateGpuMemoryBufferImageCHROMIUM to not accept GL_BGRA. BUG=595948 CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel Review URL: https://codereview.chromium.org/1832923002 Cr-Commit-Position: refs/heads/master@{#383243} --- .../compositor/gpu_process_transport_factory.cc | 2 +- gpu/command_buffer/client/gles2_implementation.cc | 18 ++++++++--- gpu/command_buffer/service/image_factory.cc | 35 +++++++++++++++------- .../gl_copy_tex_image_2d_workaround_unittest.cc | 2 +- .../gl_iosurface_readback_workaround_unittest.cc | 2 +- .../platform/graphics/Canvas2DLayerBridge.cpp | 2 +- 6 files changed, 43 insertions(+), 18 deletions(-) diff --git a/content/browser/compositor/gpu_process_transport_factory.cc b/content/browser/compositor/gpu_process_transport_factory.cc index 01dcecd..f0bdc4d4 100644 --- a/content/browser/compositor/gpu_process_transport_factory.cc +++ b/content/browser/compositor/gpu_process_transport_factory.cc @@ -342,7 +342,7 @@ void GpuProcessTransportFactory::EstablishedGpuChannel( GLenum format = GL_RGB; #if defined(OS_MACOSX) target = GL_TEXTURE_RECTANGLE_ARB; - format = GL_BGRA_EXT; + format = GL_RGBA; #endif surface = make_scoped_ptr(new GpuSurfacelessBrowserCompositorOutputSurface( diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc index 8cb9716..96896b8 100644 --- a/gpu/command_buffer/client/gles2_implementation.cc +++ b/gpu/command_buffer/client/gles2_implementation.cc @@ -5923,8 +5923,8 @@ void GLES2Implementation::WaitSyncTokenCHROMIUM(const GLbyte* sync_token) { namespace { -bool ValidImageFormat(GLenum internalformat, - const Capabilities& capabilities) { +bool CreateImageValidInternalFormat(GLenum internalformat, + const Capabilities& capabilities) { switch (internalformat) { case GL_ATC_RGB_AMD: case GL_ATC_RGBA_INTERPOLATED_ALPHA_AMD: @@ -5947,6 +5947,16 @@ bool ValidImageFormat(GLenum internalformat, } } +bool CreateGpuMemoryBufferValidInternalFormat(GLenum internalformat) { + switch (internalformat) { + case GL_RGB: + case GL_RGBA: + return true; + default: + return false; + } +} + bool ValidImageUsage(GLenum usage) { return usage == GL_READ_WRITE_CHROMIUM; } @@ -5967,7 +5977,7 @@ GLuint GLES2Implementation::CreateImageCHROMIUMHelper(ClientBuffer buffer, return 0; } - if (!ValidImageFormat(internalformat, capabilities_)) { + if (!CreateImageValidInternalFormat(internalformat, capabilities_)) { SetGLError(GL_INVALID_VALUE, "glCreateImageCHROMIUM", "invalid format"); return 0; } @@ -6033,7 +6043,7 @@ GLuint GLES2Implementation::CreateGpuMemoryBufferImageCHROMIUMHelper( return 0; } - if (!ValidImageFormat(internalformat, capabilities_)) { + if (!CreateGpuMemoryBufferValidInternalFormat(internalformat)) { SetGLError(GL_INVALID_VALUE, "glCreateGpuMemoryBufferImageCHROMIUM", "invalid format"); diff --git a/gpu/command_buffer/service/image_factory.cc b/gpu/command_buffer/service/image_factory.cc index a96da9a..fd06d3f 100644 --- a/gpu/command_buffer/service/image_factory.cc +++ b/gpu/command_buffer/service/image_factory.cc @@ -9,15 +9,8 @@ namespace gpu { -ImageFactory::ImageFactory() { -} - -ImageFactory::~ImageFactory() { -} - -// static -gfx::BufferFormat ImageFactory::DefaultBufferFormatForImageFormat( - unsigned internalformat) { +namespace { +gfx::BufferFormat BufferFormatForInternalFormat(unsigned internalformat) { switch (internalformat) { case GL_RED: return gfx::BufferFormat::R_8; @@ -49,6 +42,28 @@ gfx::BufferFormat ImageFactory::DefaultBufferFormatForImageFormat( } } +} // namespace + +ImageFactory::ImageFactory() { +} + +ImageFactory::~ImageFactory() { +} + +// static +gfx::BufferFormat ImageFactory::DefaultBufferFormatForImageFormat( + unsigned internalformat) { + switch (internalformat) { + case GL_RGB: + return gfx::BufferFormat::BGRX_8888; + case GL_RGBA: + return gfx::BufferFormat::RGBA_8888; + default: + NOTREACHED(); + return gfx::BufferFormat::RGBA_8888; + } +} + // static bool ImageFactory::IsImageFormatCompatibleWithGpuMemoryBufferFormat( unsigned internalformat, @@ -67,7 +82,7 @@ bool ImageFactory::IsImageFormatCompatibleWithGpuMemoryBufferFormat( case gfx::BufferFormat::YUV_420: case gfx::BufferFormat::YUV_420_BIPLANAR: case gfx::BufferFormat::UYVY_422: - return format == DefaultBufferFormatForImageFormat(internalformat); + return format == BufferFormatForInternalFormat(internalformat); case gfx::BufferFormat::RGBA_4444: return internalformat == GL_RGBA; } diff --git a/gpu/command_buffer/tests/gl_copy_tex_image_2d_workaround_unittest.cc b/gpu/command_buffer/tests/gl_copy_tex_image_2d_workaround_unittest.cc index cb80ae3..e426794 100644 --- a/gpu/command_buffer/tests/gl_copy_tex_image_2d_workaround_unittest.cc +++ b/gpu/command_buffer/tests/gl_copy_tex_image_2d_workaround_unittest.cc @@ -60,7 +60,7 @@ TEST_P(GLCopyTexImage2DWorkaroundTest, UseIntermediaryTexture) { glTexParameteri(source_target, GL_TEXTURE_MIN_FILTER, GL_LINEAR); glTexParameteri(source_target, GL_TEXTURE_MAG_FILTER, GL_LINEAR); GLuint image_id = glCreateGpuMemoryBufferImageCHROMIUM( - width, height, GL_BGRA_EXT, GL_READ_WRITE_CHROMIUM); + width, height, GL_RGBA, GL_READ_WRITE_CHROMIUM); ASSERT_NE(0u, image_id); glBindTexImage2DCHROMIUM(source_target, image_id); diff --git a/gpu/command_buffer/tests/gl_iosurface_readback_workaround_unittest.cc b/gpu/command_buffer/tests/gl_iosurface_readback_workaround_unittest.cc index 6a1a7c6..e8e7bc5 100644 --- a/gpu/command_buffer/tests/gl_iosurface_readback_workaround_unittest.cc +++ b/gpu/command_buffer/tests/gl_iosurface_readback_workaround_unittest.cc @@ -56,7 +56,7 @@ TEST_F(GLIOSurfaceReadbackWorkaroundTest, ReadPixels) { glTexParameteri(source_target, GL_TEXTURE_MIN_FILTER, GL_LINEAR); glTexParameteri(source_target, GL_TEXTURE_MAG_FILTER, GL_LINEAR); GLuint image_id = glCreateGpuMemoryBufferImageCHROMIUM( - width, height, GL_BGRA_EXT, GL_READ_WRITE_CHROMIUM); + width, height, GL_RGBA, GL_READ_WRITE_CHROMIUM); ASSERT_NE(0u, image_id); glBindTexImage2DCHROMIUM(source_target, image_id); diff --git a/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp b/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp index ade818e..44c74f6 100644 --- a/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp +++ b/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp @@ -253,7 +253,7 @@ Canvas2DLayerBridge::ImageInfo Canvas2DLayerBridge::createIOSurfaceBackedTexture } gpu::gles2::GLES2Interface* gl = contextGL(); - GLuint imageId = gl->CreateGpuMemoryBufferImageCHROMIUM(m_size.width(), m_size.height(), GL_BGRA_EXT, GC3D_SCANOUT_CHROMIUM); + GLuint imageId = gl->CreateGpuMemoryBufferImageCHROMIUM(m_size.width(), m_size.height(), GL_RGBA, GC3D_SCANOUT_CHROMIUM); if (!imageId) return Canvas2DLayerBridge::ImageInfo(); -- cgit v1.1