[CSS Shapes] Add parsing support for gradients

[CSS Shapes] Add parsing support for gradients

Added support for shape-outside gradient values.

This is a merge of http://trac.webkit.org/changeset/167518 by Hans Muller <hmuller@adobe.com>;.

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

[rwlbuis, 2014-04-21 17:58:38]

It turns out the (WebKit) expectation test places the first green rect one pixel
more to the right. Could this be subpixel layout/gradient calculation related?

[Hans Muller, 2014-04-22 15:32:53]

On 2014/04/21 17:58:38, rwlbuis wrote:
> It turns out the (WebKit) expectation test places the first green rect one
pixel
> more to the right. Could this be subpixel layout/gradient calculation related?

Predicting exactly where the 50% linear gradient threshold will land is tricky
however the Blink/WebKit scan conversion code (Shape::createRasterShape())
should produce the same results. I assume that the source o the difference is in
the Blink/WebKit gradient generation process.  To check that you'd have to
compare gradient images. Sorry.

[rwlbuis, 2014-04-22 19:23:06]

On 2014/04/22 15:32:53, Hans Muller wrote:
> On 2014/04/21 17:58:38, rwlbuis wrote:
> > It turns out the (WebKit) expectation test places the first green rect one
> pixel
> > more to the right. Could this be subpixel layout/gradient calculation
related?
> 
> Predicting exactly where the 50% linear gradient threshold will land is tricky
> however the Blink/WebKit scan conversion code (Shape::createRasterShape())
> should produce the same results. I assume that the source o the difference is
in
> the Blink/WebKit gradient generation process.  To check that you'd have to
> compare gradient images. Sorry.

Thanks for the pointers! Here is what I did. I changed the test to use the
gradient with a background instead of shape-outside
in order to get a clear visual representation. I ran the test in WebkitGTK and
Blink and used ImageMagick to get the diff as png. I'll
add the png diff to the bug. Describing it, there are some differences in the
50% to 100% region. To work around the bug I noticed that
a threshold of 0.75 works as it places the box 25% to the left (and only
starting from 50% is the gradient different). I am not sure
at this point if this problem should hold up this bug, and if the difference
requires a fix. Any ideas?

[rwlbuis, 2014-04-22 20:08:20]

On 2014/04/22 15:32:53, Hans Muller wrote:
> On 2014/04/21 17:58:38, rwlbuis wrote:
> > It turns out the (WebKit) expectation test places the first green rect one
> pixel
> > more to the right. Could this be subpixel layout/gradient calculation
related?
> 
> Predicting exactly where the 50% linear gradient threshold will land is tricky
> however the Blink/WebKit scan conversion code (Shape::createRasterShape())
> should produce the same results. I assume that the source o the difference is
in
> the Blink/WebKit gradient generation process.  To check that you'd have to
> compare gradient images. Sorry.

Looking back, I think you are right. I am looking at
GradientGeneratedImage::draw and see
it will use a gc for the gradient drawing (fillRect). AFAIK on Blink this will
always end
up using Skia, on WebKit somethign else (cairo on GTK?). In other words
differences
here will probably be unavoidable. So it seems to me we have to accept this and
change
the test to work for Blink (maybe slightly different threshold with a comment to
explain why).

[Hans Muller, 2014-04-24 15:39:46]

On 2014/04/22 20:08:20, rwlbuis wrote:
> On 2014/04/22 15:32:53, Hans Muller wrote:
> > On 2014/04/21 17:58:38, rwlbuis wrote:
> > > It turns out the (WebKit) expectation test places the first green rect one
> > pixel
> > > more to the right. Could this be subpixel layout/gradient calculation
> related?
> > 
> > Predicting exactly where the 50% linear gradient threshold will land is
tricky
> > however the Blink/WebKit scan conversion code (Shape::createRasterShape())
> > should produce the same results. I assume that the source o the difference
is
> in
> > the Blink/WebKit gradient generation process.  To check that you'd have to
> > compare gradient images. Sorry.
> 
> Looking back, I think you are right. I am looking at
> GradientGeneratedImage::draw and see
> it will use a gc for the gradient drawing (fillRect). AFAIK on Blink this will
> always end up using Skia, on WebKit somethign else (cairo on GTK?). In other
words
> differences  here will probably be unavoidable. So it seems to me we have to
accept this and
> change the test to work for Blink (maybe slightly different threshold with a
comment to
> explain why).

