Bug 52531

Summary:

REGRESSION(75139): SVG gradients are not applied to texts

Product:

WebKit

Reporter:

Dirk Schulze <krit>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, helder, simon.fraser, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

52724

Bug Blocks:

51869, 52640

Attachments:

Description	Flags
Patch	none
context -> layerContext. This fixes the SVG text gradient fill.	none
Translate layerContext CTM - make pixel test succeed again	none

Dirk Schulze

Reported 2011-01-16 03:00:07 PST

The change on GraphcisContextCG in http://trac.webkit.org/changeset/75139 caused a problem with filling and stroking texts with gradients in SVG. The order of concatCTM and clipRect was mixed up.

Attachments
Patch (111.33 KB, patch) 2011-01-18 01:27 PST, Helder Correia	no flags	Details Formatted Diff Diff
context -> layerContext. This fixes the SVG text gradient fill. (1.86 KB, patch) 2011-01-24 23:16 PST, Helder Correia	no flags	Details Formatted Diff Diff
Translate layerContext CTM - make pixel test succeed again (2.08 KB, patch) 2011-01-24 23:35 PST, Helder Correia	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Helder Correia

Comment 1 2011-01-18 01:27:01 PST

Created attachment 79253 [details] Patch

Dirk Schulze

Comment 2 2011-01-18 01:32:37 PST

Comment on attachment 79253 [details] Patch Hm. the question is, why is the shadow not black? Do you see similar behaviors on Canvas rect with shadow? Or Canvas texts and shadow? (filled with a gradient of course).

Helder Correia

Comment 3 2011-01-18 01:59:30 PST

(In reply to comment #2) > (From update of attachment 79253 [details]) > Hm. the question is, why is the shadow not black? Do you see similar behaviors on Canvas rect with shadow? Or Canvas texts and shadow? (filled with a gradient of course). Looks like I asked the same thing at the same time on https://bugs.webkit.org/show_bug.cgi?id=51869#c24 :) On canvas, the shadow is black for rect and text (or whatever color is set to it). The non-black SVG text-shadow is the behavior I get if I go back some revisions. Same in Google Chrome Dev Channel on Mac. So, this is not something that was broken recently.

Dirk Schulze

Comment 4 2011-01-18 02:33:36 PST

Comment on attachment 79253 [details] Patch I trust you, if you say it was wrong before you committed your patch. r=me.

Nikolas Zimmermann

Comment 5 2011-01-18 07:28:42 PST

I tested this patch on Leopard, it fixes _most_ issues. But svg/css/composite-shadow-text.svg remains broken, the first gradient doesn't show up. Gradient on text remains broken, when a shadow is also applied to the target. <rect fill="url(#someGradient" style="-webkit-svg-shadow: ..../">

WebKit Commit Bot

Comment 6 2011-01-18 09:19:34 PST

Comment on attachment 79253 [details] Patch Clearing flags on attachment: 79253 Committed r76029: <http://trac.webkit.org/changeset/76029>

WebKit Commit Bot

Comment 7 2011-01-18 09:19:39 PST

All reviewed patches have been landed. Closing bug.

Dirk Schulze

Comment 8 2011-01-18 10:10:19 PST

(In reply to comment #5) > I tested this patch on Leopard, it fixes _most_ issues. > But svg/css/composite-shadow-text.svg remains broken, the first gradient doesn't show up. > > Gradient on text remains broken, when a shadow is also applied to the target. > <rect fill="url(#someGradient" style="-webkit-svg-shadow: ..../"> I feared it. I have hoped that SVG Text + text-shadow covers it :-( Niko, can you check if removing the if and it's content makes it run again?

Simon Fraser (smfr)

Comment 9 2011-01-18 10:24:47 PST

(In reply to comment #3) > On canvas, the shadow is black for rect and text (or whatever color is set to it). The non-black SVG text-shadow is the behavior I get if I go back some revisions. Same in Google Chrome Dev Channel on Mac. So, this is not something that was broken recently. But when was it broken?

Dirk Schulze

Comment 10 2011-01-18 10:40:47 PST

(In reply to comment #9) > (In reply to comment #3) > > > On canvas, the shadow is black for rect and text (or whatever color is set to it). The non-black SVG text-shadow is the behavior I get if I go back some revisions. Same in Google Chrome Dev Channel on Mac. So, this is not something that was broken recently. > > But when was it broken? We never had test cases for SVG texts, filled with a gradient together with a text shadow. All test cases were with solid colors and they still run. It's likely that it never worked. Needs to be checked.

Nikolas Zimmermann

Comment 11 2011-01-19 01:46:10 PST

(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #3) > > > > > On canvas, the shadow is black for rect and text (or whatever color is set to it). The non-black SVG text-shadow is the behavior I get if I go back some revisions. Same in Google Chrome Dev Channel on Mac. So, this is not something that was broken recently. > > > > But when was it broken? > > We never had test cases for SVG texts, filled with a gradient together with a text shadow. All test cases were with solid colors and they still run. It's likely that it never worked. Needs to be checked. Well we have svg/css/composite-shadow-text.svg, added recently which covers svg gradient + -webkit-svg-shadow, that is broken atm.

Dirk Schulze

Comment 12 2011-01-19 10:51:32 PST

Helder, do you plan to fix it, or should we roll out the patch?

Helder Correia

Comment 13 2011-01-19 17:36:14 PST

(In reply to comment #12) > Helder, do you plan to fix it, or should we roll out the patch? Of course, we need composite-shadow-text.svg to work again. I'll look at it asap. Any hints are more than welcome, since I've never touched SVG code and I'm new to CG.

Helder Correia

Comment 14 2011-01-24 23:16:32 PST

Created attachment 80025 [details] context -> layerContext. This fixes the SVG text gradient fill.

Helder Correia

Comment 15 2011-01-24 23:35:24 PST

Created attachment 80026 [details] Translate layerContext CTM - make pixel test succeed again

Dirk Schulze

Comment 16 2011-01-25 00:14:20 PST

Comment on attachment 80026 [details] Translate layerContext CTM - make pixel test succeed again Makes sense. r=me

Helder Correia

Comment 17 2011-01-25 10:33:27 PST

(In reply to comment #16) > (From update of attachment 80026 [details]) > Makes sense. r=me Thanks. Can you please reopen the bug (and possibly reapply R+) so that the patch gets sent to the CQ? Or maybe land it manually.

Dirk Schulze

Comment 18 2011-01-25 10:37:56 PST

reopening for landing the patch.

WebKit Commit Bot

Comment 19 2011-01-25 10:54:37 PST

Comment on attachment 80026 [details] Translate layerContext CTM - make pixel test succeed again Clearing flags on attachment: 80026 Committed r76612: <http://trac.webkit.org/changeset/76612>

WebKit Commit Bot

Comment 20 2011-01-25 10:54:43 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.