988808 - Convert SVG hit-testing to use Moz2D instead of Thebes gfxASurface

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

We should convert SVG hit-testing to use Moz2D instead of Thebes gfxASurface.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Jonathan Watt [:jwatt]]

The AutoStrokeOptions class is primarily to get around the issue of the lifetime of mDathPattern. Avoiding allocating in the common case is a side benefit.

Comment 3

[Jonathan Watt [:jwatt]]

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

We need to be able to generate the path using a different fill rule to the current computed style value, so I need to supply the fill rule to the nsSVGPathGeometryElement::BuildPath overrides. Actually, to be more flexible (I'm pretty sure I'm going to need this) I'll change these overrides so that a PathBuilder object can be supplied.

Comment 4

[Jonathan Watt [:jwatt]]

Attachment: Allow a PathBuilder object to be supplied to nsSVGPathGeometryElement::BuildPath() and its overrides

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: Convert SVG hit-testing to use Moz2D instead of Thebes backed gfxContext

Comment 6

[Bas Schouten (:bas.schouten)]

Comment on attachment 8450522 [details] [diff] [review]
Allow a PathBuilder object to be supplied to nsSVGPathGeometryElement::BuildPath() and its overrides

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

::: content/svg/content/src/SVGPathData.cpp
@@ +301,5 @@
>      }                                                                         \
>    } while(0)
>  
>  TemporaryRef<Path>
> +SVGPathData::BuildPath(PathBuilder* builder,

nit: Please just rename the arg to our conventions and change the rest of the function ;).

::: content/svg/content/src/SVGPathElement.cpp
@@ +404,5 @@
>    }
>  
> +  RefPtr<PathBuilder> builder;
> +  if (aBuilder) {
> +    builder = aBuilder;

nit: Might be good to assert aBuilder->GetFillRule() == GetFillRule()

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 8450523 [details] [diff] [review]
Convert SVG hit-testing to use Moz2D instead of Thebes backed gfxContext

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

::: content/svg/content/src/nsSVGPathGeometryElement.cpp
@@ +222,5 @@
> +  aStroke->mMiterLimit = Float(style->mStrokeMiterlimit);
> +
> +  GetStrokeDashData(aStroke, aFrame);
> +
> +  switch (style->mStrokeLinejoin) {

nit: Can you move these to helper functions if you think we'll ever need these again? :).

::: content/svg/content/src/nsSVGPathGeometryElement.h
@@ +116,5 @@
> +        mDashPattern = mSmallArray;
> +        return mSmallArray;
> +      }
> +      static const mozilla::fallible_t fallible = mozilla::fallible_t();
> +      Float* nonConstTmp = new (fallible) Float[mDashLength];

nit: Just pass fallible_t() or std::nothrow directly? No need to have the stack var.

::: layout/svg/nsSVGPathGeometryFrame.cpp
@@ +233,5 @@
>  }
>  
>  nsIFrame*
>  nsSVGPathGeometryFrame::GetFrameForPoint(const nsPoint &aPoint)
>  {

nit: This function could use some vertical whitespace for readability.

@@ +264,2 @@
>  
> +  // Using ScreenReferenceDrawTarget() opens us to Moz2D backend specific hit-

I don't think so, first of all we want to kill Cairo, second of all if something has a bug that displays something 10 pixels to the right, I'd rather have hittesting move along with it :-).

Comment 8

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/15767e979335
https://hg.mozilla.org/integration/mozilla-inbound/rev/60d0ee6ffa30

Comment 9

[Jonathan Watt [:jwatt]]

I had to change content/svg/content/test/test_pointer-events-3.xhtml to not test right on the boundary of the circle. I also moved the style getting stuff to SVGContentUtils rather than having it on nsSVGPathGeometryElement since I realized we'll actually need that for text later.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15767e979335
https://hg.mozilla.org/mozilla-central/rev/60d0ee6ffa30