Bug 25956 - Canvas' composition is incorrect when alpha=0
Summary: Canvas' composition is incorrect when alpha=0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 25417
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-22 00:50 PDT by Shinichiro Hamaji
Modified: 2009-05-22 06:25 PDT (History)
1 user (show)

See Also:


Attachments
Remove optimization path alpha=0 case from GraphicContext(CG|Skia) (20.33 KB, patch)
2009-05-22 00:52 PDT, Shinichiro Hamaji
oliver: review+
Details | Formatted Diff | Diff
Remove optimization path alpha=0 case from GraphicContext(CG|Skia) (22.46 KB, patch)
2009-05-22 03:54 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinichiro Hamaji 2009-05-22 00:50:53 PDT
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
Comment 1 Shinichiro Hamaji 2009-05-22 00:52:54 PDT
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(-)
Comment 2 Shinichiro Hamaji 2009-05-22 00:53:29 PDT
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.
Comment 3 Oliver Hunt 2009-05-22 02:40:49 PDT
Comment on attachment 30577 [details]
Remove optimization path alpha=0 case from GraphicContext(CG|Skia)

r=me
Comment 4 Shinichiro Hamaji 2009-05-22 03:54:42 PDT
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(-)
Comment 5 Shinichiro Hamaji 2009-05-22 03:56:03 PDT
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.
Comment 6 Eric Seidel (no email) 2009-05-22 06:25:05 PDT
	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