738691 - [Azure] Deal with stroking text and appending glyphs to the current path

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

For SVG we need to support stroking text outlines and appending text to the current path.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Add CopyGlyphsToBuilder API

This will allow adding the path for glyphs to a pathbuilder. This is required for supporting the new code.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Add proper stroke and path support to Azure glyph drawing

This patch adds proper stroke and path support to Azure glyph drawing. It also fixes numerous other bugs related to patterns being properly transformed when a skew is applied and using the proper pattern for glyph fills.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Bas Schouten (:bas) from comment #0)
> For SVG we need to support stroking text outlines and appending text to the
> current path.

What about SVG requires this?

Comment 4

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> (In reply to Bas Schouten (:bas) from comment #0)
> > For SVG we need to support stroking text outlines and appending text to the
> > current path.
> 
> What about SVG requires this?

Rendering strokes of glyphs is required by several things, appending glyphs to the current path is needed for SVG masking. See nsSVGGlyphFrame.cpp for the calling code.

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 608748 [details] [diff] [review]
Part 1: Add CopyGlyphsToBuilder API

How about instead of doing this as CopyGlyphsToBuilder()
We add AppendPathToBuilder(GetPathForGlyphs())?

Comment 6

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #5)
> Comment on attachment 608748 [details] [diff] [review]
> Part 1: Add CopyGlyphsToBuilder API
> 
> How about instead of doing this as CopyGlyphsToBuilder()
> We add AppendPathToBuilder(GetPathForGlyphs())?

I considered this, this would perform significantly worse for D2D, (where we can pass a PathSink directly do an IDWriteScaledFont) although not really hard to implement. It is probably even harder to implement efficient for some other backends. (i.e. in this code if the builder passed in is the current cairo context for example, it could just append the path info for the glyphs to that, so it doesn't have to go through creating a temporary path context.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 608748 [details] [diff] [review]
Part 1: Add CopyGlyphsToBuilder API

Looks fine. It would be great if you could add the note about why CopyGlyphsToBuilder instead of AppendPathToBuilder that would be great.

Comment 8

[Bas Schouten (:bas.schouten)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/23fb49402114

Comment 9

[Bas Schouten (:bas.schouten)]

Belay that last comment:

https://hg.mozilla.org/integration/mozilla-inbound/rev/89dfa68488e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b468e61ac02

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/89dfa68488e4
https://hg.mozilla.org/mozilla-central/rev/1b468e61ac02