341292 - Remove nsISVGPathFlatten interface

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[tor]

Remove interface plus the cairo path -> generic storage conversion.

Comment 1

[tor]

Attachment: interface removal

Comment 2

[Robert Longson [:longsonr]]

Comment on attachment 225317 [details] [diff] [review]
interface removal

I think that nsSVGPathElement::GetTotalLength need to flush the layout before it calls GetFlattenedPath as GetFlattenedPath no longer flushes itself. Either that or flush it in GetFlattenedPath.

+  float midpoint = aXOffset + aAdvance/2;
+      float normalization = 1.0/sqrt(dx*dx+dy*dy);

nits: Additional spaces here?

+class nsSVGFlattenedPath
+{
+private:
+  cairo_path_t *mPath;
+
+public:
+  nsSVGFlattenedPath(cairo_path_t *aPath) : mPath(aPath) {}
+  ~nsSVGFlattenedPath() { if (mPath) cairo_path_destroy(mPath); }
+
+  float Length();
+  void FindPoint(float aAdvance, float aXOffset, float aYOffset,
+                 float *aX, float *aY, float *aAngle);
+};
+

Wouldn't Length make more sense as GetLength?

Comment 3

[tor]

(In reply to comment #2)
> (From update of attachment 225317 [details] [diff] [review] [edit])
> I think that nsSVGPathElement::GetTotalLength need to flush the layout before
> it calls GetFlattenedPath as GetFlattenedPath no longer flushes itself. Either
> that or flush it in GetFlattenedPath.

With this patch, there isn't any dependency on layout for flattening, so we don't need to flush anymore.

Comment 4

[Robert Longson [:longsonr]]

Comment on attachment 225317 [details] [diff] [review]
interface removal

OK. My other points are pretty cosmetic. I'm sure you can fix them up on checkin. r=rl.

Comment 5

[Robert Longson [:longsonr]]

I guess that should have been r=longsonr

Comment 6

[tor]

Attachment: address review comments

In the future we'll probably want to cache the flattened path as a content property.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

+  nsSVGFlattenedPath *flat = GetFlattenedPath();

Use nsAutoPtr to hold this, it will delete for you.

+  return new nsSVGFlattenedPath(path);

If the 'new' fails, this leaks path.

+    /* should never happen with a flattened path */

Put an NS_WARNING in here

+  }
+  }

Can you indent somewhere in here so you don't need two } at the same level?

Comment 8

[tor]

Attachment: address roc's comments

I need to get around to configuring emacs to automatically indent switch statements properly one of these days...

Comment 9

[tor]

Checked in.