Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 658603002: 2D canvas: fix shadow rendering with compositing modes that require temp layers (Closed)

Created:
6 years, 2 months ago by Justin Novosad
Modified:
6 years, 2 months ago
Reviewers:
Stephen White, jbroman
CC:
blink-reviews, krit, pdr+graphicswatchlist_chromium.org, blink-reviews-html_chromium.org, jbroman, mkwst+moarreviews_chromium.org, danakj, dglazkov+blink, Rik, Stephen Chennney, aandrey+blink_chromium.org, rwlbuis
Project:
blink
Visibility:
Public.

Description

2D canvas: fix shadow rendering with compositing modes that require temp layers For compositng mode that are known as "full canvas compositing modes", i.e. modes that affect pixels outside of the primitive, we need to draw to a temporary buffer, and composite it back to the canvas. In cases where shadows are being painted, it is important to composite the shadow independently of the foreground to produse spec-compliant behavior. BUG=411316 TEST=layout test fast/canvas/canvas-shadow-source-in.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=183774

Patch Set 1 #

Patch Set 2 : rehash #

Total comments: 3

Patch Set 3 : corrections+rebase #

Patch Set 4 : remove spurious new line #

Patch Set 5 : fix rebase #

Total comments: 2

Patch Set 6 : comment fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -8 lines) Patch
A LayoutTests/fast/canvas/canvas-shadow-source-in.html View 1 chunk +16 lines, -0 lines 0 comments Download
A LayoutTests/fast/canvas/canvas-shadow-source-in-expected.html View 1 chunk +14 lines, -0 lines 0 comments Download
M Source/core/html/canvas/CanvasRenderingContext2D.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/canvas/CanvasRenderingContext2D.cpp View 1 2 3 4 4 chunks +20 lines, -3 lines 0 comments Download
M Source/platform/graphics/GraphicsContext.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/platform/graphics/GraphicsContext.cpp View 1 2 3 4 5 1 chunk +10 lines, -3 lines 0 comments Download
M Source/platform/graphics/GraphicsTypes.h View 1 2 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (5 generated)
Justin Novosad
6 years, 2 months ago (2014-10-14 19:57:56 UTC) #2
Stephen White
https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsContext.cpp File Source/platform/graphics/GraphicsContext.cpp (right): https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsContext.cpp#newcode310 Source/platform/graphics/GraphicsContext.cpp:310: // shadow only, but there is no shadow: use ...
6 years, 2 months ago (2014-10-14 21:02:12 UTC) #3
jbroman
https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsContext.cpp File Source/platform/graphics/GraphicsContext.cpp (right): https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsContext.cpp#newcode326 Source/platform/graphics/GraphicsContext.cpp:326: nit: spurious newline https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsTypes.h File Source/platform/graphics/GraphicsTypes.h (right): https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsTypes.h#newcode78 Source/platform/graphics/GraphicsTypes.h:78: ...
6 years, 2 months ago (2014-10-14 21:09:48 UTC) #5
Justin Novosad
New Patch
6 years, 2 months ago (2014-10-15 15:46:45 UTC) #6
Justin Novosad
On 2014/10/14 21:02:12, Stephen White wrote: > https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsContext.cpp > File Source/platform/graphics/GraphicsContext.cpp (right): > > https://codereview.chromium.org/658603002/diff/20001/Source/platform/graphics/GraphicsContext.cpp#newcode310 ...
6 years, 2 months ago (2014-10-15 15:52:59 UTC) #7
Stephen White
LGTM https://codereview.chromium.org/658603002/diff/80001/LayoutTests/fast/canvas/canvas-shadow-source-in-expected.html File LayoutTests/fast/canvas/canvas-shadow-source-in-expected.html (right): https://codereview.chromium.org/658603002/diff/80001/LayoutTests/fast/canvas/canvas-shadow-source-in-expected.html#newcode1 LayoutTests/fast/canvas/canvas-shadow-source-in-expected.html:1: <!DOCTYPE html> Thanks for using a reftest, BTW! ...
6 years, 2 months ago (2014-10-15 17:15:30 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/658603002/100001
6 years, 2 months ago (2014-10-15 17:56:14 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/31940)
6 years, 2 months ago (2014-10-15 21:48:21 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/658603002/100001
6 years, 2 months ago (2014-10-15 22:10:38 UTC) #14
commit-bot: I haz the power
6 years, 2 months ago (2014-10-16 00:17:43 UTC) #15
Message was sent while issue was closed. 
Committed patchset #6 (id:100001) as 183774

Powered by Google App Engine
This is Rietveld 408576698