Attempt at getting GDI text to render when drawn using canvas. I added...

Attempt at getting GDI text to render when drawn using canvas. I added
layer depth to PlatformContextSkia and explicitly do a begin/end layer
when drawing GDI text if there isn't already one. This seems heavy
handed, but works. Let me know if you prefer another approach.

I realize this will need to be upstreamed and not land here, I wanted
to get your approval before doing that.

BUG=none
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=8409

[sky, 2009-01-20 20:53:18]

I've changed this around based on our conversation Friday, specifically:
. When drawing text with GDI to an image buffer we create a layer.
. I've implemented clipToImageBuffer by way of a specical layer. The clipping
  is ultimately done by way of combining the two images with kDstIn_Mode.

[brettw, 2009-01-21 18:04:25]

http://codereview.chromium.org/18339/diff/212/14
File third_party/WebKit/WebCore/platform/graphics/chromium/FontChromiumWin.cpp
(right):

http://codereview.chromium.org/18339/diff/212/14#newcode184
Line 184: graphicsContext->beginTransparencyLayer(1.0);
Can you mention that this doesn't handle ClearType well, and this should be
removed when we support better text and transparency handling?

http://codereview.chromium.org/18339/diff/212/14#newcode264
Line 264: if (createdLayer) {
Don't use {} for single-line ifs.

http://codereview.chromium.org/18339/diff/212/15
File third_party/WebKit/WebCore/platform/graphics/skia/PlatformContextSkia.cpp
(right):

http://codereview.chromium.org/18339/diff/212/15#newcode479
Line 479: paint.setPorterDuffXfermode(SkPorterDuff::kDstIn_Mode);
This looks like it uses the alpha of the clipping image. Are you sure that's
what we want and not use the color density instead? (I don't actually know the
answer.)

http://codereview.chromium.org/18339/diff/212/16
File third_party/WebKit/WebCore/platform/graphics/skia/PlatformContextSkia.h
(right):

http://codereview.chromium.org/18339/diff/212/16#newcode76
Line 76: void setDrawingToImageBuffer(bool value);
Can you comment these?

http://codereview.chromium.org/18339/diff/212/16#newcode85
Line 85: void beginLayerClippedToImage(const WebCore::FloatRect& rect,
Can you comment on the lifetime of the pointer (I assume that the caller must
make sure it lives until restore() is called).

[sky, 2009-01-21 18:38:40]

New snapshot uploaded.

http://codereview.chromium.org/18339/diff/212/15
File third_party/WebKit/WebCore/platform/graphics/skia/PlatformContextSkia.cpp
(right):

http://codereview.chromium.org/18339/diff/212/15#newcode479
Line 479: paint.setPorterDuffXfermode(SkPorterDuff::kDstIn_Mode);
On 2009/01/21 18:04:25, brettw wrote:
> This looks like it uses the alpha of the clipping image. Are you sure that's
> what we want and not use the color density instead? (I don't actually know the
> answer.)

I'm not positive (wouldn't documentation be nice?), but I think this is what we
want.

For the test case I'm looking at WebKit explicitly uses black to fill in the
clip image. As black is 0x000000 using a color multiply results in all black
output. Using DstInt gives us close enough to the expected output.

There is only one other place that uses this method, it's in
RenderBox::paintFillLayerExtended. As far as I can tell this forces black as
well, but I'm not positive as I don't have a test case that triggers this code
path.

http://codereview.chromium.org/18339/diff/212/16
File third_party/WebKit/WebCore/platform/graphics/skia/PlatformContextSkia.h
(right):

http://codereview.chromium.org/18339/diff/212/16#newcode85
Line 85: void beginLayerClippedToImage(const WebCore::FloatRect& rect,
On 2009/01/21 18:04:25, brettw wrote:
> Can you comment on the lifetime of the pointer (I assume that the caller must
> make sure it lives until restore() is called).

I thought that was the case, but it isn't. I've changed the code to copy off the
underlying SkBitmap.

[brettw, 2009-01-21 18:56:55]

http://codereview.chromium.org/18339/diff/212/15
File third_party/WebKit/WebCore/platform/graphics/skia/PlatformContextSkia.cpp
(right):

http://codereview.chromium.org/18339/diff/212/15#newcode118
Line 118: , m_imageBufferClip(NULL)
Use 0 instead of Null in WebKit.

http://codereview.chromium.org/18339/diff/212/15#newcode479
Line 479: paint.setPorterDuffXfermode(SkPorterDuff::kDstIn_Mode);
Can you put a note here that we don't actually know what's going on? This way
somebody in the future debugging why image masks don't work won't assume that
it's correct.

http://codereview.chromium.org/18339/diff/217/220
File third_party/WebKit/WebCore/platform/graphics/skia/PlatformContextSkia.h
(right):

http://codereview.chromium.org/18339/diff/217/220#newcode77
Line 77: // for a canvas.
This doesn't make any sense, canvas isn't what you mean here. Can you say a
GraphicsContext for a web page, and also say what this actually controls
(basically our assumptions about the opacity of the background).

[sky, 2009-01-21 19:09:25]

Updated. If you still aren't happy with the comment in PlatformContextSkia.h
just send me what you want:)

Thanks!

[brettw, 2009-01-21 19:11:11]

LGTM.

For the note, can you tack on "I'm not 100% sure this is what WebKit expects for
image clipping." to the end of the comment.