Bug 72234 - Test JPEG image decoding RGB pixel endianess
Summary: Test JPEG image decoding RGB pixel endianess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: noel gordon
URL:
Keywords:
Depends on:
Blocks: 59670
  Show dependency treegraph
 
Reported: 2011-11-13 19:15 PST by noel gordon
Modified: 2011-11-14 17:13 PST (History)
2 users (show)

See Also:


Attachments
Patch (6.46 KB, patch)
2011-11-13 19:16 PST, noel gordon
no flags Details | Formatted Diff | Diff
Patch (6.50 KB, patch)
2011-11-14 03:35 PST, noel gordon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description noel gordon 2011-11-13 19:15:35 PST
Test JPEG image decoding RGB pixel endianess
Comment 1 noel gordon 2011-11-13 19:16:32 PST
Created attachment 114872 [details]
Patch
Comment 2 Kent Tamura 2011-11-14 01:23:15 PST
Comment on attachment 114872 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114872&action=review

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:9
> +<img src="resources/rgb-jpeg-red.jpg"   onload="test(this, [255,0,0,255])">
> +<img src="resources/rgb-jpeg-green.jpg" onload="test(this, [0,255,0,255])">
> +<img src="resources/rgb-jpeg-blue.jpg"  onload="test(this, [0,0,255,255])">
> +
> +<pre id="log">PASS</pre>

Because there are three tests, we had better show three 'PASS' in the test result.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:13
> +    layoutTestController.waitUntilDone(), layoutTestController.dumpAsText();

No reason to use a comma operator here.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:24
> +        r += data[i + 0], g += data[i + 1], b += data[i + 2], a += data[i + 3];

ditto.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:26
> +    return [ r / size, g / size, b / size, a / size ];

nit: spaces after ']' and before ']' are not needed.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:36
> +    if (tolerance = tolerance || 0, delta > tolerance)

The tolerance argument is always specified.  We don't need "tolerance = tolerance || 0".
Also, there is no reason to use a comma operator here.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:60
> +        if (testImage(image, expect), ++loadedImages < 3)

No reason to use a comma operator here.
Comment 3 noel gordon 2011-11-14 03:35:45 PST
Created attachment 114910 [details]
Patch
Comment 4 noel gordon 2011-11-14 03:38:23 PST
(In reply to comment #2)
> (From update of attachment 114872 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=114872&action=review
> 
> > LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:9
> > +<img src="resources/rgb-jpeg-red.jpg"   onload="test(this, [255,0,0,255])">
> > +<img src="resources/rgb-jpeg-green.jpg" onload="test(this, [0,255,0,255])">
> > +<img src="resources/rgb-jpeg-blue.jpg"  onload="test(this, [0,0,255,255])">
> > +
> > +<pre id="log">PASS</pre>
> 
> Because there are three tests, we had better show three 'PASS' in the test result.

We shown up to 3 failures should they happen, sufficient? 
 
> > LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:13
> > +    layoutTestController.waitUntilDone(), layoutTestController.dumpAsText();
> 
> No reason to use a comma operator here.

One day layoutTestController will return its |this| context.  Removed.
 
> > LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:24
> > +        r += data[i + 0], g += data[i + 1], b += data[i + 2], a += data[i + 3];
> 
> ditto.

All done, and the others you mentioned.
Comment 5 Kent Tamura 2011-11-14 16:54:37 PST
Comment on attachment 114910 [details]
Patch

ok
Comment 6 noel gordon 2011-11-14 16:55:51 PST
Thankyou Tamura-san
Comment 7 WebKit Review Bot 2011-11-14 17:13:41 PST
Comment on attachment 114910 [details]
Patch

Clearing flags on attachment: 114910

Committed r100220: <http://trac.webkit.org/changeset/100220>
Comment 8 WebKit Review Bot 2011-11-14 17:13:46 PST
All reviewed patches have been landed.  Closing bug.