[Webkit-unassigned] [Bug 98728] [Texmap] Swizzling BGRA to RGBA in temp buffer while USE(TEXMAP_OPENGL_ES_2)

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=98728


Noam Rosenthal <noam.rosenthal at nokia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #169095|review?, commit-queue?      |review+, commit-queue-
               Flag|                            |




--- Comment #13 from Noam Rosenthal <noam.rosenthal at nokia.com>  2012-10-17 08:05:22 PST ---
(From update of attachment 169095)
View in context: https://bugs.webkit.org/attachment.cgi?id=169095&action=review

Great! Please fix nits and then commit or cq?

> Source/WebCore/ChangeLog:13
> +        BitmapTextureGL::updateContents() try to swizzle source Image if opengl
> +        driver doesn't support BGRA format.
> +        In case of directly composited image, swizzling source image
> +        should not be modified because another BitmapTexture will also try
> +        to swizzle.
> +
> +        BitmapTexture now provide UpdateContentsFlag to separate whether
> +        source image can be modified or not

Some rewording:
BitmapTexture swizzles the source image if the OpenGL driver doesn't support the BGRA extension.
In case of directly composited images, the source image should not be modified.

BitmapTexture::updateContents now accepts an UpdateContentsFlag argument to separate the different cases.

> Source/WebCore/ChangeLog:19
> +        No new tests.

This should be tested - there are a few current test cases for directly composited images, do they test this?

> Source/WebCore/platform/graphics/texmap/TextureMapper.h:62
> +    enum UpdateContentsFlag {
> +        UpdateCanModifyOriginalImageData = 0x01,
> +        UpdateCannotModifyOriginalImageData = 0x02
> +    };

Any reason to use 0x01 and 0x02 instead of 0 and 1?

> Source/WebCore/platform/graphics/texmap/TextureMapperImageBuffer.cpp:57
> +void BitmapTextureImageBuffer::updateContents(Image* image, const IntRect& targetRect, const IntPoint& offset, UpdateContentsFlag /*updatecontentsflag*/)

You don't need the comment, just omit the argument name.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.