The attached testcase shows a problem where, when the mask, which is a canvas, changes, the element is not redrawn. The testcase has an image with style: -webkit-mask: -webkit-canvas(canvas1); and an onmousemove() handler that redraws the canvas using the mouse coordinate to determine the size of the gradient. What I see: When the page loads, the image is drawn with the gradient mask, but never changes thereafter. What I expect: The image + mask is redrawn whenever the canvas is udpated.
Created attachment 22166 [details] Testcase
<rdar://problem/6061428>
Created attachment 22167 [details] Patch to repaint() when the mask image changes First cut at a patch. Not sure how to do a non-manual testcase for this.
Comment on attachment 22167 [details] Patch to repaint() when the mask image changes (1) Adding maskBoxImage to RenderBox's first check is good. (2) Adding maskImage is bad. We want precise invalidation like we have with backgrounds. See the layer walk at the end of the function. You should make a little static helper function that walks layers to do the invalidation. Then you can call it on the background layers and on the mask layers. (2) The patch to RenderImage should just call the base class if you have a mask. So changing if (hasBoxDecorations()) to if (hasBoxDecorations() || hasMask()) should be sufficient.
Created attachment 22191 [details] Patch addressing review comments > (1) Adding maskBoxImage to RenderBox's first check is good. I didn't add that, I just unwrapped the line. > (2) Adding maskImage is bad. We want precise invalidation like we have with > backgrounds. See the layer walk at the end of the function. You should make a > little static helper function that walks layers to do the invalidation. Then > you can call it on the background layers and on the mask layers. Done. Note that I had to add FillLayer::containsWrappedImage(). I added a method, repaintLayerRectsForImage(), rather than a static method, because it needs to get at the view() for invalidation. > (2) The patch to RenderImage should just call the base class if you have a > mask. So changing if (hasBoxDecorations()) to if (hasBoxDecorations() || > hasMask()) should be sufficient. Done.
Comment on attachment 22191 [details] Patch addressing review comments containsWrappedImage is unnecessary. Doesn't really buy you anything and worst case causes you to do two walks of the layers instead of one. The existing walk already checks for the image.
But I don't have a StyleImage to pass into containsImage(), just a WrappedImagePtr.
Created attachment 22193 [details] Final patch
Comment on attachment 22193 [details] Final patch r=me
Created attachment 22199 [details] Patch with testcase and changelog.
Comment on attachment 22199 [details] Patch with testcase and changelog. Transferring r= from hyatt
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog A WebCore/manual-tests/canvas-mask-redraw.html M WebCore/rendering/RenderBox.cpp M WebCore/rendering/RenderBox.h M WebCore/rendering/RenderImage.cpp M WebCore/rendering/RenderObject.cpp Committed r35101
Committed by dino in r35101