[webkit-reviews] review denied: [Bug 23291] -webkit-box-shadow doesn't work on Qt-WebKit : [Attachment 39060] Repeat of previous patch, but with PNG files incorporated.

[bugzilla-daemon at webkit.org]

Ariya Hidayat <ariya.hidayat at trolltech.com> has denied Carol Szabo
<carol.szabo at nokia.com>'s request for review:
Bug 23291: -webkit-box-shadow doesn't work on Qt-WebKit
https://bugs.webkit.org/show_bug.cgi?id=23291

Attachment 39060: Repeat of previous patch, but with PNG files incorporated.
https://bugs.webkit.org/attachment.cgi?id=39060&action=review

------- Additional Comments from Ariya Hidayat <ariya.hidayat at trolltech.com>

> +	   Need a short description and bug URL (OOPS!)
> +	   https://bugs.webkit.org/show_bug.cgi?id=23291

Don't forget to write down something descriptive here, for both
WebCore/ChangeLog and LayoutTests/ChangeLog.

> +	   QTransform t(p->worldTransform());
> +	   p->translate(shadowSize.width(), shadowSize.height());
> +	   p->fillPath(*path, QBrush(shadowColor));
> +	   p->setWorldTransform(t);

I suggest translating back (negative offset) the painter after filling the
path. That is much faster than stashing and restoring the transformation
matrix.

> -    if (m_common->state.shadowsIgnoreTransforms) {
> -	   // Meaning that this graphics context is associated with a
CanvasRenderingContext
> -	   // We flip the height since CG and HTML5 Canvas have opposite Y axis

> -	   m_common->state.shadowSize = IntSize(size.width(), -size.height());
> -    }

I added this block because it is necessary to make the shadows working properly
inside the Canvas.
Why is this removed? Do we now take Canvas into account somewhere else?

In principle, the patch LGTM except the minor issues.

r- until the issues are solved.