Add ColorspaceConversion in ImageBitmapOptions

Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. Layout tests are added to ensure the correctness.

BUG=592005
Committed: https://crrev.com/1e71ee33752db1b8817870cc25324ae40dbcd6d3
Cr-Commit-Position: refs/heads/master@{#381224}

[xidachen, 2016-03-04 18:49:18]

Description was changed from

==========
WIP: Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option.

BUG=
==========

to

==========
WIP: Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option.

BUG=592005
==========

[xidachen, 2016-03-04 18:49:37]

Description was changed from

==========
WIP: Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option.

BUG=592005
==========

to

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob.

BUG=592005
==========

[xidachen, 2016-03-04 19:00:30]

Description was changed from

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob.

BUG=592005
==========

to

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob.

The testing for this case will be on KhronosGroup/WebGL. In particular,
there is a .png file with colorspin-profile. Locally I have tried to use
an existing WebGL conformance test to load this image file, and set
the gl.UNPACK_COLORSPACE_CONVERSION_WEBGL parameter to be NONE, and I
observe the color to be blue. With the parameter set to BROWSER_DEFAULT_WEBGL,
the observed color is red using chromium.

With this CL, I used createImageBitmap(Blob, colorspaceConversion="none")
and the observed color is blue, while it becomes red when the second
parameter is set to be "default". This result indicates that the
change of behavior after this CL is consistent with the existing tests.

BUG=592005
==========

[xidachen, 2016-03-08 18:45:02]

Description was changed from

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob.

The testing for this case will be on KhronosGroup/WebGL. In particular,
there is a .png file with colorspin-profile. Locally I have tried to use
an existing WebGL conformance test to load this image file, and set
the gl.UNPACK_COLORSPACE_CONVERSION_WEBGL parameter to be NONE, and I
observe the color to be blue. With the parameter set to BROWSER_DEFAULT_WEBGL,
the observed color is red using chromium.

With this CL, I used createImageBitmap(Blob, colorspaceConversion="none")
and the observed color is blue, while it becomes red when the second
parameter is set to be "default". This result indicates that the
change of behavior after this CL is consistent with the existing tests.

BUG=592005
==========

to

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. A layout test is also added to ensure the correctness.


BUG=592005
==========

[xidachen, 2016-03-08 20:53:16]

Description was changed from

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. A layout test is also added to ensure the correctness.


BUG=592005
==========

to

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. Layout tests are added to ensure the correctness.


BUG=592005
==========

[xidachen, 2016-03-09 12:05:06]

xidachen@chromium.org changed reviewers:
+ junov@chromium.org

[xidachen, 2016-03-09 12:05:07]

PTAL.

[Justin Novosad, 2016-03-09 16:29:39]

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js
(right):

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js:102:
{ sub: true },
The structure of this test is getting messy as it grows. This test would be a
lot clearer if we just used 'cases' for the expansion.

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js:116:
runOneIteration(cases[i].sub, bindingTarget, program, bitmaps.colorspaceDefault,
false, true, retVal, 'default');
What is the difference between colorspaceDef and colorspaceDefault?

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js:118:
runOneIteration(cases[i].sub, bindingTarget, program, bitmaps.colorspaceDef,
false, true, retVal);
How is this different from the line with bitmaps.defaultOption above?

[xidachen, 2016-03-14 13:29:42]

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js
(right):

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js:102:
{ sub: true },
On 2016/03/09 16:29:39, Justin Novosad wrote:
> The structure of this test is getting messy as it grows. This test would be a
> lot clearer if we just used 'cases' for the expansion.

This is now cleaned up, everything is put in cases.

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js:116:
runOneIteration(cases[i].sub, bindingTarget, program, bitmaps.colorspaceDefault,
false, true, retVal, 'default');
On 2016/03/09 16:29:39, Justin Novosad wrote:
> What is the difference between colorspaceDef and colorspaceDefault?

colorspaceDef is created with out given any colorspace options.
colorspaceDefault is created with { colorspaceConversion: 'default' }. The
created ImageBitmap should be the same when using chromium, and this is the
testing to make sure that they are the same.

https://codereview.chromium.org/1763043002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/canvas/webgl/resources/tex-image-and-sub-image-image-bitmap-utils.js:118:
runOneIteration(cases[i].sub, bindingTarget, program, bitmaps.colorspaceDef,
false, true, retVal);
On 2016/03/09 16:29:39, Justin Novosad wrote:
> How is this different from the line with bitmaps.defaultOption above?

bitmaps.colorspaceDef is created with a png that has color-spin profile.
bitmaps.defaultOption is created with another png that is 2*2 with red on top
and green on bottom.

[Justin Novosad, 2016-03-14 15:12:51]

> On 2016/03/09 16:29:39, Justin Novosad wrote:
> > What is the difference between colorspaceDef and colorspaceDefault?
> 
> colorspaceDef is created with out given any colorspace options.
> colorspaceDefault is created with { colorspaceConversion: 'default' }. The
> created ImageBitmap should be the same when using chromium, and this is the
> testing to make sure that they are the same.
> 

I do not follow.
What I see is "colorspace: retVal.colorspaceEffect ? 'default' : 'empty'" in
both cases.

[xidachen, 2016-03-14 15:32:34]

On 2016/03/14 15:12:51, Justin Novosad wrote:
> > On 2016/03/09 16:29:39, Justin Novosad wrote:
> > > What is the difference between colorspaceDef and colorspaceDefault?
> > 
> > colorspaceDef is created with out given any colorspace options.
> > colorspaceDefault is created with { colorspaceConversion: 'default' }. The
> > created ImageBitmap should be the same when using chromium, and this is the
> > testing to make sure that they are the same.
> > 
> 
> I do not follow.
> What I see is "colorspace: retVal.colorspaceEffect ? 'default' : 'empty'" in
> both cases.

Sorry that I totally missed this point.

I made some changes and now colorspace can have 4 options: 'default', 'none',
'empty', 'notprovided'. 'default' and 'none' is for the cases where
colorspaceConversion option is given. 'empty' is for the cases of loading an
image without any colorspace profile. 'notprovided' is for the cases of loading
an image with colorspace profile but the colorspaceConversion option is not
provided. Using chromium, the option 'notprovided' would have the same test
result as 'default'.

[Justin Novosad, 2016-03-14 15:46:54]

On 2016/03/14 15:32:34, xidachen wrote:
> On 2016/03/14 15:12:51, Justin Novosad wrote:
> > > On 2016/03/09 16:29:39, Justin Novosad wrote:
> > > > What is the difference between colorspaceDef and colorspaceDefault?
> > > 
> > > colorspaceDef is created with out given any colorspace options.
> > > colorspaceDefault is created with { colorspaceConversion: 'default' }. The
> > > created ImageBitmap should be the same when using chromium, and this is
the
> > > testing to make sure that they are the same.
> > > 
> > 
> > I do not follow.
> > What I see is "colorspace: retVal.colorspaceEffect ? 'default' : 'empty'" in
> > both cases.
> 
> Sorry that I totally missed this point.
> 
> I made some changes and now colorspace can have 4 options: 'default', 'none',
> 'empty', 'notprovided'. 'default' and 'none' is for the cases where
> colorspaceConversion option is given. 'empty' is for the cases of loading an
> image without any colorspace profile. 'notprovided' is for the cases of
loading
> an image with colorspace profile but the colorspaceConversion option is not
> provided. Using chromium, the option 'notprovided' would have the same test
> result as 'default'.

lgtm

[xidachen, 2016-03-14 15:49:00]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-14 15:49:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1763043002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1763043002/120001

[xidachen, 2016-03-14 17:09:25]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2016-03-15 13:25:29]

