From the spec at http://dev.w3.org/html5/2dcontext/#dom-context-2d-shadowblur : "The shadowBlur attribute specifies the level of the blurring effect. (The units do not map to coordinate space units, and are not affected by the current transformation matrix.)" Currently, blurring is applied in the untransformed layerImage. When layerImage is transformed and displayed on screen, the blur radius will also be affected by the scaling factor. The solution is dividing the blur radius by the balanced (X and Y) scaling factor.
Created attachment 76734 [details] Patch
Comment on attachment 76734 [details] Patch 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.
(In reply to comment #2) > Seems that it is possible to refactor this to be usable for Cairo as well. I'll wait until bug 50422 is resolved for good.
Created attachment 76950 [details] Untested with GTK
Comment on attachment 76950 [details] Untested with GTK View in context: https://bugs.webkit.org/attachment.cgi?id=76950&action=review Very nice. A couple nits below. > WebCore/platform/graphics/ContextShadow.cpp:177 > + if (!transform.isIdentity()) { I think this is a good place to use an early return. > WebCore/platform/graphics/cairo/ContextShadowCairo.cpp:97 > double x1, x2, y1, y2; > cairo_clip_extents(context, &x1, &y1, &x2, &y2); > + adjustBlurDistance(context); Maybe move the third line above the first two lines? The clip extents are only used for the call to calculateLayerBoundingRect, so this is in the middle of another "sentence" so to speak.
Comment on attachment 76950 [details] Untested with GTK View in context: https://bugs.webkit.org/attachment.cgi?id=76950&action=review > LayoutTests/ChangeLog:5 > + [Qt] Blur distance should not be affected by scaling s/scaling/transformation
Created attachment 76951 [details] Suggestions in comment #5
Created attachment 76952 [details] Correction from comment #7
Comment on attachment 76952 [details] Correction from comment #7 Looks good. re=me
Comment on attachment 76952 [details] Correction from comment #7 Clearing flags on attachment: 76952 Committed r74328: <http://trac.webkit.org/changeset/74328>
All reviewed patches have been landed. Closing bug.
The commit-queue encountered the following flaky tests while processing attachment 76952 [details]: http/tests/navigation/ping-cross-origin-from-https.html bug 51314 (author: japhet@chromium.org) The commit-queue is continuing to process your patch.