Bug 23332

Summary: Fix some skia pattern creation issues
Product: WebKit Reporter: Michael Moss <mmoss>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
URL: http://codereview.chromium.org/17365
Attachments:
Description Flags
PatternSkia fix
eric: review-
restyled fix eric: review+

Michael Moss
Reported 2009-01-14 14:55:45 PST
Upstreaming a Chromium fix. Workaround a skia limitation on repeated patterns, and remove extra transformations applied to pattern.
Attachments
PatternSkia fix (3.54 KB, patch)
2009-01-14 15:00 PST, Michael Moss 		eric: review-
DetailsFormatted DiffDiff
restyled fix (3.46 KB, patch)
2009-01-14 16:44 PST, Michael Moss 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Moss
Comment 1 2009-01-14 15:00:46 PST
Created attachment 26732 [details] PatternSkia fix
Eric Seidel (no email)
Comment 2 2009-01-14 16:05:29 PST
Comment on attachment 26732 [details] PatternSkia fix WebKit style doesn't put {} around single-line if clauses. However in this case, it seems an early-return would be more appropriate. if normal repeating case can early-return a shader, and then no else is needed after the return for the more complicated case (thus not indenting an extra 4 spaces). Also, WebKit uses FIXME instead of TODO with no username attribution. WebKit also doesn't wrap to 80cols like Google does. I'm surprised bm2.eraseARGB(0x00, 0x00, 0x00, 0x00); doesn't take a color argument instead. WebKit's style is a bit strange (probably less so than Google's though), but at least it's documented: http://webkit.org/coding/coding-style.html r- for the style issues. Otherwise looks fine.
Michael Moss
Comment 3 2009-01-14 16:44:01 PST
Created attachment 26737 [details] restyled fix
Eric Seidel (no email)
Comment 4 2009-01-14 16:49:04 PST
Comment on attachment 26737 [details] restyled fix No need to hold the shader in a local variable. The changelog should also have a link back to this bug. I'll fix when landing.
Eric Seidel (no email)
Comment 5 2009-01-14 16:55:28 PST
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/platform/graphics/skia/PatternSkia.cpp Committed r39921
Note You need to log in before you can comment on or make changes to this bug.