Bug 88218 – [chromium] Use WEBPImportPictureRGBX|BGRX to import picture data

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[noel gordon]

Part of a two-sided patch: https://chromiumcodereview.appspot.com/10496016 defines WEBPImportPictureRGBX|BGRX for importing picture data ignoring the alpha channel.  WebKit should use these methods to import data for encoding when creating a image/webp dataURL.  The methods WebKit currently uses, namely WEBPImportPictureRGBA|BGRA, will include the alpha channel during import in a future release of libwebp so we should stop using these methods to future-proof the code.

[noel gordon]

Created attachment 145546 [details]
Patch

[noel gordon]

Waiting on DEPS http://src.chromium.org/viewvc/chrome?view=rev&revision=140264 to roll into webkit.

[Pascal Massimino]

lgtm, but one need to wait for the new functions to be in the official libwebp tree (release 0.1.4?) before using them...

btw: there's a missing checksum reported for the second PNG

[noel gordon]

(In reply to comment #3)
> lgtm, but one need to wait for the new functions to be in the official libwebp tree (release 0.1.4?) before using them...

Actually, I just need chromium DEPS to rev 140264 to use the functions since they are in the chromium repository.
 
> btw: there's a missing checksum reported for the second PNG

I generated the png result with run-webkit-tests, that should create a checksum, and the error report says "create the png with run-webkit-tests" :)

Dunno what's going on there.

[noel gordon]

http://trac.webkit.org/changeset/119468 chromium DEPS rolled.

[noel gordon]

Created attachment 145918 [details]
Patch

[noel gordon]

Since the linux EWS are backed up, built chromium-linux and ran the new test.  Works fines.

[Kent Tamura]

Comment on attachment 145918 [details]
Patch

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

> LayoutTests/ChangeLog:13
> +        * fast/canvas/canvas-toDataURL-webp-no-alpha-expected.txt: Added.
> +        * fast/canvas/canvas-toDataURL-webp-no-alpha.html: Added.

Can you make this a reftest?

[noel gordon]

Maybe, prefer to see the composite onto black pixels here.

[noel gordon]

Discussed the existing test fast/canvas/canvas-toDataURL-webp.html with tkent, and we agree to replace it with the new test.

[noel gordon]

Created attachment 146291 [details]
Patch

[Kent Tamura]

Comment on attachment 146291 [details]
Patch

ok

[WebKit Review Bot]

Comment on attachment 146291 [details]
Patch

Rejecting attachment 146291 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12908507

[noel gordon]

Created attachment 146437 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 146437 [details]
Patch for landing

Clearing flags on attachment: 146437

Committed r119787: <http://trac.webkit.org/changeset/119787>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[noel gordon]

(In reply to comment #4)
>
> > btw: there's a missing checksum reported for the second PNG
> 
> I generated the png result with run-webkit-tests, that should create a checksum, and the error report says "create the png with run-webkit-tests" :)
> 
> Dunno what's going on there.

Bug 88368.

[noel gordon]

Added rebaselines on http://trac.webkit.org/changeset/119801

[Ojan Vafai]

FYI, as per garden-o-matic, the MISSING line in TestExpectations needs to include Lion and you need a line for the equivalent virtual gpu test platform/chromium/virtual/gpu/fast/canvas/canvas-toDataURL-webp.html.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcanvas%2Fcanvas-toDataURL-webp.html

[noel gordon]

http://trac.webkit.org/changeset/119804 for the virtual GPU.

[noel gordon]

(In reply to comment #19)
> FYI, as per garden-o-matic, the MISSING line in TestExpectations needs to include Lion

Want lion: so optimize http://trac.webkit.org/changeset/119812

[noel gordon]

(In reply to comment #19)
> http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcanvas%2Fcanvas-toDataURL-webp.html

All look good now, thanks for fyi.