Masks are currently disabled, as the concept was flawed. Patterns still worked but the implementation is very hack, relying on NSGraphicsContext graphicsPort trickery (in platform/mac/GraphicsContextMac.mm). Fix that properly by adding off-screen rendering functionality to GraphicsContext, and rewrite SVGPatternElement / SVGMaskElement to use the new static GraphicsContext::createImageContext function. Attaching patch soon.
Created attachment 11903 [details] Initial patch Fixes all masking regressions, and speeds up gradients-on-stroke of text a lot.
Comment on attachment 11903 [details] Initial patch Looks good. Tell me just before you land :D
Created attachment 11913 [details] Updated patch Updated patch fixing all known problems. Uses optimal sized images for gradient-on-stroke of text now.
Comment on attachment 11913 [details] Updated patch Ok, I looked through this briefly. A few thoughts. 1. I'm not sure that GraphicsContext should carry a size. By doing so, you're sorta making it both an image and a context. Contexts (at least for CG) are infinite planes which know how to receive drawing commands. A context made form an image just has an implicit clip. Better would probably be a make a separate ImageBuffer object, which you could in turn make a GraphicsContext from. You could then make the CGImage directly from the ImageBuffer object (the ImageBuffer could enve know how to cache a CGImage pointer, so you don't have to recreate one every time during the pattern callback -- which is likely slow, since it's at least one malloc, and possibly a buffer copy.) 2. It feels weird to pass around GraphicsContext* as images. Again, I think an ImageBuffer is in order here. "drawMaskerContext" logic should just be generic logic on RenderObject. Should be able to say "drawSubTree()" or something, possibly passing it a cliprect. And it would just return you an ImageBuffer* or something. Maybe you have to pass it the ImageBuffer, that might make more sense. You should probably chat with Hyatt or Maciej (someone who is more current with the WebKit project than I). All in all this is a *great* start. I think it could be done a bit cleaner though, and doing it in a really clean way will be a huge benifit for the rest of the project, as this code can be re-used all over the place (mask, pattern, filter, <img> with SVG are just a few uses which pop to mind).
Comment on attachment 11913 [details] Updated patch Looks sane to me, anyone else have any comments?
(In reply to comment #5) > (From update of attachment 11913 [details] [edit]) > Looks sane to me, anyone else have any comments? I'm preparing a new patch, adressing Eric's comments.
(In reply to comment #4) > (From update of attachment 11913 [details] [edit]) > Ok, I looked through this briefly. A few thoughts. > > 1. I'm not sure that GraphicsContext should carry a size. By doing so, you're > sorta making it both an image and a context. Contexts (at least for CG) are > infinite planes which know how to receive drawing commands. A context made > form an image just has an implicit clip. Better would probably be a make a > separate ImageBuffer object, which you could in turn make a GraphicsContext > from. You could then make the CGImage directly from the ImageBuffer object > (the ImageBuffer could enve know how to cache a CGImage pointer, so you don't > have to recreate one every time during the pattern callback -- which is likely > slow, since it's at least one malloc, and possibly a buffer copy.) Oh good catch Eric, regarding the patternCallback() thing! This imageContextSize() stuff was really unclean, it's gone now and a new "static ImageBuffer* createImageBuffer(const IntSize&, bool grayScale)" has been added to GraphicsContext, this time w/O SVG_SUPPORT blocks, as it's really generic enough now. > 2. It feels weird to pass around GraphicsContext* as images. Again, I think > an ImageBuffer is in order here. Definately. All fixed by passing around ImageBuffer pointers now. > "drawMaskerContext" logic should just be generic logic on RenderObject. Should > be able to say "drawSubTree()" or something, possibly passing it a cliprect. > And it would just return you an ImageBuffer* or something. Maybe you have to > pass it the ImageBuffer, that might make more sense. Sounds nice, thining about that now. > You should probably chat with Hyatt or Maciej (someone who is more current with > the WebKit project than I). All in all this is a *great* start. I think it > could be done a bit cleaner though, and doing it in a really clean way will be > a huge benifit for the rest of the project, as this code can be re-used all > over the place (mask, pattern, filter, <img> with SVG are just a few uses which > pop to mind). Absolutely agreeing, 4am was a bad time to think about design :-) Keep tuned... patch coming soon
Created attachment 11915 [details] Updated patch II Incorporated all comments. Still no new regressions, all masking related stuff fixed. Heavy speedup for gradient-on-stroke.
Created attachment 11916 [details] Final patch This fixes all comments, from IRC review with Sam.
Landed in r18315.