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

RESOLVED FIXED in mozilla14

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bas, Assigned: bas)

Tracking

unspecified
mozilla14
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
For SVG we need to support stroking text outlines and appending text to the current path.
(Assignee)

Comment 1

5 years ago
Created attachment 608748 [details] [diff] [review]
Part 1: Add CopyGlyphsToBuilder API

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

5 years ago
Created attachment 608750 [details] [diff] [review]
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.
Attachment #608750 - Flags: review?(jmuizelaar)
(Assignee)

Updated

5 years ago
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?
(Assignee)

Comment 4

5 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 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

5 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 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+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/23fb49402114
(Assignee)

Comment 9

5 years ago
Belay that last comment:

https://hg.mozilla.org/integration/mozilla-inbound/rev/89dfa68488e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b468e61ac02
https://hg.mozilla.org/mozilla-central/rev/89dfa68488e4
https://hg.mozilla.org/mozilla-central/rev/1b468e61ac02
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14

Updated

5 years ago
Depends on: 742727
Depends on: 743767
(Assignee)

Updated

5 years ago
No longer blocks: 715768
(Assignee)

Updated

5 years ago
Blocks: 715768
You need to log in before you can comment on or make changes to this bug.