Bug 25956 – Canvas' composition is incorrect when alpha=0

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Shinichiro Hamaji]

In GraphicsContextCG and GraphicsContextSkia, there are optimization path for alpha=0 case, with which WebKit renders nothing. However, this behavior is incorrect for some composite modes (e.g., 'copy').

See also this bug: https://bugs.webkit.org/show_bug.cgi?id=25417

[Shinichiro Hamaji]

Created attachment 30577 [details]
Remove optimization path alpha=0 case from GraphicContext(CG|Skia)

 .../canvas/canvas-composite-alpha-expected.txt     |  202 +++++++++++++++++++-
 .../fast/canvas/canvas-composite-alpha.html        |   89 ++++++++--
 .../canvas/canvas-composite-alpha-expected.txt     |  181 ------------------
 WebCore/platform/graphics/cg/GraphicsContextCG.cpp |   42 ++---
 .../platform/graphics/skia/GraphicsContextSkia.cpp |   27 +---
 5 files changed, 290 insertions(+), 251 deletions(-)

[Shinichiro Hamaji]

Comment on attachment 30577 [details]
Remove optimization path alpha=0 case from GraphicContext(CG|Skia)

This is the reason why I introduced failing test cases in the previous patch:

https://bugs.webkit.org/show_bug.cgi?id=25417

This optimization was introduced 3 years ago. At this time, fillRect() always calls fillRectSourceOver() so that this optimization was OK. So, I guess the current behavior (when alpha is zero, all fillrect operations are skipped even if the composite mode is 'copy') is not intentional.

http://trac.webkit.org/changeset/13992/trunk/WebCore/platform/mac/GraphicsContextMac.mm

Fortunately, Skia has the similar optimization path inside Skia and Skia checks the composition mode if it is safe to do this optimization. I don't know if CG has similar logic, but anyway I think this patch won't reduce performance significantly as alpha=0 cases may be rare.

I also made error messages of the layout test case better, and added test case for path and fill.

[Oliver Hunt]

Comment on attachment 30577 [details]
Remove optimization path alpha=0 case from GraphicContext(CG|Skia)

r=me

[Shinichiro Hamaji]

Created attachment 30578 [details]
Remove optimization path alpha=0 case from GraphicContext(CG|Skia)

 LayoutTests/ChangeLog                              |   12 ++
 .../canvas/canvas-composite-alpha-expected.txt     |  202 +++++++++++++++++++-
 .../fast/canvas/canvas-composite-alpha.html        |   89 ++++++++--
 .../canvas/canvas-composite-alpha-expected.txt     |  181 ------------------
 WebCore/ChangeLog                                  |   26 +++
 WebCore/platform/graphics/cg/GraphicsContextCG.cpp |   42 ++---
 .../platform/graphics/skia/GraphicsContextSkia.cpp |   27 +---
 7 files changed, 328 insertions(+), 251 deletions(-)

[Shinichiro Hamaji]

Comment on attachment 30577 [details]
Remove optimization path alpha=0 case from GraphicContext(CG|Skia)

Sorry, I forgot to add ChangeLog. The difference between this patch and the next should be only ChangeLogs.

[Eric Seidel (no email)]

	M	LayoutTests/ChangeLog
	M	LayoutTests/fast/canvas/canvas-composite-alpha-expected.txt
	M	LayoutTests/fast/canvas/canvas-composite-alpha.html
	M	LayoutTests/platform/gtk/fast/canvas/canvas-composite-alpha-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/platform/graphics/cg/GraphicsContextCG.cpp
	M	WebCore/platform/graphics/skia/GraphicsContextSkia.cpp
Committed r44041