Summary: | Skia platform doesn't render text to a canvas or support clipping to an image buffer | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Scott Violet <sky> | ||||||||||||||
Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | fishd, krit | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | OS X 10.5 | ||||||||||||||||
Attachments: |
|
Description
Scott Violet
2009-01-29 12:00:31 PST
Created attachment 27155 [details]
Fix for 23625
This patch helps Chromium pass a number of canvas related test cases.
Comment on attachment 27155 [details]
Fix for 23625
WebKit style omits names for arguments in headers when the names do not provide additional clarity:
Like this:
+ void applyClipFromImage(const WebCore::FloatRect& rect, const SkBitmap& imageBuffer);
neither rect or imageBuffer should be in the definition.
Otherwise this looks fine. One of us can fix those nits when landing.
The end of clipToImageBuffer is not necessarily reached with the next restore. You can use combinations of save and restore between clipToImageBuffer and it's corresponding restore. SVG Masking uses different save/restores between clipToImageBuffer and the real restore. At the moment we use clipToImageBuffer on three points. Filling/stroking texts with gradients or patterns on Canvas, clipping text on a background for CSS and SVG Masking. There is no implementation for SVG Masking on Skia, but there is a patch for making SVG Masking platform aware at https://bugs.webkit.org/show_bug.cgi?id=19243 And I don't know if it is possible to use clipToImageBuffer during a clipToImageBuffer. So it's maybe necessary to store more clipImages: context->save(); context->clipToImageBuffer(); ... context->save(); context->clipToImageBuffer(); ... context->restore(); ... context->restore(); Do you have a test case I can use? I believe: PlatformContextSkia::State::State(const State& other) Needs to reset the m_imageBufferClip to an empty bitmap. Then the scenario you mention should work fine. Well, just use the patch mentined above (for the save/restore's during the clipToImageBuffer call) and visit sites like: http://svg.tutorial.aptico.de/grafik_svg/kap14_2.svg http://www.w3.org/Graphics/SVG/Test/20061213/htmlObjectHarness/full-masking-mask-01-b.html I'don't have an example for the use of clipToImageBuffer during a clipToImageBuffer call. Don't know if masking an object with an masked object in SVG could cause that situation, but maybe not. Created attachment 27269 [details]
masked mask
This is an example of an masked object as a mask. Don't know if webkit can handle it. Haven't tested it on Safari. But it seems to call clipToImageBuffer during the clipToImageBuffer call.
The SVG Masking patch from above is needed to test this on skia.
(In reply to comment #7) > Created an attachment (id=27269) [review] > masked mask Can you attach a screenshot as to what this should look like? Created attachment 27281 [details]
masked mask as tif
... on safari 3.2
(In reply to comment #9) > Created an attachment (id=27281) [review] > masked mask as tif > > ... on safari 3.2 > If I fix the bug in PlatformContextSkia when I run your test case I hit a divide by zero error in SkPMColorToColor in SkiaUtils.cpp. This is likely happening because getImageData doesn't fix up the alpha before extracting a value. I'll attach a patch shortly that fixes PlatformContextSkia. Created attachment 27298 [details]
Temporary patch to PlatfromContextSkia to avoid crash
This patch fixes a crash in PlatformContextSkia if beginLayerClippedToImage is called without a subsequent restore. This patch is finicky though, and don't consider it a final patch. It's only useful to see the the divide by zero I just mentioned.
Created attachment 27319 [details]
Fixes three bugs introduced in first patch
This fixes three bugs found in the first patch:
. When a new layer was started clipped to an image we used the
assignment operator to copy the SkBitmap. If the SkBitmap owns it's
pixels, this is not the right thing to do. Instead we need to create
a copy of the image.
. State holds an SkBitmap by value. State's copy constructor does a
memcpy. This is confusing and subtle, I've converted to use a member
initializer list which I think is clearer and less error prone.
. When creating a new layer there is no need to copy the clip image.
Comment on attachment 27319 [details]
Fixes three bugs introduced in first patch
I don't think this is the patch you meant to attach.
Created attachment 27320 [details]
Fixes three bugs introduced in first patch (the right patch this time)
Yes, you are absolutely right. Sorry about that.
Eric, can you look at this again? Comment on attachment 27320 [details]
Fixes three bugs introduced in first patch (the right patch this time)
Looks sane enough. Sad we're not using smart pointers here and have to do our own "safeRef()" calls.
Landed as http://trac.webkit.org/changeset/40689. *** Bug 23555 has been marked as a duplicate of this bug. *** |