Status

()

Core
SVG
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: tor, Assigned: tor)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 6 obsolete attachments)

(Assignee)

Description

12 years ago
 
(Assignee)

Comment 1

12 years ago
Created attachment 203291 [details] [diff] [review]
<mask> implementation
Attachment #203291 - Flags: review?(scootermorris)

Comment 2

12 years ago
Should nsSVGFeaturesList.h be modified to have http://www.w3.org/TR/SVG11/feature#Mask as a supported feature as part of this patch?
(Assignee)

Comment 3

12 years ago
Created attachment 204045 [details] [diff] [review]
modify feature list, too
Attachment #203291 - Attachment is obsolete: true
Attachment #204045 - Flags: review?(scootermorris)
Attachment #203291 - Flags: review?(scootermorris)
(Assignee)

Comment 4

12 years ago
Created attachment 204046 [details] [diff] [review]
try the right attachment...
Attachment #204045 - Attachment is obsolete: true
Attachment #204046 - Flags: review?(scootermorris)
Attachment #204045 - Flags: review?(scootermorris)

Comment 5

12 years ago
FYI: The tree has drifted underneath this patch (nsSVGUtils.h and nsSVGUtils.cpp to name two examples).  Pretty simple to workaround, but I thought I'd let you know.
(Assignee)

Comment 6

12 years ago
Created attachment 207988 [details] [diff] [review]
update to tip
Attachment #204046 - Attachment is obsolete: true
Attachment #207988 - Flags: review?(scootermorris)
Attachment #204046 - Flags: review?(scootermorris)
(Assignee)

Comment 7

12 years ago
Created attachment 208766 [details] [diff] [review]
also fix clipPaths for multiple overlapping children
Attachment #207988 - Attachment is obsolete: true
Attachment #208766 - Flags: review?(scootermorris)
Attachment #207988 - Flags: review?(scootermorris)

Comment 8

12 years ago
Comment on attachment 208766 [details] [diff] [review]
also fix clipPaths for multiple overlapping children

>Index: layout/svg/base/src/nsSVGClipPathFrame.cpp

