Bug 78527

Summary: [chromium] CSS Filters on accelerated layers: Brightness and Contrast don't work, Saturation is wrong
Product: WebKit Reporter: Stephen White <senorblanco>
Component: CSSAssignee: Stephen White <senorblanco>
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, jamesr, kbr, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
unskip the css3/filters layout tests kbr: review+

Stephen White
Reported 2012-02-13 13:46:47 PST
The Brightness and Contrast filters are unimplemented on accelerated layers. The saturation filter is wrong (colours are inverted -- see css3/filters/effect-saturate-hw.html).
Attachments
Patch (3.75 KB, patch)
2012-02-13 13:53 PST, Stephen White 		no flags
DetailsFormatted DiffDiff
unskip the css3/filters layout tests (4.98 KB, patch)
2012-02-13 14:00 PST, Stephen White 		kbr: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Stephen White
Comment 1 2012-02-13 13:53:21 PST
Created attachment 126824 [details] Patch
Kenneth Russell
Comment 2 2012-02-13 13:58:12 PST
Comment on attachment 126824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126824&action=review Looks good to me though I'm no expert on the math. Couple of minor nits. > Source/WebCore/platform/graphics/chromium/cc/CCRenderSurfaceFilters.cpp:43 > + memset(matrix, 0, 20 * sizeof(SkScalar)); Instead of "20 * sizeof(SkScalar)" how about just sizeof(matrix)? > Source/WebCore/platform/graphics/chromium/cc/CCRenderSurfaceFilters.cpp:58 > + memset(matrix, 0, 20 * sizeof(SkScalar)); Same here.
Stephen White
Comment 3 2012-02-13 14:00:13 PST
(In reply to comment #2) > (From update of attachment 126824 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126824&action=review > > Looks good to me though I'm no expert on the math. Couple of minor nits. > > > Source/WebCore/platform/graphics/chromium/cc/CCRenderSurfaceFilters.cpp:43 > > + memset(matrix, 0, 20 * sizeof(SkScalar)); > > Instead of "20 * sizeof(SkScalar)" how about just sizeof(matrix)? Doesn't work for array parameters, since they're actually pointers. You get sizeof(SkScalar*). (I actually wrote it this way first, and only a clang warning saved me!) > > Source/WebCore/platform/graphics/chromium/cc/CCRenderSurfaceFilters.cpp:58 > > + memset(matrix, 0, 20 * sizeof(SkScalar)); > > Same here.
Stephen White
Comment 4 2012-02-13 14:00:39 PST
Created attachment 126828 [details] unskip the css3/filters layout tests
Kenneth Russell
Comment 5 2012-02-13 14:07:57 PST
Comment on attachment 126824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126824&action=review >>> Source/WebCore/platform/graphics/chromium/cc/CCRenderSurfaceFilters.cpp:43 >>> + memset(matrix, 0, 20 * sizeof(SkScalar)); >> >> Instead of "20 * sizeof(SkScalar)" how about just sizeof(matrix)? > > Doesn't work for array parameters, since they're actually pointers. You get sizeof(SkScalar*). (I actually wrote it this way first, and only a clang warning saved me!) Huh, I didn't know that!
Kenneth Russell
Comment 6 2012-02-13 14:08:16 PST
Comment on attachment 126828 [details] unskip the css3/filters layout tests Still looks good.
Stephen White
Comment 7 2012-02-13 14:23:11 PST
Committed r107611: <http://trac.webkit.org/changeset/107611>
Note You need to log in before you can comment on or make changes to this bug.