[webkit-reviews] review denied: [Bug 51161] [Qt] shadowBlur should not be affected by scaling : [Attachment 76734] Patch

[bugzilla-daemon at webkit.org]

Ariya Hidayat <ariya.hidayat at gmail.com> has denied Helder Correia
<helder at sencha.com>'s request for review:
Bug 51161: [Qt] shadowBlur should not be affected by scaling
https://bugs.webkit.org/show_bug.cgi?id=51161

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

------- Additional Comments from Ariya Hidayat <ariya.hidayat at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=76734&action=review

Minor issues, otherwise LGTM.

> WebCore/ChangeLog:5
> +	   [Qt] shadowBlur should not be affected by scaling

Although shadowBlur is the term used by the spec, I guess it's clearer if here
we put e.g. "Shadow distance" instead.

> WebCore/platform/graphics/qt/ContextShadowQt.cpp:119
> +    // Adjust blur if we're scaling, since the radius must not be affected
by transformations.

Seems that it is possible to refactor this to be usable for Cairo as well.

> WebCore/platform/graphics/qt/ContextShadowQt.cpp:142
> +	   // Calculate X axis scale factor.
> +	   const float xAxisHorizontalChange = transformedXAxisUnit.x() -
transformedOrigin.x();
> +	   const float xAxisVerticalChange = transformedXAxisUnit.y() -
transformedOrigin.y();
> +	   const float xAxisScale = sqrtf(xAxisHorizontalChange *
xAxisHorizontalChange
> +					  + xAxisVerticalChange *
xAxisVerticalChange);
> +
> +	   // Calculate Y axis scale factor.
> +	   const float yAxisHorizontalChange = transformedYAxisUnit.x() -
transformedOrigin.x();
> +	   const float yAxisVerticalChange = transformedYAxisUnit.y() -
transformedOrigin.y();
> +	   const float yAxisScale = sqrtf(yAxisHorizontalChange *
yAxisHorizontalChange
> +					  + yAxisVerticalChange *
yAxisVerticalChange);

This could be simplified using QVector2D.