Currently there's only a CG path. Switching to the platform independent GraphicsContext API should allow canvas shadows to work with cairo or skia.
Created attachment 25645 [details] take 1 Is there a way for me to convert CMYK to RGB to convert the last method as well?
Created attachment 25646 [details] take 2 Oops, remove an unnecessary call to applyShadow.
Comment on attachment 25646 [details] take 2 CGContextSetShadowWithColor takes an offset and radius in device space, but GraphicsContext::setShadow()'s parameters are in user space. I think this patch will lead to the wrong behavior in transformed contexts.
(In reply to comment #3) > (From update of attachment 25646 [details] [review]) > CGContextSetShadowWithColor takes an offset and radius in device space, but > GraphicsContext::setShadow()'s parameters are in user space. I think this patch > will lead to the wrong behavior in transformed contexts. You're right, this patch causes transformations to be applied to the shadow as well. It's unclear to me what the expected behavior is. On Safari 3.2, I see the transformation not being applied to the shadow. In Safari nightlies, I see the transformation being applied. I'll upload a test case.
Created attachment 25686 [details] test for transform and shadow On some versions of safari, the red box is to the right, in others, it is below and to the right.
TOT WebKit was changed to make CSS text-shadow and box-shadow respect transforms. However, my reading of the canvas specification is that when drawing in a canvas, the canvas's CTM should not affect shadow drawing: "The shadowOffsetX and shadowOffsetY attributes specify the distance that the shadow will be offset in the positive horizontal and positive vertical distance respectively. Their values are in coordinate space units. They are not affected by the current transformation matrix" <http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#shadows>.
Created attachment 25698 [details] [1/1] Convert CanvasRenderingContext2D::setShadow to use platform-independent GraphicsContext API. --- LayoutTests/fast/css/shadow-scale.html | 8 ++ .../mac/fast/css/shadow-scale-expected.checksum | 1 + .../mac/fast/css/shadow-scale-expected.png | Bin 0 -> 14750 bytes .../mac/fast/css/shadow-scale-expected.txt | 7 ++ WebCore/html/CanvasRenderingContext2D.cpp | 76 +++++--------------- WebCore/platform/graphics/GraphicsContext.cpp | 4 +- WebCore/platform/graphics/GraphicsContext.h | 4 +- .../graphics/cairo/GraphicsContextCairo.cpp | 2 +- WebCore/platform/graphics/cg/GraphicsContextCG.cpp | 31 +++++--- WebCore/platform/graphics/qt/GraphicsContextQt.cpp | 2 +- 10 files changed, 60 insertions(+), 75 deletions(-)
Comment on attachment 25698 [details] [1/1] Convert CanvasRenderingContext2D::setShadow to use Ok, no longer apply transforms to canvas shadows.
Comment on attachment 25698 [details] [1/1] Convert CanvasRenderingContext2D::setShadow to use The patch is missing a change log entry. I think the test is redundant, because fast/transforms/shadows.html covers that case as well as several other. Given the expected usage pattern, do you think it will make more sense to make the shadow behavior part of the graphics state, so that canvas can just set it once when it creates the context?
(In reply to comment #9) > (From update of attachment 25698 [details] [review]) > The patch is missing a change log entry. I think the test is redundant, because > fast/transforms/shadows.html covers that case as well as several other. Given > the expected usage pattern, do you think it will make more sense to make the > shadow behavior part of the graphics state, so that canvas can just set it once > when it creates the context? It's possible, but a bit tricky. The GraphicsContext is owned by an ImageBuffer which is owned by the HTML canvas element. I guess this would involve adding a default bool param to ImageBuffer::create for the transform case. If you think this is the right approach, let me know and I'll make the change. Otherwise, I'll make a new patch without the layout test and with a ChangeLog entry.
Hyatt, what do you think?
I agree with Mitz.
Created attachment 25782 [details] [1/1] Add a bool to GraphicsContext so that shadows can ignore transformations. This is needed by HTML canvas element where the spec says shadows are applied w/o transformations. --- WebCore/ChangeLog | 23 ++++++ WebCore/html/CanvasRenderingContext2D.cpp | 76 +++++--------------- WebCore/html/HTMLCanvasElement.cpp | 2 + WebCore/platform/graphics/GraphicsContext.cpp | 5 ++ WebCore/platform/graphics/GraphicsContext.h | 1 + WebCore/platform/graphics/GraphicsContextPrivate.h | 3 + WebCore/platform/graphics/cg/GraphicsContextCG.cpp | 29 +++++--- 7 files changed, 71 insertions(+), 68 deletions(-)
Comment on attachment 25782 [details] [1/1] Add a bool to GraphicsContext so that shadows can ignore Looks good. I really need to add a float-enabled Color class to platform, since with this patch, we only support 256 grays for shadows. Which is fine... but still lame. r+
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/html/CanvasRenderingContext2D.cpp M WebCore/html/HTMLCanvasElement.cpp M WebCore/platform/graphics/GraphicsContext.cpp M WebCore/platform/graphics/GraphicsContext.h M WebCore/platform/graphics/GraphicsContextPrivate.h M WebCore/platform/graphics/cg/GraphicsContextCG.cpp Committed r39105