Bug 68366

Summary: Fix nonpremultiplied webgl toDataURL to jpeg
Product: WebKit Reporter: John Bauman <jbauman>
Component: WebGLAssignee: John Bauman <jbauman>
Status: RESOLVED FIXED    
Severity: Normal CC: hbono, kbr, mdelaney7, noel.gordon, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 68410    
Bug Blocks: 67402    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description John Bauman 2011-09-19 09:28:18 PDT
Fix nonpremultiplied webgl toDataURL to jpeg
Comment 1 John Bauman 2011-09-19 09:31:56 PDT
Created attachment 107873 [details]
Patch
Comment 2 Kenneth Russell 2011-09-19 13:36:11 PDT
Comment on attachment 107873 [details]
Patch

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

Looks good overall. Thanks for catching this. Please update the comment when landing. r=me

> LayoutTests/fast/canvas/webgl/premultiplyalpha-test.html:5
> + -->

There shouldn't be any copyright headers in the WebKit layout tests; please remove this one.
Comment 3 John Bauman 2011-09-19 14:06:26 PDT
Created attachment 107918 [details]
Patch
Comment 4 Kenneth Russell 2011-09-19 14:07:04 PDT
Comment on attachment 107918 [details]
Patch

Looks good.
Comment 5 WebKit Review Bot 2011-09-19 15:22:34 PDT
Comment on attachment 107918 [details]
Patch

Clearing flags on attachment: 107918

Committed r95482: <http://trac.webkit.org/changeset/95482>
Comment 6 WebKit Review Bot 2011-09-19 15:22:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Kenneth Russell 2011-09-19 17:25:46 PDT
This was rolled out due to breakage of the CG port. Reopening.
Comment 8 noel gordon 2011-09-19 19:36:28 PDT
Any mac-chrome breakage?  I believe bug 40147 affects the CG mac ports.
Comment 9 John Bauman 2011-09-19 19:38:55 PDT
Yeah, mac-chrome was broken. I'm currently working on a fix (which will hopefully fix bug 40147 as well).
Comment 10 noel gordon 2011-09-19 19:53:04 PDT
Thanks John.  For skia, maybe avoid the division by 255 at lines @96-98 with a loop body like ...

    unsigned char alpha = pixels[3];
    if (alpha != 255) {
        *output++ = SkMulDiv255Round(pixels[0], alpha);
        *output++ = SkMulDiv255Round(pixels[1], alpha);
        *output++ = SkMulDiv255Round(pixels[2], alpha);
    } else {
        *output++ = pixels[0];
        *output++ = pixels[1];
        *output++ = pixels[2];
    }
Comment 11 John Bauman 2011-09-21 18:17:16 PDT
Created attachment 108265 [details]
Patch
Comment 12 Kenneth Russell 2011-09-26 10:56:15 PDT
Comment on attachment 108265 [details]
Patch

Looks good to me. Nice work. Please keep an eye on the bots (since it seems the previous failure was only caught by the Chromium canaries) and once this settles down please take https://bugs.webkit.org/show_bug.cgi?id=40147 and close it as a duplicate of this one.
Comment 13 WebKit Review Bot 2011-09-26 15:06:40 PDT
Comment on attachment 108265 [details]
Patch

Clearing flags on attachment: 108265

Committed r96000: <http://trac.webkit.org/changeset/96000>
Comment 14 WebKit Review Bot 2011-09-26 15:06:45 PDT
All reviewed patches have been landed.  Closing bug.
Comment 15 John Bauman 2011-09-26 15:46:49 PDT
*** Bug 40147 has been marked as a duplicate of this bug. ***