Sounds OK to me.

The rest of the patch looks fine.

[rwlbuis, 2014-04-24 16:32:09]

On 2014/04/24 15:39:46, Hans Muller wrote:
> On 2014/04/22 20:08:20, rwlbuis wrote:
> > On 2014/04/22 15:32:53, Hans Muller wrote:
> > > On 2014/04/21 17:58:38, rwlbuis wrote:
> > > > It turns out the (WebKit) expectation test places the first green rect
one
> > > pixel
> > > > more to the right. Could this be subpixel layout/gradient calculation
> > related?
> > > 
> > > Predicting exactly where the 50% linear gradient threshold will land is
> tricky
> > > however the Blink/WebKit scan conversion code (Shape::createRasterShape())
> > > should produce the same results. I assume that the source o the difference
> is
> > in
> > > the Blink/WebKit gradient generation process.  To check that you'd have to
> > > compare gradient images. Sorry.
> > 
> > Looking back, I think you are right. I am looking at
> > GradientGeneratedImage::draw and see
> > it will use a gc for the gradient drawing (fillRect). AFAIK on Blink this
will
> > always end up using Skia, on WebKit somethign else (cairo on GTK?). In other
> words
> > differences  here will probably be unavoidable. So it seems to me we have to
> accept this and
> > change the test to work for Blink (maybe slightly different threshold with a
> comment to
> > explain why).
> 
> Sounds OK to me.
> 
> The rest of the patch looks fine.

Thanks! I updated the test in the patch.

[Hans Muller, 2014-04-24 18:08:08]

On 2014/04/24 16:32:09, rwlbuis wrote:
> On 2014/04/24 15:39:46, Hans Muller wrote:
> > On 2014/04/22 20:08:20, rwlbuis wrote:
> > > On 2014/04/22 15:32:53, Hans Muller wrote:
> > > > On 2014/04/21 17:58:38, rwlbuis wrote:
> > > > > It turns out the (WebKit) expectation test places the first green rect
> one
> > > > pixel
> > > > > more to the right. Could this be subpixel layout/gradient calculation
> > > related?
> > > > 
> > > > Predicting exactly where the 50% linear gradient threshold will land is
> > tricky
> > > > however the Blink/WebKit scan conversion code
(Shape::createRasterShape())
> > > > should produce the same results. I assume that the source o the
difference
> > is
> > > in
> > > > the Blink/WebKit gradient generation process.  To check that you'd have
to
> > > > compare gradient images. Sorry.
> > > 
> > > Looking back, I think you are right. I am looking at
> > > GradientGeneratedImage::draw and see
> > > it will use a gc for the gradient drawing (fillRect). AFAIK on Blink this
> will
> > > always end up using Skia, on WebKit somethign else (cairo on GTK?). In
other
> > words
> > > differences  here will probably be unavoidable. So it seems to me we have
to
> > accept this and
> > > change the test to work for Blink (maybe slightly different threshold with
a
> > comment to
> > > explain why).
> > 
> > Sounds OK to me.
> > 
> > The rest of the patch looks fine.
> 
> Thanks! I updated the test in the patch.

This patch looks good to me.

[rwlbuis, 2014-04-24 18:42:22]

PTAL. Last one before we can close BUG 322165!

[leviw_travelin_and_unemployed, 2014-04-28 17:33:55]

LGTM as well :)

[rwlbuis, 2014-04-28 17:34:30]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-04-28 17:34:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/244693003/20001

[commit-bot: I haz the power, 2014-04-28 18:09:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-28 18:09:35]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[rwlbuis, 2014-04-28 18:14:01]

The CQ bit was checked by rob.buis@samsung.com

[commit-bot: I haz the power, 2014-04-28 18:14:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rob.buis@samsung.com/244693003/20001

[commit-bot: I haz the power, 2014-04-28 19:25:18]

Change committed as 172790