930468 - Add a Moz2D version of SVGPathData::ConstructPath

Status: RESOLVED FIXED

(Core :: SVG, defect)

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Bas, can you review the use of the Moz2D API in this patch?

Comment 2

[Jonathan Watt [:jwatt]]

Comment on attachment 821668 [details] [diff] [review]
patch

Cameron, can you look at the SVG changes?

Comment 3

[Bas Schouten (:bas.schouten)]

Comment on attachment 821668 [details] [diff] [review]
patch

Review of attachment 821668 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/svg/content/src/SVGPathData.cpp
@@ +6,5 @@
>  #include "SVGPathData.h"
>  
>  #include "gfx2DGlue.h"
>  #include "gfxPlatform.h"
> +#include "mozilla/gfx/2D.h"

nit: None of this is needed since we now include 2D.h in the header.

@@ +255,5 @@
> +  // coordinates, we double this for |tinyAdvance|:
> +
> +  Matrix currentTransform = aDT->GetTransform();
> +  currentTransform.Invert();
> +  Size tinyAdvance = currentTransform * Size(2.0/256.0, 0.0);

nit: spaces around the / operator and let's use point.

::: content/svg/content/src/SVGPathData.h
@@ +11,5 @@
>  #include "nsIContent.h"
>  #include "nsINode.h"
>  #include "nsIWeakReferenceUtils.h"
> +#include "mozilla/gfx/2D.h"
> +#include "mozilla/RefPtr.h"

nit: You don't need this if you're including 2D.h

Comment 4

[Jonathan Watt [:jwatt]]

I can't land this patch in its entirety just yet since we need Moz2D path measuring to work, and that isn't working on Mac jut yet. Bug 926258 will fix that.

We end up doing measuring from the path when, for example, we end up under this stack:

  mozilla::gfx::Path::ComputeLength
  gfxPath::GetLength
  mozilla::dom::SVGPathElement::GetPathLengthScale
  GetStrokeDashData
  nsSVGUtils::SetupCairoStrokeGeometry
  nsSVGUtils::SetupCairoStroke
  nsSVGPathGeometryFrame::Render
  nsSVGPathGeometryFrame::PaintSVG

Comment 5

[Jonathan Watt [:jwatt]]

I span out the enabling part of the patch into bug 930577 and landed the rest:

https://hg.mozilla.org/integration/mozilla-inbound/rev/075fc0110d9f

Comment 6

[Ed Morley [:emorley]]

Push backed out for crashes on WinXP:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=XP%20&rev=075fc0110d9f

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1203ba8ffc0d
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/561bb4c94909
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e3c9d95e5bc0

Comment 7

[Jonathan Watt [:jwatt]]

The WinXP crashes were caused by bug 930516. Relanded:

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b98c52e5e08

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2b98c52e5e08