The CQ bit was checked by xidachen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-15 13:25:44]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1763043002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1763043002/180001

[xidachen, 2016-03-15 14:39:00]

The CQ bit was unchecked by xidachen@chromium.org

[xidachen, 2016-03-15 14:43:49]

The CQ bit was checked by xidachen@chromium.org

[xidachen, 2016-03-15 14:43:50]

The patchset sent to the CQ was uploaded after l-g-t-m from junov@chromium.org
Link to the patchset: https://codereview.chromium.org/1763043002/#ps180001
(title: "change tolerance to 13")

[commit-bot: I haz the power, 2016-03-15 14:44:00]

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

[commit-bot: I haz the power, 2016-03-15 14:51:31]

Description was changed from

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. Layout tests are added to ensure the correctness.


BUG=592005
==========

to

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. Layout tests are added to ensure the correctness.


BUG=592005
==========

[commit-bot: I haz the power, 2016-03-15 14:51:32]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-03-15 14:53:11]

Description was changed from

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. Layout tests are added to ensure the correctness.


BUG=592005
==========

to

==========
Add ColorspaceConversion in ImageBitmapOptions

This option is specifically for createImageBitmap from a Blob or a
HTMLImageElement. This CL adds support for this option, starting with
Blob. Layout tests are added to ensure the correctness.

BUG=592005

Committed: https://crrev.com/1e71ee33752db1b8817870cc25324ae40dbcd6d3
Cr-Commit-Position: refs/heads/master@{#381224}
==========

[commit-bot: I haz the power, 2016-03-15 14:53:12]

Patchset 10 (id:??) landed as
https://crrev.com/1e71ee33752db1b8817870cc25324ae40dbcd6d3
Cr-Commit-Position: refs/heads/master@{#381224}