Bug 95620 - Use -webkit-clip-path shapes to clip SVG elements
Summary: Use -webkit-clip-path shapes to clip SVG elements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Schulze
URL:
Keywords:
Depends on:
Blocks: 95389
  Show dependency tree / graph
 
Reported: 2012-08-31 15:55 PDT by Dirk Schulze
Modified: 2012-09-01 17:40 PDT (History)
4 users (show)

See Also:

Attachments
Patch (24.46 KB, patch)
2012-08-31 18:05 PDT, Dirk Schulze 		no flags Details | Formatted Diff | Diff
Patch (24.95 KB, patch)
2012-08-31 20:17 PDT, Dirk Schulze 		rwlbuis: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Schulze 2012-08-31 15:55:42 PDT 
Use -webkit-clip-path shapes to clip SVG elements.
Comment 1 Dirk Schulze 2012-08-31 18:05:06 PDT 
Created attachment 161802 [details]
Patch
Comment 2 Dirk Schulze 2012-08-31 18:05:49 PDT 
This is actually not for review. I just want to see if it builds everywhere.
Comment 3 Dirk Schulze 2012-08-31 20:17:23 PDT 
Created attachment 161812 [details]
Patch
Comment 4 Rob Buis 2012-09-01 09:45:40 PDT 
Comment on attachment 161812 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=161812&action=review

Looks good.

> Source/WebCore/rendering/style/BasicShapes.cpp:47
> +                                   m_cornerRadiusY.isUndefined() ? 0 : floatValueForLength(m_cornerRadiusY, boundingBox.height())));

Seems like indenting is not quite right?

> Source/WebCore/rendering/svg/SVGRenderingContext.cpp:152
> +    if (clipper && !style->clipPath()) {

Why not do !style->clipPath() test first? Only if true you'd need to get clipper. You could also reuse the clipShape variable from above.

> LayoutTests/svg/clip-path/clip-path-shape-circle-1-expected.svg:1
> +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">

Is the xlink stuff needed in any of these tests?

> LayoutTests/svg/clip-path/clip-path-shape-ellipse-2.svg:2
> +<rect width="200" height="200" fill="green" style="-webkit-clip-path: ellipse(100px, 75px, 100px, 75px)"/>

Should height be 150 here?
Comment 5 Dirk Schulze 2012-09-01 17:15:14 PDT 
Committed r127383: <http://trac.webkit.org/changeset/127383>
Comment 6 Dirk Schulze 2012-09-01 17:40:58 PDT 
Fixed all suggested changes before landing, but not the change on the value for the ellipse. It is rx,ry and not with and height. (just for the logs)