> NS_IMETHODIMP
> nsSVGClipPathFrame::ClipPaint(nsISVGRendererCanvas* canvas,
>+                              nsISVGRendererSurface* aClipSurface,
>                               nsISVGChildFrame* aParent,
>                               nsCOMPtr<nsIDOMSVGMatrix> aMatrix)
> {
>   nsRect dirty;
>   nsresult rv;
> 
>   mClipParent = aParent,
>   mClipParentMatrix = aMatrix;
> 
>   NotifyCanvasTMChanged(PR_TRUE);
> 
>-  rv = canvas->SetRenderMode(nsISVGRendererCanvas::SVG_RENDER_MODE_CLIP);

Could you provide a comment here about what IsTrivial means?  Something like:
// See if we have more than one SVG child.  If not do a simple clip.

>+  PRBool isTrivial;
>+  IsTrivial(&isTrivial);
>+
>+  if (isTrivial)
>+    rv = canvas->SetRenderMode(nsISVGRendererCanvas::SVG_RENDER_MODE_CLIP);
>+  else {
>+    rv = canvas->SetRenderMode(nsISVGRendererCanvas::SVG_RENDER_MODE_CLIP_MASK);
>+
>+    canvas->PushSurface(aClipSurface);
>+  }
>+
>   
> 

>Index: layout/svg/base/src/nsSVGGFrame.cpp
>===================================================================

>@@ -112,66 +110,106 @@ nsSVGGFrame::PaintSVG(nsISVGRendererCanv
>     if (mFilter) {
>       if (!mFilterRegion)
>         mFilter->GetInvalidationRegion(this, getter_AddRefs(mFilterRegion));
>       mFilter->FilterPaint(canvas, this);
>       return NS_OK;
>     }
>   }
> 
>+  nsISVGOuterSVGFrame* outerSVGFrame = GetOuterSVGFrame();
>+
>+  /* check for a clip path */
>+
>+  PRBool trivialClip = PR_TRUE;
>   nsSVGClipPathFrame *clip = NULL;
>+  nsCOMPtr<nsISVGRendererSurface> clipMaskSurface;
>+
>   aURI = GetStyleSVGReset()->mClipPath;
>   if (aURI) {
>     NS_GetSVGClipPathFrame(&clip, aURI, mContent);
> 
>     if (clip) {
>-      nsCOMPtr<nsIDOMSVGMatrix> matrix = GetCanvasTM();
>-      canvas->PushClip();
>-      clip->ClipPaint(canvas, this, matrix);

Again, a comment would be helpful...

>+      clip->IsTrivial(&trivialClip);
>+
>+      if (trivialClip) {
>+        canvas->PushClip();
>+      } else {
>+        nsSVGUtils::GetSurface(outerSVGFrame, getter_AddRefs(clipMaskSurface));
>+        if (!clipMaskSurface)
>+          clip = nsnull;
>+      }
>+
>+      if (clip) {
>+        nsCOMPtr<nsIDOMSVGMatrix> matrix = GetCanvasTM();
>+        clip->ClipPaint(canvas, clipMaskSurface, this, matrix);
>+      }
>     }
>   }


>+  /* check for mask */
>+
>+  nsSVGMaskFrame *mask = nsnull;
>+  nsCOMPtr<nsISVGRendererSurface> maskSurface, maskedSurface;
>+
>+  aURI = GetStyleSVGReset()->mMask;
>+  if (aURI) {
>+    NS_GetSVGMaskFrame(&mask, aURI, mContent);
>+
>+    if (mask) {
>+      nsSVGUtils::GetSurface(outerSVGFrame, getter_AddRefs(maskSurface));
>+
>+      if (maskSurface) {
>+        nsCOMPtr<nsIDOMSVGMatrix> matrix = GetCanvasTM();
>+        if (NS_FAILED(mask->MaskPaint(canvas, maskSurface, this, matrix,
>+                                      display->mOpacity)))
>+          maskSurface = nsnull;
>       }
>     }
>   }
> 
>+  if (maskSurface || clipMaskSurface || display->mOpacity != 1.0) {
>+    nsSVGUtils::GetSurface(outerSVGFrame, getter_AddRefs(maskedSurface));
>+    if (maskedSurface) {
>+      canvas->PushSurface(maskedSurface);
>+    } else
>+      maskSurface = nsnull;



Why is this needed?  A comment is probably in order.

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

Addref'ing a frame is probably not a good idea...
>+    NS_ADDREF_THIS();
>+    return NS_OK;
>+  }
>+  return (nsSVGDefsFrame::QueryInterface(aIID, aInstancePtr));
>+}


>+NS_IMETHODIMP
>+nsSVGMaskFrame::InitSVG()
>+{
>+  nsresult rv = nsSVGDefsFrame::InitSVG();
>+  if (NS_FAILED(rv))
>+    return rv;
>+

Shouldn't this be mMaskParentMatrix = nsnull; ?
>+  mMaskParentMatrix = NULL;
>+
(Assignee)

Comment 9

12 years ago
Created attachment 209004 [details] [diff] [review]
adjust per review comments
Attachment #208766 - Attachment is obsolete: true
Attachment #209004 - Flags: review?(scootermorris)
Attachment #208766 - Flags: review?(scootermorris)
(Assignee)

Comment 10

12 years ago
Created attachment 209009 [details] [diff] [review]
right version...
Attachment #209004 - Attachment is obsolete: true
Attachment #209009 - Flags: review?(scootermorris)
Attachment #209004 - Flags: review?(scootermorris)

Comment 11

12 years ago
Comment on attachment 209009 [details] [diff] [review]
right version...

Looks fine to me
Attachment #209009 - Flags: review?(scootermorris) → review+
(Assignee)

Comment 12

12 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
This broke the AIX build:

/home/tbox/sb/tinderbox/AIX_5.1_Clobber/mozilla/layout/svg/base/src/nsSVGClipPathFrame.cpp
"/usr/include/math.h", line 236.9: 1540-0848 (S) The macro name "M_PI" is already defined with a different definition.
"/home/tbox/sb/tinderbox/AIX_5.1_Clobber/mozilla/layout/svg/base/src/nsSVGUtils.h", line 60.9: 1540-0425 (I) "M_PI" is defined on line 60 of "/home/tbox/sb/tinderbox/AIX_5.1_Clobber/mozilla/layout/svg/base/src/nsSVGClipPathFrame.cpp".
You need to log in before you can comment on or make changes to this bug.