Closed Bug 1290781 Opened 3 years ago Closed 3 years ago

Make the propagation of context paint to SVG glyphs much more robust


(Core :: SVG, defect)

Not set



Tracking Status
firefox50 --- affected
firefox51 --- fixed


(Reporter: jwatt, Assigned: jwatt)


(Blocks 1 open bug)



(1 file)

Our SVG-in-OpenType implementation allows SVG glyphs to use the context paint (fill/stroke/etc. specified on the text element that is being painted). The context paint is currently propagated to SVG glyphs by setting it on the Moz2D DrawTarget that is passed into nsSVGUtils::PaintSVGGlyph. This is going to be flakey though. The SVG code creates temporary DrawTarget objects in many places in order to paint things like masks, non-trivial clipPaths, patters, etc.

A better approach would be to temporarily set the context paint on the glyph element's SVGDocument while painting that glyph. That provides a convenient and reliable place from which to obtain the context paint.
Attached patch patchSplinter Review
Daniel is away just now, so hopefully you're okay to review this, Cameron.
Attachment #8776443 - Flags: review?(cam)
Comment on attachment 8776443 [details] [diff] [review]

Review of attachment 8776443 [details] [diff] [review]:

::: gfx/thebes/gfxSVGGlyphs.cpp
@@ -37,5 @@
>  using namespace mozilla;
>  typedef mozilla::dom::Element Element;
> -mozilla::gfx::UserDataKey gfxTextContextPaint::sUserDataKey;

Remove the declaration in gfxSVGGlyphs.h too.

::: layout/svg/SVGTextFrame.cpp
@@ +3723,5 @@
>    TextRenderedRunIterator it(this, TextRenderedRunIterator::eVisibleFrames);
>    TextRenderedRun run = it.Current();
> +  gfxTextContextPaint *outerContextPaint = nsSVGUtils::GetContextPaint(mContent);

Nit: move the "*" next to the type, while you're touching this line.

::: layout/svg/nsSVGPathGeometryFrame.cpp
@@ +803,5 @@
>        return;
>      }
>    }
> +  gfxTextContextPaint *contextPaint = nsSVGUtils::GetContextPaint(mContent);

Nit: and here.

@@ +871,5 @@
>  void
>  nsSVGPathGeometryFrame::PaintMarkers(gfxContext& aContext,
>                                       const gfxMatrix& aTransform)
>  {
> +  gfxTextContextPaint *contextPaint = nsSVGUtils::GetContextPaint(mContent);

Nit: here too.
Attachment #8776443 - Flags: review?(cam) → review+
Pushed by
Make the propagation of context paint to SVG glyphs much more robust. r=heycam
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1094247
You need to log in before you can comment on or make changes to this bug.