Bug 33498 - REGRESSION: svg/css/circle-in-mask-with-shadow.svg failing pixel tests
Summary: REGRESSION: svg/css/circle-in-mask-with-shadow.svg failing pixel tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2010-01-11 16:02 PST by Beth Dakin
Modified: 2010-01-14 23:33 PST (History)
5 users (show)

See Also:


Attachments
Patch (47.21 KB, patch)
2010-01-14 16:33 PST, Beth Dakin
sam: review+
Details | Formatted Diff | Diff
proposed fix for Qt port (27.48 KB, patch)
2010-01-14 22:46 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2010-01-11 16:02:58 PST
The shadow is getting clipped away. This is probably <mask> related.
Comment 1 Beth Dakin 2010-01-14 16:33:22 PST
Created attachment 46620 [details]
Patch
Comment 2 WebKit Review Bot 2010-01-14 16:39:14 PST
Attachment 46620 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/rendering/style/SVGRenderStyle.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1
Comment 3 Beth Dakin 2010-01-14 16:42:05 PST
(In reply to comment #2)
> Attachment 46620 [details] did not pass style-queue:
> 
> Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
> WebCore/rendering/style/SVGRenderStyle.h:37:  Code inside a namespace should
> not be indented.  [whitespace/indent] [4]
> Total errors found: 1

I do not intend to fix this style problem with this patch since it would involve re-tabbing the entire header file, and it would make the actual changes to the file for the sake of fixing this bug difficult to see. However, I would be happy to re-tabulate the file in a separate patch after the fact if it makes a difference to reviewers.
Comment 4 Sam Weinig 2010-01-14 17:08:44 PST
Comment on attachment 46620 [details]
Patch

Please add the FIXME we discussed to SVGRenderStyle.h
Comment 5 Beth Dakin 2010-01-14 17:13:55 PST
Fixed with r53300.
Comment 6 Eric Seidel (no email) 2010-01-14 17:53:42 PST
Looks like this broke 6 tests on the Qt bot:
http://build.webkit.org/results/Qt%20Linux%20Release/r53300%20(6017)/results.html

I guess Qt probably has custom results for those tests?
Comment 7 Eric Seidel (no email) 2010-01-14 18:02:58 PST
Re-opening due to bot failures.
Comment 8 Csaba Osztrogonác 2010-01-14 22:46:35 PST
Created attachment 46643 [details]
proposed fix for Qt port

Qt port has own expected files for SVG tests, because sometimes small pixel can be occured (because of different fonts) and some other reason. eg. MACs dump SVG path with fixed point (2 decimal) number, Qt port dump it with floating point number (so many decimal as need). The latter thing is not so good, I'm going to refactor it to be same to MAC. (I prepared a patch before, but some tests should be updated with it.)

Now we should update results. The differents are analogous as in http://trac.webkit.org/changeset/53300
Comment 9 Csaba Osztrogonác 2010-01-14 23:33:05 PST
Comment on attachment 46643 [details]
proposed fix for Qt port

rs-ed by Kenneth Rohde Christiansen, anded landed in 53322.