[Android] ImageBuffer toDataURL("image/webp") has RGBA pixel input

[Android] ImageBuffer toDataURL("image/webp") has RGBA pixel input

On Android, a premultiplied ImageBuffer source has RGBA pixels. Import
its bitmap for encoding in that form, rather than BGRA.

Covered by fast/canvas/canvas-toDataURL-webp.html

BUG=421454
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183712

[Noel Gordon, 2014-10-11 10:14:04]

Patchset #1 (id:1) has been deleted

[Noel Gordon, 2014-10-11 11:01:43]

noel@chromium.org changed reviewers:
+ tkent@chromium.org

[skal, 2014-10-11 13:53:53]

skal@google.com changed reviewers:
+ skal@google.com

[skal, 2014-10-11 13:53:54]

https://codereview.chromium.org/645333002/diff/50001/Source/platform/image-en...
File Source/platform/image-encoders/skia/WEBPImageEncoder.cpp (right):

https://codereview.chromium.org/645333002/diff/50001/Source/platform/image-en...
Source/platform/image-encoders/skia/WEBPImageEncoder.cpp:110: if (!premultiplied
&& !importPictureRGBX<false>(pixels, &picture))
the logic is hard to follow as written. Would be better with:

if (premultiplied) {
#if SK_B32_SHIFT // Little-endian RGBA pixels. (Android)
     if (!importPictureRGBX<true>(pixels, &picture))
          return false;
#else
     if (!importPictureBGRX<true>(pixels, &picture))
          return false;
#endif
} else {
     if (!importPictureRGBX<false>(pixels, &picture))
          return false;
}


btw: won't we need an "#ifdef SK_B32_SHIFT" for the latter case too?

[Noel Gordon, 2014-10-14 16:50:20]

On 2014/10/11 13:53:54, skal wrote:
>
https://codereview.chromium.org/645333002/diff/50001/Source/platform/image-en...
> File Source/platform/image-encoders/skia/WEBPImageEncoder.cpp (right):
> 
>
https://codereview.chromium.org/645333002/diff/50001/Source/platform/image-en...
> Source/platform/image-encoders/skia/WEBPImageEncoder.cpp:110: if
(!premultiplied
> && !importPictureRGBX<false>(pixels, &picture))
> the logic is hard to follow as written. Would be better with:
> 
> if (premultiplied) {
> #if SK_B32_SHIFT // Little-endian RGBA pixels. (Android)
>      if (!importPictureRGBX<true>(pixels, &picture))
>           return false;
> #else
>      if (!importPictureBGRX<true>(pixels, &picture))
>           return false;
> #endif
> } else {
>      if (!importPictureRGBX<false>(pixels, &picture))
>           return false;
> }

Lemme fiddle a little and see if I can pretty it some ...
 
> btw: won't we need an "#ifdef SK_B32_SHIFT" for the latter case too?

Good question.  We had to make this fix for JPEG and only needed to change the
premultiplied case, see https://bugs.webkit.org/show_bug.cgi?id=78540 

Try the test case on an Android device: test page should produce blue boxes.

https://philip.html5.org/tests/canvas/suite/tests/index.toDataURL.jpeg.qualit...

[Noel Gordon, 2014-10-14 17:32:24]

On 2014/10/14 16:50:20, Noel Gordon wrote:

> Lemme fiddle a little and see if I can pretty it some ...

Ditched the #ifdefs.

[Noel Gordon, 2014-10-14 17:35:44]

noel@chromium.org changed reviewers:
+ kbr@chromium.org
- skal@google.com, tkent@chromium.org

[urvang (Google), 2014-10-14 17:50:25]

urvang@google.com changed reviewers:
+ urvang@google.com

[urvang (Google), 2014-10-14 17:50:26]

lgtm

[Ken Russell (switch to Gerrit), 2014-10-14 19:07:57]

LGTM FWIW.

[Noel Gordon, 2014-10-14 23:22:20]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2014-10-14 23:22:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/645333002/180001

[Noel Gordon, 2014-10-14 23:44:04]

The CQ bit was unchecked by noel@chromium.org

[Noel Gordon, 2014-10-14 23:45:56]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2014-10-14 23:46:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/645333002/200001

[commit-bot: I haz the power, 2014-10-15 00:50:43]

Committed patchset #3 (id:200001) as 183712

[Noel Gordon, 2014-10-15 01:08:31]

My dear CQ, win-blink_rel did in fact pass
 
http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/31807
<eom>