We are not currently the tiled rendering in the inset shadow blur for the ports using cairo backend.
Created attachment 207444 [details] Proposed patch
Comment on attachment 207444 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=207444&action=review Nice. Just a couple suggestions before landing. > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1054 > + if (!roundedHoleRect.radii().isZero()) > + path.addRoundedRect(roundedHoleRect); > + else > + path.addRect(roundedHoleRect.rect()); I wonder if this optimization should go into Path.cpp eventually? > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1061 > + setFillRule(RULE_EVENODD); > + setFillColor(color, colorSpace); I think that instead of setting the fill rule and color for the GraphicsContext you should just set them for the Cairo context. That way you can use cairo_save/cairo_restore, instead of oldFillRule and oldFillColor. > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1065 > + ShadowBlur& shadow = platformContext()->shadowBlur(); > + shadow.drawInsetShadow(this, rect, roundedHoleRect.rect(), roundedHoleRect.radii()); I think you can avoid the temporary and move this code to the very start of the method after the early return.
(In reply to comment #2) > (From update of attachment 207444 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207444&action=review > > Nice. Just a couple suggestions before landing. > > > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1054 > > + if (!roundedHoleRect.radii().isZero()) > > + path.addRoundedRect(roundedHoleRect); > > + else > > + path.addRect(roundedHoleRect.rect()); > > I wonder if this optimization should go into Path.cpp eventually? > Good point, not sure, I can check it in a follow-up patch. > > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1061 > > + setFillRule(RULE_EVENODD); > > + setFillColor(color, colorSpace); > > I think that instead of setting the fill rule and color for the GraphicsContext you should just set them for the Cairo context. That way you can use cairo_save/cairo_restore, instead of oldFillRule and oldFillColor. > Yep, it sounds better option. > > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1065 > > + ShadowBlur& shadow = platformContext()->shadowBlur(); > > + shadow.drawInsetShadow(this, rect, roundedHoleRect.rect(), roundedHoleRect.radii()); > > I think you can avoid the temporary and move this code to the very start of the method after the early return. Ok, Thanks for the review.
Created attachment 207927 [details] Proposed patch
Comment on attachment 207927 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=207927&action=review > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1056 > + if (this->mustUseShadowBlur()) > + platformContext()->shadowBlur().drawInsetShadow(this, rect, roundedHoleRect.rect(), roundedHoleRect.radii()); You could even move this above the path manipulation, I think.
(In reply to comment #5) > (From update of attachment 207927 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207927&action=review > > > Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp:1056 > > + if (this->mustUseShadowBlur()) > > + platformContext()->shadowBlur().drawInsetShadow(this, rect, roundedHoleRect.rect(), roundedHoleRect.radii()); > > You could even move this above the path manipulation, I think. Yeah, I've modified the patch before landing, thanks. http://trac.webkit.org/changeset/142608