Bug 51982 – Shadow is not drawn when filling a path with a gradient

• Component: Canvas
• Status: RESOLVED FIXED

[Helder Correia]

This happens in CG and seems to be the very same issue as in bug 51869, this time related to GraphicsContext::fillPath(const Path& path). I'll upload a patch soon.

[Helder Correia]

Created attachment 78288 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 78288 [details]
Patch

Clearing flags on attachment: 78288

Committed r75341: <http://trac.webkit.org/changeset/75341>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 78288 [details]:

fast/workers/storage/use-same-database-in-page-and-workers.html bug 50995 (author: [e-mail])
The commit-queue is continuing to process your patch.

[Jeremy Orlow]

This was rolled out by https://bugs.webkit.org/show_bug.cgi?id=52157 due to this:

The mac results on http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fbackgrounds%2Fsvg-as-background-2.html&showExpectations=true&master=ChromiumWebkit seem to demonstrate a real test failure.  There may have been others as well.  You can see where things broke here: http://build.chromium.org/p/chromium.webkit/waterfall?builder=Webkit%20Mac&last_time=1294549999

[Jeremy Orlow]

Looks like the revert did indeed fix the problem.  Sorry to be the bearer of bad news!

[Helder Correia]

(In reply to comment #6)
> Looks like the revert did indeed fix the problem.

Is any of the SVG folks able to help me figure out how this patch may have broken some (gradient?) functionality? Thanks

[Dirk Schulze]

(In reply to comment #7)
> (In reply to comment #6)
> > Looks like the revert did indeed fix the problem.
> 
> Is any of the SVG folks able to help me figure out how this patch may have broken some (gradient?) functionality? Thanks

Pretty easy, on the old code you apply the path and transform the CTM afterwards. With the patch you're doing it the other way around. This can't work. Change the order for lines 596/570 with 572/573 and it will work again. I bet you get the same problems for shadowed paths. So you have to do it there as well.

[Helder Correia]

Created attachment 79008 [details]
Seems to be working work now. Thanks, Dirk.

[Simon Fraser (smfr)]

Comment on attachment 79008 [details]
Seems to be working work now. Thanks, Dirk.

Do your previous GraphicsContext::fillRect() changes have the same issue?

[Helder Correia]

(In reply to comment #10)
> (From update of attachment 79008 [details])
> Do your previous GraphicsContext::fillRect() changes have the same issue?

I don't think so.

[Helder Correia]

(In reply to comment #11)
> (In reply to comment #10)
> > (From update of attachment 79008 [details] [details])
> > Do your previous GraphicsContext::fillRect() changes have the same issue?
> 
> I don't think so.

Unless I should be calling CGContextConcatCTM() after CGContextClipToRect().

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 79008 [details]:

http/tests/xmlhttprequest/basic-auth.html bug 51613 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 79008 [details]
Seems to be working work now. Thanks, Dirk.

Clearing flags on attachment: 79008

Committed r75833: <http://trac.webkit.org/changeset/75833>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

In GraphicsContext::fillRect(), don't you need to call CGContextConcatCTM(context, m_state.fillGradient->gradientSpaceTransform()); after setting up the layer's context?

[Dirk Schulze]

Helder, can you point me to the other changes you made, that Simon mentioned? We have many failing tests on SVG (pixel tests fail). As far as I can see problems with filling and stroking tests with gradients. Because we don't see this failures in DRT, it must be problems with the drawing engine.

[Helder Correia]

(In reply to comment #17)
> Helder, can you point me to the other changes you made, that Simon mentioned? We have many failing tests on SVG (pixel tests fail). As far as I can see problems with filling and stroking tests with gradients. Because we don't see this failures in DRT, it must be problems with the drawing engine.

Dirk, there's only r75139 and bug 52509 is waiting for a review. Let me know how I can help you further.

[Dirk Schulze]

(In reply to comment #18)
> (In reply to comment #17)
> > Helder, can you point me to the other changes you made, that Simon mentioned? We have many failing tests on SVG (pixel tests fail). As far as I can see problems with filling and stroking tests with gradients. Because we don't see this failures in DRT, it must be problems with the drawing engine.
> 
> Dirk, there's only r75139 and bug 52509 is waiting for a review. Let me know how I can help you further.

The change on GraphicsContextCg in r75139 looks bad. You changed the order of clipping and concatCTM. Looks like this caused the problem. I'll try it locally...