Closed
Bug 738691
Opened 13 years ago
Closed 13 years ago
[Azure] Deal with stroking text and appending glyphs to the current path
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
Details
Attachments
(2 files)
5.16 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
13.12 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
For SVG we need to support stroking text outlines and appending text to the current path.
Assignee | ||
Comment 1•13 years ago
|
||
This will allow adding the path for glyphs to a pathbuilder. This is required for supporting the new code.
Attachment #608748 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 2•13 years ago
|
||
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)
Comment 3•13 years ago
|
||
(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?
Assignee | ||
Comment 4•13 years ago
|
||
(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•13 years ago
|
||
Comment on attachment 608748 [details] [diff] [review]
Part 1: Add CopyGlyphsToBuilder API
How about instead of doing this as CopyGlyphsToBuilder()
We add AppendPathToBuilder(GetPathForGlyphs())?
Assignee | ||
Comment 6•13 years ago
|
||
(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•13 years ago
|
||
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+
Updated•13 years ago
|
Attachment #608750 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 8•13 years ago
|
||
Assignee | ||
Comment 9•13 years ago
|
||
Comment 10•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/89dfa68488e4
https://hg.mozilla.org/mozilla-central/rev/1b468e61ac02
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•