Closed Bug 738691 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Graphics, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla14

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(2 files)

For SVG we need to support stroking text outlines and appending text to the current path.
This will allow adding the path for glyphs to a pathbuilder. This is required for supporting the new code.
Attachment #608748 - Flags: review?(jmuizelaar)
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.
Attachment #608750 - Flags: review?(jmuizelaar)
Blocks: 715768
(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?
(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 on attachment 608748 [details] [diff] [review]
Part 1: Add CopyGlyphsToBuilder API

How about instead of doing this as CopyGlyphsToBuilder()
We add AppendPathToBuilder(GetPathForGlyphs())?
(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 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.
Attachment #608748 - Flags: review?(jmuizelaar) → review+
Attachment #608750 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/89dfa68488e4
https://hg.mozilla.org/mozilla-central/rev/1b468e61ac02
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Depends on: 742727
Depends on: 743767
No longer blocks: 715768
Blocks: 715768
You need to log in before you can comment on or make changes to this bug.