193825 - implement <svg:clipPath>

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Harald Albrecht]

User-Agent:       Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.3b) Gecko/20030130
Build Identifier: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.3b) Gecko/20030130

Instead of clipping away everthing outside the clip path, Mozilla draws
everything outside and the part covered by the clip path is just black (problem
with the clip mask?).

Reproducible: Always

Steps to Reproduce:
1. load the attached example svg in Mozilla

Actual Results:  
The part covered by the clip path is drawn in black. All other parts of the
graphic that should have been clipped away appear normally.

Expected Results:  
Only the part of the graphic within the clipped part should be drawn.

Comment 1

[Harald Albrecht]

Attachment: example svg file triggering the clipping bug

Comment 2

[Alex Fritze]

The problem here is that <clipPath> is not recognised by Mozilla SVG yet, so the
clipPath is not applied. Worse, because Mozilla SVG transparently ignores
unknown tags (by creating a nsSVGGenericContainerFrame for them), the SVG
content contained within the clipPath gets drawn.

Comment 3

[tor]

Attachment: <svg:clipPath> implementation

Renderer changes only done for cairo at this point.

Comment 4

[tor]

Attachment: clipPath for cairo and gdi+ - libart stubbed out

Comment 5

[tor]

Attachment: changes to shared mozilla files

Comment 6

[tor]

Attachment: changes/new svg-only files

Comment 7

[Scooter Morris]

(In reply to comment #5)
> Created an attachment (id=163745)
> changes to shared mozilla files
> 

Is there a reason that mClipPath is not part of the constructor for nsStyleSVG,
etc, like mClipRule?

Comment 8

[Scooter Morris]

Comment on attachment 163746 [details] [diff] [review]
changes/new svg-only files


>Index: content/svg/content/src/nsSVGClipPathElement.cpp
>===================================================================
>RCS file: content/svg/content/src/nsSVGClipPathElement.cpp
>diff -N content/svg/content/src/nsSVGClipPathElement.cpp
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ content/svg/content/src/nsSVGClipPathElement.cpp	28 Oct 2004 20:48:15 -0000

>+
>+/* readonly attribute nsIDOMSVGAnimatedEnumeration gradientUnits; */

Looks like the wrong comment...

>+NS_IMETHODIMP nsSVGClipPathElement::GetClipPathUnits(nsIDOMSVGAnimatedEnumeration * *aClipPathUnits)
>+{


>Index: dom/public/idl/svg/nsIDOMSVGClipPathElement.idl
>===================================================================
>RCS file: dom/public/idl/svg/nsIDOMSVGClipPathElement.idl
>diff -N dom/public/idl/svg/nsIDOMSVGClipPathElement.idl
>--- /dev/null	1 Jan 1970 00:00:00 -0000
>+++ dom/public/idl/svg/nsIDOMSVGClipPathElement.idl	28 Oct 2004 20:48:15 -0000

>+{
>+  // Extended to provide enumerations for gradientUnits

Same wrong comment....

>+  // These constants are taken from SVGUnitTypes
>+  const unsigned short SVG_CPUNITS_UNKNOWN = 0;
>+  const unsigned short SVG_CPUNITS_OBJECTBOUNDINGBOX = 1;
>+  const unsigned short SVG_CPUNITS_USERSPACEONUSE = 2;
>+
>+  readonly attribute nsIDOMSVGAnimatedEnumeration clipPathUnits;
>+};

>Index: layout/svg/base/src/nsSVGClipPathFrame.cpp
>===================================================================
>RCS file: layout/svg/base/src/nsSVGClipPathFrame.cpp
>diff -N layout/svg/base/src/nsSVGClipPathFrame.cpp

>+NS_IMETHODIMP
>+nsSVGClipPathFrame::QueryInterface(REFNSIID aIID, void** aInstancePtr)
>+{
>+  if (nsnull == aInstancePtr) {
>+    return NS_ERROR_NULL_POINTER;
>+  }
>+  if (aIID.Equals(nsSVGClipPathFrame::GetCID())) {
>+    *aInstancePtr = (void*)(nsSVGClipPathFrame*)this;
>+    NS_ADDREF_THIS();
>+    return NS_OK;
>+  }
>+  return (nsSVGDefsFrame::QueryInterface(aIID, aInstancePtr));
>+}

Hmmmm.. the need for a specific version of QueryInterface keeps coming up.  Did
anyone ever
respond with any approach that would allow you to avoid overloading
QueryInterface?

Comment 9

[tor]

Attachment: changes/new svg-only files (fix comments)

mClipPath is part of nsStyleSVGReset instead, as the property does not inherit.


No, discussion hasn't turned up a simpler way of doing the QI.

Comment 10

[tor]

Attachment: changes to shared mozilla files (updated to tip)

Comment 11

[tor]

Attachment: changes/new svg-only files (fix gdi+ problem)

Comment 12

[tor]

Attachment: changes to shared mozilla files (update to tip, fixes)

Comment 13

[tor]

Attachment: changes/new svg-only files (update to tip, fixes)

Comment 14

[Scooter Morris]

Comment on attachment 169870 [details] [diff] [review]
changes to shared mozilla files (update to tip, fixes)

scooter/r+

Comment 15

[Scooter Morris]

Comment on attachment 169871 [details] [diff] [review]
changes/new svg-only files (update to tip, fixes)

scooter/r+

Comment 16

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 169870 [details] [diff] [review]
changes to shared mozilla files (update to tip, fixes)

You should add to nsStyleContext::DumpRegressionData.

in nsStyleSVGReset constructor, use nsnull rather than NULL

in nsStyleSVG, put all the PRUint8 values next to each other so the struct
packs better (and perhaps separate the others by type as well, int8, int32,
pointer and float), preferably starting with the larger things (i.e., pointer,
float, int32, int8)

Comment 17

[tor]

Checked in.  Reordering nsStyleSVG reduced it from 88 to 76 bytes.