Closed Bug 1290781 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox50 --- affected
firefox51 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

References

(Blocks 1 open bug)

Details

Attachments

(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]
patch

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 jwatt@jwatt.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15bd0363e516
Make the propagation of context paint to SVG glyphs much more robust. r=heycam
https://hg.mozilla.org/mozilla-central/rev/15bd0363e516
Status: NEW → RESOLVED
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.