content/common/gpu/client/gl_helper.cc - Issue 143683003: Support format using enum argument for Async readback

Unified Diff: content/common/gpu/client/gl_helper.cc

Issue 143683003: Support format using enum argument for Async readback (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Code changed as per review comments. Created 6 years, 11 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« content/browser/renderer_host/render_widget_host_view_aura.cc ('K') | « content/common/gpu/client/gl_helper.h ('k') | content/port/browser/render_widget_host_view_port.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: content/common/gpu/client/gl_helper.cc

diff --git a/content/common/gpu/client/gl_helper.cc b/content/common/gpu/client/gl_helper.cc

index 7da9389bc10ef9c31f001eb5757a344f3ba69f30..5fb641ca8bea8043abc528894103fd7708fce8a5 100644

--- a/content/common/gpu/client/gl_helper.cc

+++ b/content/common/gpu/client/gl_helper.cc

@@ -127,7 +127,7 @@ class GLHelper::CopyTextureToImpl

const gfx::Rect& src_subrect,

const gfx::Size& dst_size,

unsigned char* out,

- bool readback_config_rgb565,

+ const SkBitmap::Config config,

const base::Callback<void(bool)>& callback,

GLHelper::ScalerQuality quality);

@@ -142,7 +142,7 @@ class GLHelper::CopyTextureToImpl

int32 bytes_per_row, // generally dst_size.width() * 4

int32 row_stride_bytes, // generally dst_size.width() * 4

unsigned char* out,

- bool readback_config_rgb565,

+ const SkBitmap::Config config,

const base::Callback<void(bool)>& callback);

void ReadbackPlane(TextureFrameBufferPair* source,

@@ -288,7 +288,7 @@ class GLHelper::CopyTextureToImpl

const gfx::Size& dst_size,

bool vertically_flip_texture,

bool swizzle,

- bool readback_config_rgb565,

+ SkBitmap::Config config,

GLHelper::ScalerQuality quality);

static void nullcallback(bool success) {}

