cc/resources/resource_provider.cc - Issue 135273008: cc: Use ResourceProvider::Resouce::type to check the type of the given resource.

Unified Diff: cc/resources/resource_provider.cc

Issue 135273008: cc: Use ResourceProvider::Resouce::type to check the type of the given resource. (Closed) Base URL: https://git.chromium.org/chromium/src.git@master

Patch Set: In addition, clarify texture_pool 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

Index: cc/resources/resource_provider.cc

diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc

index b43164847a0ad7ac62db9c801c499cd1448d7745..dbeebd9137df7c8eccd5e3323089636ddec70d3b 100644

--- a/cc/resources/resource_provider.cc

+++ b/cc/resources/resource_provider.cc

@@ -190,7 +190,7 @@ ResourceProvider::Resource::Resource()

wrap_mode(0),

lost(false),

hint(TextureUsageAny),

- type(static_cast<ResourceType>(0)),

+ type(InvalidType),

format(RGBA_8888),

shared_bitmap(NULL) {}

@@ -198,6 +198,7 @@ ResourceProvider::Resource::~Resource() {}

ResourceProvider::Resource::Resource(GLuint texture_id,

gfx::Size size,

+ Origin origin,

GLenum target,

GLenum filter,

GLenum texture_pool,

@@ -214,7 +215,7 @@ ResourceProvider::Resource::Resource(GLuint texture_id,

imported_count(0),

exported_count(0),

locked_for_write(false),

- origin(Internal),

+ origin(origin),

marked_for_deletion(false),

pending_set_pixels(false),

set_pixels_completion_forced(false),

@@ -236,11 +237,13 @@ ResourceProvider::Resource::Resource(GLuint texture_id,

format(format),

shared_bitmap(NULL) {

DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT);

+ DCHECK(texture_pool || origin != Internal);

dshwang 2014/01/28 20:35:36 For this DCHECK, I change the constructor.

danakj 2014/01/28 21:59:02 DCHECK_EQ(origin == Internal, !!texture_pool) ?

}

ResourceProvider::Resource::Resource(uint8_t* pixels,

SharedBitmap* bitmap,

gfx::Size size,

+ Origin origin,

GLenum filter,

GLint wrap_mode)

: child_id(0),

@@ -253,7 +256,7 @@ ResourceProvider::Resource::Resource(uint8_t* pixels,

imported_count(0),

exported_count(0),

locked_for_write(false),

- origin(Internal),

+ origin(origin),

marked_for_deletion(false),

pending_set_pixels(false),

set_pixels_completion_forced(false),

@@ -275,6 +278,7 @@ ResourceProvider::Resource::Resource(uint8_t* pixels,

format(RGBA_8888),

shared_bitmap(bitmap) {

DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT);

+ DCHECK(pixels);

}

ResourceProvider::Child::Child() : marked_for_deletion(false) {}

@@ -392,8 +396,15 @@ ResourceProvider::ResourceId ResourceProvider::CreateGLTexture(

DCHECK(thread_checker_.CalledOnValidThread());

ResourceId id = next_id_++;

- Resource resource(

- 0, size, target, GL_LINEAR, texture_pool, wrap_mode, hint, format);

+ Resource resource(0,

+ size,

+ Resource::Internal,

+ target,

+ GL_LINEAR,

+ texture_pool,

+ wrap_mode,

+ hint,

+ format);

resource.allocated = false;

resources_[id] = resource;

return id;

@@ -415,7 +426,7 @@ ResourceProvider::ResourceId ResourceProvider::CreateBitmap(

ResourceId id = next_id_++;

Resource resource(

- pixels, bitmap.release(), size, GL_LINEAR, wrap_mode);

+ pixels, bitmap.release(), size, Resource::Internal, GL_LINEAR, wrap_mode);

resource.allocated = true;

resources_[id] = resource;

return id;

@@ -440,13 +451,13 @@ ResourceProvider::CreateResourceFromExternalTexture(

ResourceId id = next_id_++;

Resource resource(texture_id,

gfx::Size(),

+ Resource::External,

texture_target,

GL_LINEAR,

GL_CLAMP_TO_EDGE,

TextureUsageAny,

RGBA_8888);

- resource.origin = Resource::External;

resource.allocated = true;

resources_[id] = resource;

return id;

@@ -463,6 +474,7 @@ ResourceProvider::ResourceId ResourceProvider::CreateResourceFromTextureMailbox(

if (mailbox.IsTexture()) {

resource = Resource(0,

gfx::Size(),

+ Resource::External,

mailbox.target(),

GL_LINEAR,

@@ -482,10 +494,10 @@ ResourceProvider::ResourceId ResourceProvider::CreateResourceFromTextureMailbox(

resource = Resource(pixels,

shared_bitmap.release(),

mailbox.shared_memory_size(),

+ Resource::External,

GL_LINEAR,

GL_CLAMP_TO_EDGE);

}

- resource.origin = Resource::External;

resource.allocated = true;

resource.mailbox = mailbox;

resource.release_callback =

@@ -543,7 +555,8 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,

}

if (resource->mailbox.IsValid() && resource->origin == Resource::External) {

GLuint sync_point = resource->mailbox.sync_point();

- if (resource->mailbox.IsTexture()) {

+ if (resource->type == GLTexture) {

+ DCHECK(resource->mailbox.IsTexture());

lost_resource |= lost_output_surface_;

GLES2Interface* gl = ContextGL();

DCHECK(gl);

@@ -573,6 +586,7 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,

}

if (resource->shared_bitmap) {

DCHECK(resource->origin != Resource::External);

+ DCHECK_EQ(Bitmap, resource->type);

delete resource->shared_bitmap;

resource->pixels = NULL;

}

@@ -609,7 +623,8 @@ void ResourceProvider::SetPixels(ResourceId id,

DCHECK(ReadLockFenceHasPassed(resource));

LazyAllocate(resource);

- if (resource->gl_id) {

+ if (resource->type == GLTexture) {

+ DCHECK(resource->gl_id);

DCHECK(!resource->pending_set_pixels);

DCHECK_EQ(resource->target, static_cast<GLenum>(GL_TEXTURE_2D));

GLES2Interface* gl = ContextGL();

@@ -622,9 +637,8 @@ void ResourceProvider::SetPixels(ResourceId id,

dest_offset,

resource->format,

resource->size);

- }

- if (resource->pixels) {

+ } else {

+ DCHECK_EQ(Bitmap, resource->type);

DCHECK(resource->allocated);

DCHECK_EQ(RGBA_8888, resource->format);

SkBitmap src_full;

@@ -747,8 +761,9 @@ const ResourceProvider::Resource* ResourceProvider::LockForRead(ResourceId id) {

LazyCreate(resource);

- if (!resource->gl_id && resource->mailbox.IsTexture()) {

+ if (!resource->gl_id && resource->type == GLTexture) {

danakj 2014/01/28 21:59:02 can you reverse these 2 conditions? checking the g

DCHECK(resource->origin != Resource::Internal);

+ DCHECK(resource->mailbox.IsTexture());

GLES2Interface* gl = ContextGL();

DCHECK(gl);

if (resource->mailbox.sync_point()) {

@@ -1087,11 +1102,16 @@ void ResourceProvider::ReceiveFromChild(

ResourceId local_id = next_id_++;

Resource& resource = resources_[local_id];

if (it->is_software) {

- resource = Resource(

- pixels, bitmap.release(), it->size, GL_LINEAR, GL_CLAMP_TO_EDGE);

+ resource = Resource(pixels,

+ bitmap.release(),

+ it->size,

+ Resource::Delegated,

+ GL_LINEAR,

+ GL_CLAMP_TO_EDGE);

} else {

resource = Resource(0,

it->size,

+ Resource::Delegated,

it->target,

it->filter,

@@ -1102,7 +1122,6 @@ void ResourceProvider::ReceiveFromChild(

TextureMailbox(it->mailbox, it->target, it->sync_point);

}

resource.child_id = child;

- resource.origin = Resource::Delegated;

// Don't allocate a texture for a child.

resource.allocated = true;

resource.imported_count = 1;

@@ -1272,7 +1291,8 @@ void ResourceProvider::TransferResource(GLES2Interface* gl,

resource->filter = source->filter;

resource->size = source->size;

- if (source->shared_bitmap) {

+ if (source->type == Bitmap) {

+ DCHECK(source->shared_bitmap);

danakj 2014/01/28 21:59:02 given the next line will crash anyway, not sure we

resource->mailbox = source->shared_bitmap->id();

resource->is_software = true;

} else if (!source->mailbox.IsValid()) {

@@ -1337,7 +1357,8 @@ void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(

DCHECK(child_info->child_to_parent_map.count(child_id));

bool is_lost =

- resource.lost || (!resource.shared_bitmap && lost_output_surface_);

+ resource.lost ||

+ (resource.type == GLTexture && lost_output_surface_);

if (resource.exported_count > 0) {

if (style != ForShutdown) {

// Defer this until we receive the resource back from the parent.

@@ -1367,7 +1388,7 @@ void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(

ReturnedResource returned;

returned.id = child_id;

returned.sync_point = resource.mailbox.sync_point();

- if (!returned.sync_point && !resource.shared_bitmap)

+ if (!returned.sync_point && resource.type == GLTexture)

need_sync_point = true;

returned.count = resource.imported_count;

returned.lost = is_lost;

@@ -1418,9 +1439,8 @@ void ResourceProvider::AcquirePixelBuffer(ResourceId id) {

NULL,

GL_DYNAMIC_DRAW);

gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);

- }

- if (resource->pixels) {

+ } else {

+ DCHECK_EQ(Bitmap, resource->type);

if (resource->pixel_buffer)

return;

@@ -1456,9 +1476,8 @@ void ResourceProvider::ReleasePixelBuffer(ResourceId id) {

gl->BufferData(

GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0, NULL, GL_DYNAMIC_DRAW);

gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);

- }

- if (resource->pixels) {

+ } else {

+ DCHECK_EQ(Bitmap, resource->type);

if (!resource->pixel_buffer)

return;

delete[] resource->pixel_buffer;

@@ -1485,11 +1504,8 @@ uint8_t* ResourceProvider::MapPixelBuffer(ResourceId id) {

CHECK(!(reinterpret_cast<intptr_t>(image) & 3));

return image;

}

- if (resource->pixels)

- return resource->pixel_buffer;

- return NULL;

+ DCHECK_EQ(Bitmap, resource->type);

+ return resource->pixel_buffer;

}

void ResourceProvider::UnmapPixelBuffer(ResourceId id) {

@@ -1549,7 +1565,8 @@ void ResourceProvider::BeginSetPixels(ResourceId id) {

resource->allocated = true;

LockForWrite(id);

- if (resource->gl_id) {

+ if (resource->type == GLTexture) {

dshwang 2014/01/28 20:35:36 For this change, I changed ResourceProvider::LazyC

+ DCHECK(resource->gl_id);

GLES2Interface* gl = ContextGL();

DCHECK(gl);

DCHECK(resource->gl_pixel_buffer_id);

@@ -1584,9 +1601,8 @@ void ResourceProvider::BeginSetPixels(ResourceId id) {

}

gl->EndQueryEXT(GL_ASYNC_PIXEL_UNPACK_COMPLETED_CHROMIUM);

gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);

- }

- if (resource->pixels) {

+ } else {

+ DCHECK_EQ(Bitmap, resource->type);

DCHECK(!resource->mailbox.IsValid());

DCHECK(resource->pixel_buffer);

DCHECK_EQ(RGBA_8888, resource->format);

@@ -1648,13 +1664,15 @@ GLenum ResourceProvider::TargetForTesting(ResourceId id) {

}

void ResourceProvider::LazyCreate(Resource* resource) {

- if (resource->type != GLTexture || resource->gl_id != 0)

+ if (resource->type != GLTexture || resource->origin != Resource::Internal) {

+ DCHECK(!resource->texture_pool);

danakj 2014/01/28 21:59:02 I think these DCHECKs can be covered in the constr

return;

+ }

- // Early out for resources that don't require texture creation.

- if (resource->texture_pool == 0)

+ if (resource->gl_id)

return;

+ DCHECK(resource->texture_pool);

dshwang 2014/01/28 20:35:36 how about the change of LazyCreate()?

DCHECK(resource->origin == Resource::Internal);

DCHECK(!resource->mailbox.IsValid());

resource->gl_id = texture_id_allocator_->NextId();

@@ -1788,17 +1806,15 @@ uint8_t* ResourceProvider::MapImage(ResourceId id) {

DCHECK(resource->origin == Resource::Internal);

DCHECK_EQ(resource->exported_count, 0);

- if (resource->image_id) {

+ if (resource->type == GLTexture) {

+ DCHECK(resource->image_id);

GLES2Interface* gl = ContextGL();

DCHECK(gl);

return static_cast<uint8_t*>(

gl->MapImageCHROMIUM(resource->image_id, GL_READ_WRITE));

}

- if (resource->pixels)

- return resource->pixels;

- return NULL;

+ DCHECK_EQ(Bitmap, resource->type);

+ return resource->pixels;

}

void ResourceProvider::UnmapImage(ResourceId id) {

« cc/resources/resource_provider.h ('K') | « cc/resources/resource_provider.h ('k') | no next file » | no next file with comments »