Bug 51982 - Shadow is not drawn when filling a path with a gradient
Summary: Shadow is not drawn when filling a path with a gradient
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 52157
Blocks:
  Show dependency treegraph
 
Reported: 2011-01-05 22:14 PST by Helder Correia
Modified: 2011-01-16 02:46 PST (History)
6 users (show)

See Also:


Attachments
Patch (12.60 KB, patch)
2011-01-07 16:42 PST, Helder Correia
no flags Details | Formatted Diff | Diff
Seems to be working work now. Thanks, Dirk. (12.68 KB, patch)
2011-01-14 15:05 PST, Helder Correia
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Helder Correia 2011-01-05 22:14:12 PST
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.
Comment 1 Helder Correia 2011-01-07 16:42:20 PST
Created attachment 78288 [details]
Patch
Comment 2 WebKit Commit Bot 2011-01-08 18:35:33 PST
Comment on attachment 78288 [details]
Patch

Clearing flags on attachment: 78288

Committed r75341: <http://trac.webkit.org/changeset/75341>
Comment 3 WebKit Commit Bot 2011-01-08 18:35:39 PST
All reviewed patches have been landed.  Closing bug.
Comment 4 WebKit Commit Bot 2011-01-08 18:53:56 PST
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: dumi@chromium.org)
The commit-queue is continuing to process your patch.
Comment 5 Jeremy Orlow 2011-01-10 11:17:50 PST
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
Comment 6 Jeremy Orlow 2011-01-10 12:10:25 PST
Looks like the revert did indeed fix the problem.  Sorry to be the bearer of bad news!
Comment 7 Helder Correia 2011-01-14 12:48:08 PST
(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
Comment 8 Dirk Schulze 2011-01-14 13:07:33 PST
(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.
Comment 9 Helder Correia 2011-01-14 15:05:47 PST
Created attachment 79008 [details]
Seems to be working work now. Thanks, Dirk.
Comment 10 Simon Fraser (smfr) 2011-01-14 15:10:18 PST
Comment on attachment 79008 [details]
Seems to be working work now. Thanks, Dirk.

Do your previous GraphicsContext::fillRect() changes have the same issue?
Comment 11 Helder Correia 2011-01-14 15:20:58 PST
(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.
Comment 12 Helder Correia 2011-01-14 15:27:42 PST
(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().
Comment 13 WebKit Commit Bot 2011-01-14 15:45:22 PST
The commit-queue encountered the following flaky tests while processing attachment 79008 [details]:

http/tests/xmlhttprequest/basic-auth.html bug 51613 (author: ap@webkit.org)
The commit-queue is continuing to process your patch.
Comment 14 WebKit Commit Bot 2011-01-14 15:47:12 PST
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>
Comment 15 WebKit Commit Bot 2011-01-14 15:47:20 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Simon Fraser (smfr) 2011-01-14 16:06:51 PST
In GraphicsContext::fillRect(), don't you need to call CGContextConcatCTM(context, m_state.fillGradient->gradientSpaceTransform()); after setting up the layer's context?
Comment 17 Dirk Schulze 2011-01-15 23:32:16 PST
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.
Comment 18 Helder Correia 2011-01-16 00:16:26 PST
(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.
Comment 19 Dirk Schulze 2011-01-16 02:46:21 PST
(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...