Gpu can draw underlined text.

[bungeman, 2011-08-22 19:02:06]

See http://code.google.com/p/skia/issues/detail?id=343 .

Separates the drawing of the glyphs from the drawing of the decorations at the
implementation level.

[bungeman, 2011-08-22 20:08:54]

Pulled decoration call up to canvas.

[Steve VanDeBogart, 2011-08-22 20:16:19]

On 2011/08/22 20:08:54, bungeman wrote:
> Pulled decoration call up to canvas.

Doing it this way will also require changes to SkPDFDevice.  Though, unifying
underline and strikethrough across device types is probably a win.

[bungeman, 2011-08-22 20:52:58]

Pulled implementation up to canvas and added PDF change.

[reed1, 2011-08-22 21:09:51]

Might refactor inside canvas to remove the friend declaration, but LGTM for now.

[bungeman, 2011-08-22 21:32:57]

Committed revision 2154.

[Steve VanDeBogart, 2011-08-22 22:19:55]

Will this regress drawing text with an xfermode like SrcIn, since you're drawing
it in two steps now instead of one?

http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp
File src/pdf/SkPDFDevice.cpp (left):

http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp#oldc...
src/pdf/SkPDFDevice.cpp:70: SkScalar* y, SkScalar* width) {
And then we don't need the width arg

http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp#oldc...
src/pdf/SkPDFDevice.cpp:812: SkScalar width;
This chunk of code can go away as well.  width/Ptr is only used in the removed
section.

[bungeman, 2011-08-23 12:54:00]

On 2011/08/22 22:19:55, Steve VanDeBogart wrote:
> Will this regress drawing text with an xfermode like SrcIn, since you're
drawing
> it in two steps now instead of one?
> 
> http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp
> File src/pdf/SkPDFDevice.cpp (left):
> 
>
http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp#oldc...
> src/pdf/SkPDFDevice.cpp:70: SkScalar* y, SkScalar* width) {
> And then we don't need the width arg
> 
>
http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp#oldc...
> src/pdf/SkPDFDevice.cpp:812: SkScalar width;
> This chunk of code can go away as well.  width/Ptr is only used in the removed
> section.

This will not change the current behavior of SrcIn, since we draw per-glyph and
the decorations are currently drawn separately as well. The clean up here will
be done with http://codereview.appspot.com/4939044/ , since that makes things a
bit easier.

[Steve VanDeBogart, 2011-08-23 21:01:44]

On 2011/08/23 12:54:00, bungeman wrote:
> On 2011/08/22 22:19:55, Steve VanDeBogart wrote:
> > Will this regress drawing text with an xfermode like SrcIn, since you're
> drawing
> > it in two steps now instead of one?
> > 
> > http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp
> > File src/pdf/SkPDFDevice.cpp (left):
> > 
> >
>
http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp#oldc...
> > src/pdf/SkPDFDevice.cpp:70: SkScalar* y, SkScalar* width) {
> > And then we don't need the width arg
> > 
> >
>
http://codereview.appspot.com/4919047/diff/10010/src/pdf/SkPDFDevice.cpp#oldc...
> > src/pdf/SkPDFDevice.cpp:812: SkScalar width;
> > This chunk of code can go away as well.  width/Ptr is only used in the
removed
> > section.
> 
> This will not change the current behavior of SrcIn, since we draw per-glyph
and
> the decorations are currently drawn separately as well. The clean up here will

We're going from one draw call on the device to multiple draw calls to the
device.  At least for PDF (but probably for raster as well), I don't think this
will interact well with xfer modes that change DST.  You could composite the
text and underline/strikethrough on a layer first.

> be done with http://codereview.appspot.com/4939044/ , since that makes things
a
> bit easier.

[bungeman, 2011-08-23 23:16:14]

>We're going from one draw call on the device to multiple draw calls to the
>device.  At least for PDF (but probably for raster as well), I don't think this
>will interact well with xfer modes that change DST.  You could composite the
>text and underline/strikethrough on a layer first.

The raster case was already drawing glyph-at-a-time and the decorations after.
This could in fact cause issues where glyphs or the decoration overlap, but this
is not currently handled. I attempted to see what PDF does, but it appears that
the PDF device does strange things or will simply go OOM (reserving 1.5GB of
memory) on some xfer modes when not using a layer. This can be seen by replacing
canvas->saveLayer with canvas->save in xfermodes.cpp.