[CHROMIUM] Use the BGRA format for canvas 2D accel upload and readback.
Created attachment 64039 [details] Patch
Comment on attachment 64039 [details] Patch > Index: WebCore/platform/graphics/chromium/GLES2Texture.cpp > + case GLES2Texture::BGRA8: > + if (true) { // FIXME: check for BGRA extension support > + *glFormat = 0x80E1; > + *glType = GraphicsContext3D::UNSIGNED_BYTE; > + } else { > + *glFormat = GraphicsContext3D::RGBA; > + *glType = GraphicsContext3D::UNSIGNED_BYTE; > + *swizzle = true; Don't keep dead code around. If we need this later (which I think we will) we can get it out of SVN history. > void PlatformContextSkia::readbackHardwareToSoftware() const > + // FIXME: Check for BGRA extension support > + if (true) > + context->readPixels(0, height - 1 - y, width, 1, 0x80E1, GraphicsContext3D::UNSIGNED_BYTE, pixels); > + else { > + context->readPixels(0, height - 1 - y, width, 1, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, pixels); > + for (int i = 0; i < width; ++i) { > + uint32_t pixel = pixels[i]; > + // Swizzles from RGBA -> BGRA. > + pixels[i] = pixel & 0xFF00FF00 | ((pixel & 0x00FF0000) >> 16) | ((pixel & 0x000000FF) << 16); > + } > } Same here - keep the FIXME but nuke the unreachable code.
(In reply to comment #2) > (From update of attachment 64039 [details]) > > Index: WebCore/platform/graphics/chromium/GLES2Texture.cpp > > + case GLES2Texture::BGRA8: > > + if (true) { // FIXME: check for BGRA extension support > > + *glFormat = 0x80E1; > > + *glType = GraphicsContext3D::UNSIGNED_BYTE; > > + } else { > > + *glFormat = GraphicsContext3D::RGBA; > > + *glType = GraphicsContext3D::UNSIGNED_BYTE; > > + *swizzle = true; > > Don't keep dead code around. If we need this later (which I think we will) we can get it out of SVN history. In that case, I'll see about landing the change to GraphicsContext3D first. We'll definitely need this code later. > > void PlatformContextSkia::readbackHardwareToSoftware() const > > + // FIXME: Check for BGRA extension support > > + if (true) > > + context->readPixels(0, height - 1 - y, width, 1, 0x80E1, GraphicsContext3D::UNSIGNED_BYTE, pixels); > > + else { > > + context->readPixels(0, height - 1 - y, width, 1, GraphicsContext3D::RGBA, GraphicsContext3D::UNSIGNED_BYTE, pixels); > > + for (int i = 0; i < width; ++i) { > > + uint32_t pixel = pixels[i]; > > + // Swizzles from RGBA -> BGRA. > > + pixels[i] = pixel & 0xFF00FF00 | ((pixel & 0x00FF0000) >> 16) | ((pixel & 0x000000FF) << 16); > > + } > > } > > Same here - keep the FIXME but nuke the unreachable code.
Created attachment 64233 [details] Patch
Looks good to me!
Comment on attachment 64233 [details] Patch > Index: WebCore/platform/graphics/chromium/GLES2Texture.cpp > =================================================================== > --- WebCore/platform/graphics/chromium/GLES2Texture.cpp (revision 65251) > +++ WebCore/platform/graphics/chromium/GLES2Texture.cpp (working copy) > @@ -54,6 +54,30 @@ GLES2Texture::~GLES2Texture() > m_context->deleteTexture(m_textureId); > } > > +static void convertFormat(GraphicsContext3D* context, GLES2Texture::Format format, unsigned int* glFormat, unsigned int* glType, bool* swizzle) .. > + default: > + ASSERT(!"bad format"); This is odd, but I see it existed before. Anyway please consider using ASSERT_NOT_REACHED(); instead. (Also if the list of enum is short, it is considered preferable to list them and remove the default to allow the compiler to flag enums not being handled.) > + break; > + } > +} > +
Committed r65323: <http://trac.webkit.org/changeset/65323>