[Chromium] Canvas shadow is not working with drawImage

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Justin Novosad]

shadows behind images fail to render in 2D canvas in Chromium.

Associated Chromium bug: http://code.google.com/p/chromium/issues/detail?id=11153

[Justin Novosad]

Created attachment 84294 [details]
Patching the skia bindings to resolve shadow rendering issues in Chromium.  This patch also resolves bug 55410

[WebKit Review Bot]

Attachment 84294 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 3 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Justin Novosad]

Created attachment 84421 [details]
Patching the skia bindings to resolve shadow rendering issues in Chromium. This patch also resolves bug 55410

vs. patch 84294: Style corrections

[Kenneth Russell]

Comment on attachment 84421 [details]
Patching the skia bindings to resolve shadow rendering issues in Chromium. This patch also resolves bug 55410

Looks good to me.

[WebKit Commit Bot]

Comment on attachment 84421 [details]
Patching the skia bindings to resolve shadow rendering issues in Chromium. This patch also resolves bug 55410

Clearing flags on attachment: 84421

Committed r80514: <http://trac.webkit.org/changeset/80514>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Fumitoshi Ukai]

It caused assertion failures and layout test failures, so I rolled out this by http://trac.webkit.org/changeset/80519

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&group=%40ToT%20-%20chromium.org&tests=canvas%2Fphilip%2Ftests%2F2d.shadow.alpha.2.html%2Cfast%2Fcanvas%2Fcanvas-fillRect-shadow.html%2Cfast%2Fcanvas%2Fcanvas-scale-fillPath-shadow.html%2Cfast%2Fcanvas%2Fcanvas-scale-fillRect-shadow.html%2Cfast%2Fcanvas%2Fcanvas-scale-shadowBlur.html%2Cfast%2Fcanvas%2Fcanvas-scale-strokePath-shadow.html%2Cfast%2Fcanvas%2Fcanvas-transforms-fillRect-shadow.html%2Cfast%2Fblockflow%2Fbox-shadow-horizontal-bt.html%2Cfast%2Fblockflow%2Fbox-shadow-vertical-lr.html%2Cfast%2Fblockflow%2Fbox-shadow-vertical-rl.html%2Cfast%2Fblockflow%2Fenglish-lr-text.html%2Cfast%2Fborders%2Fborder-radius-split-inline.html%2Cfast%2Fbox-shadow%2Fbasic-shadows.html%2Cfast%2Fbox-shadow%2Fborder-radius-big.html%2Cfast%2Fbox-shadow%2Fbox-shadow-radius.html%2Cfast%2Fbox-shadow%2Fbox-shadow-transformed.html%2Cfast%2Fbox-shadow%2Finset-box-shadow-radius.html%2Cfast%2Fbox-shadow%2Finset-box-shadows.html%2Cfast%2Fbox-shadow%2Finset.html%2Cfast%2Fbox-shadow%2Fshadow-buffer-partial.html%2Cfast%2Fbox-shadow%2Fshadow-tiling-artifact.html%2Cfast%2Fbox-shadow%2Fspread-multiple-normal.html%2Cfast%2Fbox-shadow%2Fspread.html%2Cfast%2Fcanvas%2Fshadow-offset-4.html%2Cfast%2Fcanvas%2Fshadow-offset-5.html%2Cfast%2Fcanvas%2Fshadow-offset-6.html%2Cfast%2Fcanvas%2Fshadow-offset-7.html%2Cfast%2Fmulticol%2Fshadow-breaking.html%2Cfast%2Frepaint%2Fbox-shadow-h.html%2Cfast%2Frepaint%2Fbox-shadow-v.html%2Cfast%2Frepaint%2Fmoving-shadow-on-container.html%2Cfast%2Frepaint%2Fmoving-shadow-on-path.html%2Cfast%2Frepaint%2Fshadow-multiple-horizontal.html%2Cfast%2Frepaint%2Fshadow-multiple-strict-horizontal.html%2Cfast%2Frepaint%2Fshadow-multiple-strict-vertical.html%2Cfast%2Frepaint%2Fshadow-multiple-vertical.html%2Cfast%2Frepaint%2Ftext-shadow-horizontal.html%2Cfast%2Frepaint%2Ftext-shadow.html%2Cfast%2Frepaint%2Ftransform-replaced-shadows.html%2Cfast%2Ftext%2Fshadow-no-blur.html%2Cfast%2Ftext%2Fshadow-translucent-fill.html%2Cfast%2Ftext%2Fstroking-decorations.html%2Cfast%2Ftext%2Fstroking.html%2Cfast%2Ftransforms%2Fshadows.html%2Csvg%2Fcss%2Farrow-with-shadow.svg%2Csvg%2Fcss%2Fcomposite-shadow-example.html%2Csvg%2Fcss%2Fcomposite-shadow-text.svg%2Csvg%2Fcss%2Fcomposite-shadow-with-opacity.html%2Csvg%2Fcss%2Fgroup-with-shadow.svg%2Csvg%2Fcss%2Fshadow-changes.svg%2Csvg%2Fcss%2Fshadow-with-large-radius.svg%2Csvg%2Fcss%2Fshadow-with-negative-offset.svg%2Csvg%2Fcss%2Fstars-with-shadow.html%2Csvg%2Fcss%2Ftext-gradient-shadow.svg%2Csvg%2Fcss%2Ftext-shadow-multiple.xhtml%2Ctransitions%2Fsvg-text-shadow-transition.html%2Cjquery%2Feffects.html

