 Chromium Code Reviews
 Chromium Code Reviews Issue 21159007:
  cc: Adding support for RGBA_4444 tile textures  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 21159007:
  cc: Adding support for RGBA_4444 tile textures  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: cc/resources/resource_provider.cc | 
| diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc | 
| index 787757c3b86f62847ec3ae99a27747bd0553b414..c09af42189f0a3937e66d09d0d641036902bb997 100644 | 
| --- a/cc/resources/resource_provider.cc | 
| +++ b/cc/resources/resource_provider.cc | 
| @@ -34,24 +34,31 @@ namespace { | 
| const double kSoftwareUploadTickRate = 0.000250; | 
| const double kTextureUploadTickRate = 0.004; | 
| -GLenum TextureToStorageFormat(GLenum texture_format) { | 
| +GLenum TextureToStorageFormat(cc::ResourceProvider::Format format) { | 
| GLenum storage_format = GL_RGBA8_OES; | 
| - switch (texture_format) { | 
| - case GL_RGBA: | 
| + switch (format) { | 
| + case cc::ResourceProvider::RGBA_8888: | 
| + case cc::ResourceProvider::RGBA_4444: | 
| + case cc::ResourceProvider::LUMINANCE_8: | 
| break; | 
| - case GL_BGRA_EXT: | 
| + case cc::ResourceProvider::BGRA_8888: | 
| storage_format = GL_BGRA8_EXT; | 
| break; | 
| - default: | 
| - NOTREACHED(); | 
| - break; | 
| } | 
| return storage_format; | 
| } | 
| -bool IsTextureFormatSupportedForStorage(GLenum format) { | 
| - return (format == GL_RGBA || format == GL_BGRA_EXT); | 
| +bool IsFormatSupportedForStorage(cc::ResourceProvider::Format format) { | 
| + switch (format) { | 
| + case cc::ResourceProvider::RGBA_8888: | 
| + case cc::ResourceProvider::BGRA_8888: | 
| + return true; | 
| + case cc::ResourceProvider::RGBA_4444: | 
| + case cc::ResourceProvider::LUMINANCE_8: | 
| + return false; | 
| + } | 
| + return false; | 
| } | 
| class ScopedSetActiveTexture { | 
| @@ -95,24 +102,24 @@ ResourceProvider::Resource::Resource() | 
| enable_read_lock_fences(false), | 
| read_lock_fence(NULL), | 
| size(), | 
| - format(0), | 
| filter(0), | 
| image_id(0), | 
| texture_pool(0), | 
| wrap_mode(0), | 
| hint(TextureUsageAny), | 
| - type(static_cast<ResourceType>(0)) {} | 
| + type(static_cast<ResourceType>(0)), | 
| + format(RGBA_8888) {} | 
| ResourceProvider::Resource::~Resource() {} | 
| ResourceProvider::Resource::Resource( | 
| unsigned texture_id, | 
| gfx::Size size, | 
| - GLenum format, | 
| GLenum filter, | 
| GLenum texture_pool, | 
| GLint wrap_mode, | 
| - TextureUsageHint hint) | 
| + TextureUsageHint hint, | 
| + Format format) | 
| : gl_id(texture_id), | 
| gl_pixel_buffer_id(0), | 
| gl_upload_query_id(0), | 
| @@ -130,20 +137,19 @@ ResourceProvider::Resource::Resource( | 
| enable_read_lock_fences(false), | 
| read_lock_fence(NULL), | 
| size(size), | 
| - format(format), | 
| filter(filter), | 
| image_id(0), | 
| texture_pool(texture_pool), | 
| wrap_mode(wrap_mode), | 
| hint(hint), | 
| - type(GLTexture) { | 
| + type(GLTexture), | 
| + format(format) { | 
| DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT); | 
| } | 
| ResourceProvider::Resource::Resource( | 
| uint8_t* pixels, | 
| gfx::Size size, | 
| - GLenum format, | 
| GLenum filter, | 
| GLint wrap_mode) | 
| : gl_id(0), | 
| @@ -163,13 +169,13 @@ ResourceProvider::Resource::Resource( | 
| enable_read_lock_fences(false), | 
| read_lock_fence(NULL), | 
| size(size), | 
| - format(format), | 
| filter(filter), | 
| image_id(0), | 
| texture_pool(0), | 
| wrap_mode(wrap_mode), | 
| hint(TextureUsageAny), | 
| - type(Bitmap) { | 
| + type(Bitmap), | 
| + format(RGBA_8888) { | 
| DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT); | 
| } | 
| @@ -181,7 +187,8 @@ scoped_ptr<ResourceProvider> ResourceProvider::Create( | 
| OutputSurface* output_surface, | 
| int highp_threshold_min) { | 
| scoped_ptr<ResourceProvider> resource_provider( | 
| - new ResourceProvider(output_surface, highp_threshold_min)); | 
| + new ResourceProvider(output_surface, | 
| + highp_threshold_min)); | 
| 
reveman
2013/09/13 20:24:31
nit: looks like this fits on one line
 
kaanb
2013/09/13 22:35:23
Done.
 | 
| bool success = false; | 
| if (resource_provider->Context3d()) { | 
| @@ -211,17 +218,20 @@ bool ResourceProvider::InUseByConsumer(ResourceId id) { | 
| } | 
| ResourceProvider::ResourceId ResourceProvider::CreateResource( | 
| - gfx::Size size, GLenum format, GLint wrap_mode, TextureUsageHint hint) { | 
| + gfx::Size size, | 
| + GLint wrap_mode, | 
| + TextureUsageHint hint, | 
| + Format format) { | 
| DCHECK(!size.IsEmpty()); | 
| switch (default_resource_type_) { | 
| case GLTexture: | 
| - return CreateGLTexture(size, format, GL_TEXTURE_POOL_UNMANAGED_CHROMIUM, | 
| - wrap_mode, hint); | 
| + return CreateGLTexture(size, | 
| + GL_TEXTURE_POOL_UNMANAGED_CHROMIUM, | 
| + wrap_mode, | 
| + hint, | 
| + format); | 
| case Bitmap: | 
| - // The only wrap_mode currently implemented in software mode is | 
| - // GL_CLAMP_TO_EDGE. | 
| - // http://crbug.com/284796 | 
| - DCHECK(format == GL_RGBA); | 
| + DCHECK_EQ(RGBA_8888, format); | 
| return CreateBitmap(size); | 
| case InvalidType: | 
| break; | 
| @@ -232,14 +242,20 @@ ResourceProvider::ResourceId ResourceProvider::CreateResource( | 
| } | 
| ResourceProvider::ResourceId ResourceProvider::CreateManagedResource( | 
| - gfx::Size size, GLenum format, GLint wrap_mode, TextureUsageHint hint) { | 
| + gfx::Size size, | 
| + GLint wrap_mode, | 
| + TextureUsageHint hint, | 
| + Format format) { | 
| DCHECK(!size.IsEmpty()); | 
| switch (default_resource_type_) { | 
| case GLTexture: | 
| - return CreateGLTexture(size, format, GL_TEXTURE_POOL_MANAGED_CHROMIUM, | 
| - wrap_mode, hint); | 
| + return CreateGLTexture(size, | 
| + GL_TEXTURE_POOL_MANAGED_CHROMIUM, | 
| + wrap_mode, | 
| + hint, | 
| + format); | 
| case Bitmap: | 
| - DCHECK(format == GL_RGBA); | 
| + DCHECK_EQ(RGBA_8888, format); | 
| return CreateBitmap(size); | 
| case InvalidType: | 
| break; | 
| @@ -251,16 +267,16 @@ ResourceProvider::ResourceId ResourceProvider::CreateManagedResource( | 
| ResourceProvider::ResourceId ResourceProvider::CreateGLTexture( | 
| gfx::Size size, | 
| - GLenum format, | 
| GLenum texture_pool, | 
| GLint wrap_mode, | 
| - TextureUsageHint hint) { | 
| + TextureUsageHint hint, | 
| + Format format) { | 
| DCHECK_LE(size.width(), max_texture_size_); | 
| DCHECK_LE(size.height(), max_texture_size_); | 
| DCHECK(thread_checker_.CalledOnValidThread()); | 
| ResourceId id = next_id_++; | 
| - Resource resource(0, size, format, GL_LINEAR, texture_pool, wrap_mode, hint); | 
| + Resource resource(0, size, GL_LINEAR, texture_pool, wrap_mode, hint, format); | 
| resource.allocated = false; | 
| resources_[id] = resource; | 
| return id; | 
| @@ -272,7 +288,7 @@ ResourceProvider::ResourceId ResourceProvider::CreateBitmap(gfx::Size size) { | 
| uint8_t* pixels = new uint8_t[4 * size.GetArea()]; | 
| ResourceId id = next_id_++; | 
| - Resource resource(pixels, size, GL_RGBA, GL_LINEAR, GL_CLAMP_TO_EDGE); | 
| + Resource resource(pixels, size, GL_LINEAR, GL_CLAMP_TO_EDGE); | 
| resource.allocated = true; | 
| resources_[id] = resource; | 
| return id; | 
| @@ -297,8 +313,13 @@ ResourceProvider::CreateResourceFromExternalTexture( | 
| texture_target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE)); | 
| ResourceId id = next_id_++; | 
| - Resource resource(texture_id, gfx::Size(), 0, GL_LINEAR, 0, GL_CLAMP_TO_EDGE, | 
| - TextureUsageAny); | 
| + Resource resource(texture_id, | 
| + gfx::Size(), | 
| + GL_LINEAR, | 
| + 0, | 
| + GL_CLAMP_TO_EDGE, | 
| + TextureUsageAny, | 
| + RGBA_8888); | 
| resource.external = true; | 
| resource.allocated = true; | 
| resources_[id] = resource; | 
| @@ -313,15 +334,20 @@ ResourceProvider::ResourceId ResourceProvider::CreateResourceFromTextureMailbox( | 
| DCHECK(mailbox.IsValid()); | 
| Resource& resource = resources_[id]; | 
| if (mailbox.IsTexture()) { | 
| - resource = Resource(0, gfx::Size(), 0, GL_LINEAR, 0, GL_CLAMP_TO_EDGE, | 
| - TextureUsageAny); | 
| + resource = Resource(0, | 
| + gfx::Size(), | 
| + GL_LINEAR, | 
| + 0, | 
| + GL_CLAMP_TO_EDGE, | 
| + TextureUsageAny, | 
| + RGBA_8888); | 
| } else { | 
| DCHECK(mailbox.IsSharedMemory()); | 
| base::SharedMemory* shared_memory = mailbox.shared_memory(); | 
| DCHECK(shared_memory->memory()); | 
| uint8_t* pixels = reinterpret_cast<uint8_t*>(shared_memory->memory()); | 
| - resource = Resource(pixels, mailbox.shared_memory_size(), | 
| - GL_RGBA, GL_LINEAR, GL_CLAMP_TO_EDGE); | 
| + resource = Resource( | 
| + pixels, mailbox.shared_memory_size(), GL_LINEAR, GL_CLAMP_TO_EDGE); | 
| } | 
| resource.external = true; | 
| resource.allocated = true; | 
| @@ -438,7 +464,7 @@ void ResourceProvider::SetPixels(ResourceId id, | 
| if (resource->pixels) { | 
| DCHECK(resource->allocated); | 
| - DCHECK(resource->format == GL_RGBA); | 
| + DCHECK_EQ(RGBA_8888, resource->format); | 
| SkBitmap src_full; | 
| src_full.setConfig( | 
| SkBitmap::kARGB_8888_Config, image_rect.width(), image_rect.height()); | 
| @@ -662,7 +688,7 @@ ResourceProvider::ScopedWriteLockGL::~ScopedWriteLockGL() { | 
| void ResourceProvider::PopulateSkBitmapWithResource( | 
| SkBitmap* sk_bitmap, const Resource* resource) { | 
| DCHECK(resource->pixels); | 
| - DCHECK(resource->format == GL_RGBA); | 
| + DCHECK_EQ(RGBA_8888, resource->format); | 
| sk_bitmap->setConfig(SkBitmap::kARGB_8888_Config, | 
| resource->size.width(), | 
| resource->size.height()); | 
| @@ -708,7 +734,7 @@ ResourceProvider::ResourceProvider(OutputSurface* output_surface, | 
| use_texture_usage_hint_(false), | 
| use_shallow_flush_(false), | 
| max_texture_size_(0), | 
| - best_texture_format_(0) { | 
| + best_texture_format_(RGBA_8888) { | 
| DCHECK(output_surface_->HasClient()); | 
| } | 
| @@ -720,7 +746,7 @@ void ResourceProvider::InitializeSoftware() { | 
| default_resource_type_ = Bitmap; | 
| max_texture_size_ = INT_MAX / 2; | 
| - best_texture_format_ = GL_RGBA; | 
| + best_texture_format_ = RGBA_8888; | 
| } | 
| bool ResourceProvider::InitializeGL() { | 
| @@ -746,7 +772,9 @@ bool ResourceProvider::InitializeGL() { | 
| use_texture_usage_hint_ = caps.texture_usage; | 
| texture_uploader_ = | 
| - TextureUploader::Create(context3d, use_map_sub, use_shallow_flush_); | 
| + TextureUploader::Create(context3d, | 
| + use_map_sub, | 
| + use_shallow_flush_); | 
| GLC(context3d, context3d->getIntegerv(GL_MAX_TEXTURE_SIZE, | 
| &max_texture_size_)); | 
| best_texture_format_ = PlatformColor::BestTextureFormat(use_bgra); | 
| @@ -909,10 +937,28 @@ void ResourceProvider::ReceiveFromChild( | 
| GLC(context3d, context3d->bindTexture(GL_TEXTURE_2D, texture_id)); | 
| GLC(context3d, context3d->consumeTextureCHROMIUM(GL_TEXTURE_2D, | 
| it->mailbox.name)); | 
| + // TODO(kaanb): change TransferableResource to use the Format enum. | 
| + Format format = RGBA_8888; | 
| + switch (it->format) { | 
| + case RGBA_8888: | 
| + case RGBA_4444: | 
| + case BGRA_8888: | 
| + case LUMINANCE_8: | 
| + format = static_cast<Format>(it->format); | 
| + break; | 
| + default: | 
| + NOTREACHED(); | 
| 
reveman
2013/09/13 20:24:31
As piman already pointed out, we can't use NOTREAC
 
kaanb
2013/09/13 22:35:23
We don't need the switch statement anymore as Tran
 | 
| + } | 
| + | 
| ResourceId id = next_id_++; | 
| Resource resource( | 
| - texture_id, it->size, it->format, it->filter, 0, GL_CLAMP_TO_EDGE, | 
| - TextureUsageAny); | 
| + texture_id, | 
| + it->size, | 
| + it->filter, | 
| + 0, | 
| + GL_CLAMP_TO_EDGE, | 
| + TextureUsageAny, | 
| + format); | 
| resource.mailbox.SetName(it->mailbox); | 
| // Don't allocate a texture for a child. | 
| resource.allocated = true; | 
| @@ -964,9 +1010,9 @@ void ResourceProvider::TransferResource(WebGraphicsContext3D* context, | 
| DCHECK(!source->external || (source->external && source->mailbox.IsValid())); | 
| DCHECK(source->allocated); | 
| resource->id = id; | 
| - resource->format = source->format; | 
| resource->filter = source->filter; | 
| resource->size = source->size; | 
| + resource->format = source->format; | 
| // TODO(skaslev) Implement this path for shared memory resources. | 
| DCHECK(!source->mailbox.IsSharedMemory()); | 
| @@ -1003,9 +1049,10 @@ void ResourceProvider::AcquirePixelBuffer(ResourceId id) { | 
| context3d->bindBuffer( | 
| GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, | 
| resource->gl_pixel_buffer_id); | 
| + size_t bytes_per_pixel = BytesPerPixel(resource->format); | 
| context3d->bufferData( | 
| GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, | 
| - 4 * resource->size.GetArea(), | 
| + bytes_per_pixel * resource->size.GetArea(), | 
| NULL, | 
| GL_DYNAMIC_DRAW); | 
| context3d->bindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0); | 
| @@ -1176,26 +1223,30 @@ void ResourceProvider::BeginSetPixels(ResourceId id) { | 
| context3d->beginQueryEXT( | 
| GL_ASYNC_PIXEL_UNPACK_COMPLETED_CHROMIUM, | 
| resource->gl_upload_query_id); | 
| + DCHECK(resource->format != RGBA_4444 || | 
| 
reveman
2013/09/13 20:24:31
Can we avoid this format specific check? Shouldn't
 
kaanb
2013/09/13 22:35:23
There are unittests that upload 13x13 textures in
 | 
| + (resource->size.width() % 2) == 0); | 
| if (allocate) { | 
| - context3d->asyncTexImage2DCHROMIUM(GL_TEXTURE_2D, | 
| - 0, /* level */ | 
| - resource->format, | 
| - resource->size.width(), | 
| - resource->size.height(), | 
| - 0, /* border */ | 
| - resource->format, | 
| - GL_UNSIGNED_BYTE, | 
| - NULL); | 
| + context3d->asyncTexImage2DCHROMIUM( | 
| + GL_TEXTURE_2D, | 
| + 0, /* level */ | 
| + GetGLInternalFormat(resource->format), | 
| + resource->size.width(), | 
| + resource->size.height(), | 
| + 0, /* border */ | 
| + GetGLDataFormat(resource->format), | 
| + GetGLDataType(resource->format), | 
| + NULL); | 
| } else { | 
| - context3d->asyncTexSubImage2DCHROMIUM(GL_TEXTURE_2D, | 
| - 0, /* level */ | 
| - 0, /* x */ | 
| - 0, /* y */ | 
| - resource->size.width(), | 
| - resource->size.height(), | 
| - resource->format, | 
| - GL_UNSIGNED_BYTE, | 
| - NULL); | 
| + context3d->asyncTexSubImage2DCHROMIUM( | 
| + GL_TEXTURE_2D, | 
| + 0, /* level */ | 
| + 0, /* x */ | 
| + 0, /* y */ | 
| + resource->size.width(), | 
| + resource->size.height(), | 
| + GetGLDataFormat(resource->format), | 
| + GetGLDataType(resource->format), | 
| + NULL); | 
| } | 
| context3d->endQueryEXT(GL_ASYNC_PIXEL_UNPACK_COMPLETED_CHROMIUM); | 
| context3d->bindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0); | 
| @@ -1204,7 +1255,7 @@ void ResourceProvider::BeginSetPixels(ResourceId id) { | 
| if (resource->pixels) { | 
| DCHECK(!resource->mailbox.IsValid()); | 
| DCHECK(resource->pixel_buffer); | 
| - DCHECK(resource->format == GL_RGBA); | 
| + DCHECK_EQ(RGBA_8888, resource->format); | 
| std::swap(resource->pixels, resource->pixel_buffer); | 
| delete[] resource->pixel_buffer; | 
| @@ -1310,9 +1361,9 @@ void ResourceProvider::LazyAllocate(Resource* resource) { | 
| resource->allocated = true; | 
| WebGraphicsContext3D* context3d = Context3d(); | 
| gfx::Size& size = resource->size; | 
| - GLenum format = resource->format; | 
| + Format format = resource->format; | 
| GLC(context3d, context3d->bindTexture(GL_TEXTURE_2D, resource->gl_id)); | 
| - if (use_texture_storage_ext_ && IsTextureFormatSupportedForStorage(format)) { | 
| + if (use_texture_storage_ext_ && IsFormatSupportedForStorage(format)) { | 
| GLenum storage_format = TextureToStorageFormat(format); | 
| GLC(context3d, context3d->texStorage2DEXT(GL_TEXTURE_2D, | 
| 1, | 
| @@ -1322,12 +1373,12 @@ void ResourceProvider::LazyAllocate(Resource* resource) { | 
| } else { | 
| GLC(context3d, context3d->texImage2D(GL_TEXTURE_2D, | 
| 0, | 
| - format, | 
| + GetGLInternalFormat(format), | 
| size.width(), | 
| size.height(), | 
| 0, | 
| - format, | 
| - GL_UNSIGNED_BYTE, | 
| + GetGLDataFormat(format), | 
| + GetGLDataType(format), | 
| NULL)); | 
| } | 
| } | 
| @@ -1352,7 +1403,7 @@ void ResourceProvider::AcquireImage(ResourceId id) { | 
| resource->allocated = true; | 
| WebGraphicsContext3D* context3d = Context3d(); | 
| DCHECK(context3d); | 
| - DCHECK_EQ(static_cast<GLenum>(GL_RGBA), resource->format); | 
| + DCHECK_EQ(RGBA_8888, resource->format); | 
| resource->image_id = context3d->createImageCHROMIUM( | 
| resource->size.width(), resource->size.height(), GL_RGBA8_OES); | 
| DCHECK(resource->image_id); | 
| @@ -1432,4 +1483,60 @@ WebKit::WebGraphicsContext3D* ResourceProvider::Context3d() const { | 
| return context_provider ? context_provider->Context3d() : NULL; | 
| } | 
| +size_t ResourceProvider::BytesPerPixel(Format format) { | 
| + size_t components_per_pixel = 0; | 
| + switch (format) { | 
| + case RGBA_8888: | 
| + case RGBA_4444: | 
| + case BGRA_8888: | 
| + components_per_pixel = 4; | 
| + break; | 
| + case LUMINANCE_8: | 
| + components_per_pixel = 1; | 
| + break; | 
| + } | 
| + size_t bits_per_component = 0; | 
| + switch (format) { | 
| + case RGBA_8888: | 
| + case BGRA_8888: | 
| + case LUMINANCE_8: | 
| + bits_per_component = 8; | 
| + break; | 
| + case RGBA_4444: | 
| + bits_per_component = 4; | 
| + break; | 
| + } | 
| + const size_t kBitsPerByte = 8; | 
| + return (components_per_pixel * bits_per_component) / kBitsPerByte; | 
| +} | 
| + | 
| +GLenum ResourceProvider::GetGLDataType(Format format) { | 
| + switch (format) { | 
| + case RGBA_4444: | 
| + return GL_UNSIGNED_SHORT_4_4_4_4; | 
| + case RGBA_8888: | 
| + case BGRA_8888: | 
| + case LUMINANCE_8: | 
| + return GL_UNSIGNED_BYTE; | 
| + } | 
| + return GL_UNSIGNED_BYTE; | 
| 
reveman
2013/09/13 20:24:31
I think you can add a NOTREACHED() above this line
 
kaanb
2013/09/13 22:35:23
Done.
 | 
| +} | 
| + | 
| +GLenum ResourceProvider::GetGLDataFormat(Format format) { | 
| + switch (format) { | 
| + case RGBA_8888: | 
| + case RGBA_4444: | 
| + return GL_RGBA; | 
| + case BGRA_8888: | 
| + return GL_BGRA_EXT; | 
| + case LUMINANCE_8: | 
| + return GL_LUMINANCE; | 
| + } | 
| + return GL_RGBA; | 
| 
reveman
2013/09/13 20:24:31
and NOTREACHED() here too.
 
kaanb
2013/09/13 22:35:23
Done.
 | 
| +} | 
| + | 
| +GLenum ResourceProvider::GetGLInternalFormat(Format format) { | 
| + return GetGLDataFormat(format); | 
| +} | 
| + | 
| } // namespace cc |