@@ -334,8 +334,16 @@ GLuint GLHelper::CopyTextureToImpl::ScaleTexture(

const gfx::Size& dst_size,

bool vertically_flip_texture,

bool swizzle,

- bool readback_config_rgb565,

+ SkBitmap::Config config,

GLHelper::ScalerQuality quality) {

+ bool format_support = ((config == SkBitmap::kRGB_565_Config) ||

+ (config == SkBitmap::kARGB_8888_Config));

+ if (!format_support) {

+ DCHECK(format_support);

+ LOG(ERROR) << "Format not supported.";

piman 2014/01/22 22:10:21 no need for this LOG. DCHECK is enough.

sivag 2014/01/23 12:29:20 Done.

+ return GL_INVALID_ENUM;

piman 2014/01/22 22:10:21 Maybe return 0 rather than GL_INVALID_ENUM. GL_INV

sivag 2014/01/23 12:29:20 Done.

+ }

scoped_ptr<ScalerInterface> scaler(

helper_->CreateScaler(quality,

src_size,

@@ -343,15 +351,25 @@ GLuint GLHelper::CopyTextureToImpl::ScaleTexture(

dst_size,

vertically_flip_texture,

swizzle));

GLuint dst_texture = 0u;

gl_->GenTextures(1, &dst_texture);

{

ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, dst_texture);

- GLenum format = readback_config_rgb565 ? GL_RGB : GL_RGBA;

- GLenum type = readback_config_rgb565 ?

- GL_UNSIGNED_SHORT_5_6_5 :

- GL_UNSIGNED_BYTE;

+ // Start with ARGB8888 params as any other format which is not

+ // supported is already asserted above.

+ GLenum format = GL_RGBA , type = GL_UNSIGNED_BYTE;

+ switch (config) {

+ case SkBitmap::kARGB_8888_Config:

+ // Do nothing params already set.

+ break;

+ case SkBitmap::kRGB_565_Config:

+ format = GL_RGB;

+ type = GL_UNSIGNED_SHORT_5_6_5;

+ break;

+ default:

+ NOTIMPLEMENTED();

piman 2014/01/22 22:10:21 nit: NOTREACHED() since it's tested above.

sivag 2014/01/23 12:29:20 Done.

+ break;

+ }

gl_->TexImage2D(GL_TEXTURE_2D,

format,

@@ -371,13 +389,38 @@ void GLHelper::CopyTextureToImpl::ReadbackAsync(

int32 bytes_per_row,

int32 row_stride_bytes,

unsigned char* out,

- bool readback_config_rgb565,

+ const SkBitmap::Config config,

const base::Callback<void(bool)>& callback) {

+ bool format_support = ((config == SkBitmap::kRGB_565_Config) ||

+ (config == SkBitmap::kARGB_8888_Config));

+ if (!format_support) {

+ DCHECK(format_support);

+ LOG(ERROR) << "Format not supported.";

piman 2014/01/22 22:10:21 no need for the LOG.

sivag 2014/01/23 12:29:20 Done.

+ callback.Run(false);

+ return;

+ }

Request* request =

new Request(dst_size, bytes_per_row, row_stride_bytes, out, callback);

request_queue_.push(request);

request->buffer = 0u;

- int bytes_per_pixel = readback_config_rgb565 ? 2 : 4;

+ // Start with ARGB8888 params as any other format which is not

+ // supported is already asserted above.

+ GLenum format = GL_RGBA , type = GL_UNSIGNED_BYTE;

piman 2014/01/22 22:10:21 nit: no space before ,

sivag 2014/01/23 12:29:20 Done.

+ int bytes_per_pixel = 4;

+ switch (config) {

+ case SkBitmap::kARGB_8888_Config:

+ // Do nothing params already set.

+ break;

+ case SkBitmap::kRGB_565_Config:

+ format = GL_RGB;

+ type = GL_UNSIGNED_SHORT_5_6_5;

+ bytes_per_pixel = 2;

+ break;

+ default:

+ NOTIMPLEMENTED();

piman 2014/01/22 22:10:21 nit: NOTREACHED()

sivag 2014/01/23 12:29:20 Done.

+ break;

+ }

gl_->GenBuffers(1, &request->buffer);

gl_->BindBuffer(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM, request->buffer);

gl_->BufferData(GL_PIXEL_PACK_TRANSFER_BUFFER_CHROMIUM,

@@ -388,10 +431,6 @@ void GLHelper::CopyTextureToImpl::ReadbackAsync(

request->query = 0u;

gl_->GenQueriesEXT(1, &request->query);

gl_->BeginQueryEXT(GL_ASYNC_PIXEL_PACK_COMPLETED_CHROMIUM, request->query);

- GLenum format = readback_config_rgb565 ? GL_RGB : GL_RGBA;

- GLenum type = readback_config_rgb565 ?

- GL_UNSIGNED_SHORT_5_6_5 :

- GL_UNSIGNED_BYTE;

gl_->ReadPixels(0,

dst_size.width(),

@@ -412,9 +451,17 @@ void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(

const gfx::Rect& src_subrect,

const gfx::Size& dst_size,

unsigned char* out,

- bool readback_config_rgb565,

+ const SkBitmap::Config bitmap_config,

const base::Callback<void(bool)>& callback,

GLHelper::ScalerQuality quality) {

+ bool format_support = ((bitmap_config == SkBitmap::kRGB_565_Config) ||

+ (bitmap_config == SkBitmap::kARGB_8888_Config));

+ if (!format_support) {

+ DCHECK(format_support);

+ LOG(ERROR) << "Format not supported.";

piman 2014/01/22 22:10:21 nit: remove.

sivag 2014/01/23 12:29:20 Done.

+ callback.Run(false);

+ return;

+ }

GLuint texture = ScaleTexture(src_texture,

src_size,

src_subrect,

@@ -425,8 +472,12 @@ void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(

#else

false,

#endif

- readback_config_rgb565,

+ bitmap_config,

quality);

+ if(texture == GL_INVALID_ENUM) {

piman 2014/01/22 22:10:21 nit: you can just DCHECK(texture), since you teste

sivag 2014/01/23 12:29:20 Done.

+ callback.Run(false);

+ return;

+ }

ScopedFramebuffer dst_framebuffer(gl_);

ScopedFramebufferBinder<GL_FRAMEBUFFER> framebuffer_binder(gl_,

dst_framebuffer);

@@ -436,12 +487,23 @@ void GLHelper::CopyTextureToImpl::CropScaleReadbackAndCleanTexture(

GL_TEXTURE_2D,

texture,

0);

- int bytes_per_pixel = readback_config_rgb565 ? 2 : 4;

+ int bytes_per_pixel = 4;

+ switch (bitmap_config) {

+ case SkBitmap::kARGB_8888_Config:

+ // Do nothing params already set.

+ break;

+ case SkBitmap::kRGB_565_Config:

+ bytes_per_pixel = 2;

+ break;

+ default:

+ NOTIMPLEMENTED();

piman 2014/01/22 22:10:21 NOTREACHED()

sivag 2014/01/23 12:29:20 Done.

+ break;

+ }

ReadbackAsync(dst_size,

dst_size.width() * bytes_per_pixel,

out,

- readback_config_rgb565,

+ bitmap_config,

callback);

gl_->DeleteTextures(1, &texture);

}

@@ -450,14 +512,14 @@ void GLHelper::CopyTextureToImpl::ReadbackTextureSync(GLuint texture,

const gfx::Rect& src_rect,

unsigned char* out,

SkBitmap::Config config) {

+ DCHECK((config == SkBitmap::kRGB_565_Config) ||

+ (config == SkBitmap::kARGB_8888_Config));

ScopedFramebuffer dst_framebuffer(gl_);

ScopedFramebufferBinder<GL_FRAMEBUFFER> framebuffer_binder(gl_,

dst_framebuffer);

ScopedTextureBinder<GL_TEXTURE_2D> texture_binder(gl_, texture);

gl_->FramebufferTexture2D(

GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, texture, 0);

- DCHECK((config == SkBitmap::kRGB_565_Config) ||

- (config == SkBitmap::kARGB_8888_Config));

GLenum format = (config == SkBitmap::kRGB_565_Config) ?

GL_RGB :

GL_RGBA;

@@ -485,7 +547,7 @@ GLuint GLHelper::CopyTextureToImpl::CopyAndScaleTexture(

dst_size,

vertically_flip_texture,

false,

- false,

+ SkBitmap::kARGB_8888_Config,

quality);

}

@@ -568,7 +630,7 @@ void GLHelper::CropScaleReadbackAndCleanTexture(

const gfx::Rect& src_subrect,

const gfx::Size& dst_size,

unsigned char* out,

- bool readback_config_rgb565,

+ const SkBitmap::Config config,

const base::Callback<void(bool)>& callback) {

InitCopyTextToImpl();

copy_texture_to_impl_->CropScaleReadbackAndCleanTexture(

@@ -577,7 +639,7 @@ void GLHelper::CropScaleReadbackAndCleanTexture(

src_subrect,

dst_size,

out,

- readback_config_rgb565,

+ config,

callback,

GLHelper::SCALER_QUALITY_FAST);

}

@@ -589,12 +651,12 @@ void GLHelper::CropScaleReadbackAndCleanMailbox(

const gfx::Rect& src_subrect,

const gfx::Size& dst_size,

unsigned char* out,

- bool readback_config_rgb565,

+ const SkBitmap::Config bitmap_config,

const base::Callback<void(bool)>& callback) {

GLuint mailbox_texture = ConsumeMailboxToTexture(src_mailbox, sync_point);

CropScaleReadbackAndCleanTexture(

mailbox_texture, src_size, src_subrect, dst_size, out,

- readback_config_rgb565,

+ bitmap_config,

callback);

gl_->DeleteTextures(1, &mailbox_texture);

}

@@ -826,7 +888,7 @@ void GLHelper::CopyTextureToImpl::ReadbackPlane(

dst_subrect.width() >> size_shift,

target->stride(plane),

target->data(plane) + offset,

- false,

+ SkBitmap::kARGB_8888_Config,

callback);

}