[Webkit-unassigned] [Bug 96381] -webkit-clip-path should parse IRIs

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=96381


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #165258|                            |review-
               Flag|                            |




--- Comment #5 from Dirk Schulze <krit at webkit.org>  2012-09-22 04:38:56 PST ---
(From update of attachment 165258)
View in context: https://bugs.webkit.org/attachment.cgi?id=165258&action=review

Please use phrases on FIXME's. There is a build warning that you never run into a code path in styleresolver.

> LayoutTests/css3/masking/clip-path-reference.html:21
> +<svg height="0">
> +    <clipPath id="c1" clipPathUnits="objectBoundingBox">

Please add a test with userSpaceOnUse as well. Note that you need to specify a with and height for the SVG element then:
<svg height="200" width="200">
<clipPath id="c1" clipPathUnits="userSpaceOnUse">
    <rect x="20" y="20" width="160" height="160"/>
</clipPath>

> Source/WebCore/css/StyleBuilder.cpp:-1690
> -template <ClipPathOperation* (RenderStyle::*getterFunction)() const, void (RenderStyle::*setterFunction)(PassRefPtr<ClipPathOperation>), ClipPathOperation* (*initialFunction)()>
> -class ApplyPropertyClipPath {
> -public:

I was told to use this template, since it is the new way to use StyleBuilder. Like discussed offline, the code from StyleResolver::createPathOperation should be moved here.

> Source/WebCore/css/StyleBuilder.cpp:-2055
> -    setPropertyHandler(CSSPropertyWebkitClipPath, ApplyPropertyClipPath<&RenderStyle::clipPath, &RenderStyle::setClipPath, &RenderStyle::initialClipPath>::createHandler());

Needs to be added again.

> Source/WebCore/css/StyleResolver.cpp:5171
> +    // FIXME: needs handling for load pending SVG documents (similar to what CSS Filters)

Add the reference to the bug report https://bugs.webkit.org/show_bug.cgi?id=90405.

> Source/WebCore/rendering/RenderLayer.cpp:3149
> +            // FIXME: doesn't work if we have an external reference to a SVG document or if the SVG element is after the HTML one

The external referencing doesn't work in SVG either. We have a bug report about that. The one with SVG after HTML should reference to the bug report: https://bugs.webkit.org/show_bug.cgi?id=90405

> Source/WebCore/rendering/RenderLayer.cpp:3151
> +
> +

One empty line s enough.

> Source/WebCore/rendering/RenderLayer.cpp:3155
> +                RenderSVGResourceClipper* clipper = static_cast<RenderSVGResourceClipper*>(clipPath->renderer());
> +                if (clipper)

If you don't have more checks, move the initialisation into the if function: 
if (RenderSVGResourceClipper* clipper = static_cast<RenderSVGResourceClipper*>(clipPath->renderer()))

> Source/WebCore/rendering/RenderLayer.cpp:3156
> +                    clipper->applyClippingToContext(renderer(), calculateLayerBounds(this, rootLayer, 0), paintDirtyRect, transparencyLayerContext);

should be context, not transparencyLayerContext.

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.h:58
> +    // clipPath can be clipped too, but don't have a boundingBox or repaintRect. So we can't call
> +    // applyResource directly and use the rects from the object, since they are empty for RenderSVGResources
> +    // This function is public because we need to apply clipping on HTML element
> +    bool applyClippingToContext(RenderObject*, const FloatRect&, const FloatRect&, GraphicsContext*);

They should have a bounding box. Seems to be a bug.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.