Bug 55506 - [Chromium] Canvas shadow is not working with drawImage
Summary: [Chromium] Canvas shadow is not working with drawImage
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Justin Novosad
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-01 14:08 PST by Justin Novosad
Modified: 2011-03-18 12:33 PDT (History)
7 users (show)

See Also:


Attachments
Patching the skia bindings to resolve shadow rendering issues in Chromium. This patch also resolves bug 55410 (2.68 KB, patch)
2011-03-01 14:29 PST, Justin Novosad
no flags Details | Formatted Diff | Diff
Patching the skia bindings to resolve shadow rendering issues in Chromium. This patch also resolves bug 55410 (2.70 KB, patch)
2011-03-02 08:18 PST, Justin Novosad
no flags Details | Formatted Diff | Diff
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. (9.65 KB, patch)
2011-03-17 08:23 PDT, Justin Novosad
kbr: review-
Details | Formatted Diff | Diff
Fixed ChangeLog entries (9.74 KB, patch)
2011-03-18 07:04 PDT, Justin Novosad
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Novosad 2011-03-01 14:08:15 PST
shadows behind images fail to render in 2D canvas in Chromium.

Associated Chromium bug: http://code.google.com/p/chromium/issues/detail?id=11153
Comment 1 Justin Novosad 2011-03-01 14:29:45 PST
Created attachment 84294 [details]
Patching the skia bindings to resolve shadow rendering issues in Chromium.  This patch also resolves bug 55410
Comment 2 WebKit Review Bot 2011-03-01 14:32:31 PST
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.
Comment 3 Justin Novosad 2011-03-02 08:18:09 PST
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
Comment 4 Kenneth Russell 2011-03-07 13:07:26 PST
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.
Comment 5 WebKit Commit Bot 2011-03-07 17:37:16 PST
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>
Comment 6 WebKit Commit Bot 2011-03-07 17:37:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Fumitoshi Ukai 2011-03-07 18:13:45 PST
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
Comment 8 Justin Novosad 2011-03-17 08:23:12 PDT
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.
Comment 9 Kenneth Russell 2011-03-17 14:10:30 PDT
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.
Comment 10 Justin Novosad 2011-03-18 07:04:30 PDT
Created attachment 86161 [details]
Fixed ChangeLog entries
Comment 11 Kenneth Russell 2011-03-18 09:49:38 PDT
Comment on attachment 86161 [details]
Fixed ChangeLog entries

Thanks for fixing those. r=me
Comment 12 WebKit Commit Bot 2011-03-18 11:14:33 PDT
Comment on attachment 86161 [details]
Fixed ChangeLog entries

Clearing flags on attachment: 86161

Committed r81489: <http://trac.webkit.org/changeset/81489>
Comment 13 WebKit Commit Bot 2011-03-18 11:14:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 WebKit Commit Bot 2011-03-18 11:52:38 PDT
The commit-queue encountered the following flaky tests while processing attachment 86161 [details]:

webarchive/test-link-rel-icon.html bug 56663 (author: ddkilzer@webkit.org)
The commit-queue is continuing to process your patch.
Comment 15 WebKit Review Bot 2011-03-18 12:33:12 PDT
http://trac.webkit.org/changeset/81489 might have broken Windows Release (Build) and Windows Debug (Build)