[Justin Novosad]

Created attachment 86058 [details]
Code change is the same as previous patch. Changes were submitted to Skia in the mean time to prevent chromium canary test failures.  Tests that need re-baselineing were marked in test expectations.

[Kenneth Russell]

Comment on attachment 86058 [details]
Code change is the same as previous patch. Changes were submitted to Skia in the mean time to prevent chromium canary test failures.  Tests that need re-baselineing were marked in test expectations.

View in context: https://bugs.webkit.org/attachment.cgi?id=86058&action=review

The code changes look fine but the ChangeLogs need to be updated.

> Source/WebCore/ChangeLog:10
> +        Fixes shadow blur quality and color. Also fixes shadows behind
> +        bitmaps. Affects Chromium win/linux. Fixes the following bugs:
> +        https://bugs.webkit.org/show_bug.cgi?id=50112
> +        https://bugs.webkit.org/show_bug.cgi?id=51989
> +        https://bugs.webkit.org/show_bug.cgi?id=55506
> +        https://bugs.webkit.org/show_bug.cgi?id=55410

Please update this ChangeLog into the following format:

[Chromium] Canvas shadow is not working with drawImage
https://bugs.webkit.org/show_bug.cgi?id=55506

Followed by the extended comment including the additional bug URLs. The commit queue will pick up the first one in the ChangeLog entry (I'm pretty sure), so you want it to match the bug ID the patch is attached to.

> LayoutTests/ChangeLog:10
> +        Fixes shadow blur quality and color. Also fixes shadows behind                  bitmaps. Affects Chromium win/linux. Fixes the following bugs:
> +        https://bugs.webkit.org/show_bug.cgi?id=50112
> +        https://bugs.webkit.org/show_bug.cgi?id=51989
> +        https://bugs.webkit.org/show_bug.cgi?id=55506
> +        https://bugs.webkit.org/show_bug.cgi?id=55410
> +        No tests were added, impact is already covered by multiple layout tests.

Same comment about ChangeLog formatting.

[Justin Novosad]

Created attachment 86161 [details]
Fixed ChangeLog entries

[Kenneth Russell]

Comment on attachment 86161 [details]
Fixed ChangeLog entries

Thanks for fixing those. r=me

[WebKit Commit Bot]

Comment on attachment 86161 [details]
Fixed ChangeLog entries

Clearing flags on attachment: 86161

Committed r81489: <http://trac.webkit.org/changeset/81489>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 86161 [details]:

webarchive/test-link-rel-icon.html bug 56663 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Review Bot]

http://trac.webkit.org/changeset/81489 might have broken Windows Release (Build) and Windows Debug (Build)