[webkit-reviews] review denied: [Bug 22102] -webkit-box-shadow causes awful scroll/resize/redraw performance : [Attachment 77421] Patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 22102: -webkit-box-shadow causes awful scroll/resize/redraw performance
https://bugs.webkit.org/show_bug.cgi?id=22102

Attachment 77421: Patch
https://bugs.webkit.org/attachment.cgi?id=77421&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=77421&action=review

Looks great in general, but please share the code of drawRectShadow() as much
as possible with other platforms. r- because of the duplication.

> WebCore/platform/graphics/cg/ContextShadowCG.cpp:141
> +	   // FIXME: can't get at the ImageBuffer bits directly, so we have to
copy.

We have a bug report for this (not sure about the bug id right now), can you
add the link to this bug here please?

> WebCore/platform/graphics/cg/ContextShadowCG.cpp:145
> +			  IntSize(layerData->width(), layerData->height()),

ImageData has size() now, please use this.

> WebCore/platform/graphics/cg/ContextShadowCG.cpp:369
> +
> +    // Draw the bottom left corner.
> +    tileRect = FloatRect(0, shadowTemplateSize.height() - radiusTwice -
bottomLeftRadius.height(),
> +			    radiusTwice + bottomLeftRadius.width(), radiusTwice
+ bottomLeftRadius.height());
> +    destRect = tileRect;
> +    destRect.move(shadowRect.x(), shadowRect.y() + shadowedRect.height() -
shadowTemplateSize.height() + radiusTwice);
> +    graphicsContext->drawImageBuffer(m_layerImage, ColorSpaceDeviceRGB,
destRect, tileRect);

Most of 60 lines above look similar of identical to the code in
ContextShadowCairo. We don't need to share the complete function, but at least
this code should be shared. If you want you can drop this function into a
follow up patch. I'm very interested in ContexShadow for CG, since it is ideal
for a sample implementation of feDropShadow. Something I'm working on ;-)