[webkit-reviews] review granted: [Bug 44993] gl-teximage.html and tex-image-and-sub-image-2d-with-image.html failed on Chromium Mac bots (10.5) : [Attachment 66132] revised patch: re-enabling failing tests

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has granted Zhenyao Mo <zmo at google.com>'s
request for review:
Bug 44993: gl-teximage.html and tex-image-and-sub-image-2d-with-image.html
failed on Chromium Mac bots (10.5)
https://bugs.webkit.org/show_bug.cgi?id=44993

Attachment 66132: revised patch: re-enabling failing tests
https://bugs.webkit.org/attachment.cgi?id=66132&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
Looks good; one minor stylistic issue.

> Index: WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp
> ===================================================================
> --- WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp	(revision
66549)
> +++ WebCore/platform/graphics/cg/GraphicsContext3DCG.cpp	(working copy)
> @@ -140,7 +140,15 @@ bool GraphicsContext3D::getImageData(Ima
>	   return false;
>      const UInt8* rgba = CFDataGetBytePtr(pixelData.get());
>      outputVector.resize(width * height * 4);
> -    bool rt = packPixels(rgba, srcDataFormat, width, height, 0,
> +    unsigned int srcUnpackAlignment = 0;
> +    size_t bytesPerRow = CGImageGetBytesPerRow(cgImage);
> +    unsigned int padding = bytesPerRow - componentsPerPixel * width;
> +    if (padding) {
> +	   srcUnpackAlignment = padding + 1;
> +	   while (bytesPerRow % srcUnpackAlignment)
> +	       srcUnpackAlignment++;
> +    }

This is a little subtle but I can't think of a better way to express it. Use
preincrement however (++srcUnpackAligment).

> +    bool rt = packPixels(rgba, srcDataFormat, width, height,
srcUnpackAlignment,
>			    format, type, neededAlphaOp, outputVector.data());
>      return rt;
>  }
> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog	(revision 66549)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,12 @@
> +2010-08-31  Zhenyao Mo  <zmo at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   gl-teximage.html and tex-image-and-sub-image-2d-with-image.html
failed on Chromium Mac bots (10.5)
> +	   https://bugs.webkit.org/show_bug.cgi?id=44993
> +
> +	   * platform/chromium/test_expectations.txt: Enabling the failing
tests with the fix.
> +
>  2010-08-31  Sam Weinig  <sam at webkit.org>
>  
>	   Reviewed by Gavin Barraclough.
> Index: LayoutTests/platform/chromium/test_expectations.txt
> ===================================================================
> --- LayoutTests/platform/chromium/test_expectations.txt	(revision
66549)
> +++ LayoutTests/platform/chromium/test_expectations.txt	(working copy)
> @@ -3165,7 +3165,3 @@ BUGWK44967 MAC : fast/multicol/paginate-
>  // New tests in r66523 and r66516
>  BUGWK44996 WIN : inspector/report-API-errors.html = TEXT
>  BUGWK44996 WIN : inspector/report-protocol-errors.html = TEXT
> -
> -// Failing after r44997
> -BUGWK44997 MAC : fast/canvas/webgl/gl-teximage.html = TEXT
> -BUGWK44997 MAC :
fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html = TEXT