Bug 119845 – Implement CSS Image filter() function

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Dirk Schulze]

Implement CSS Image filter() function from Filter Effects.

[Dirk Schulze]

Created attachment 208815 [details]
Patch for filter() image function

[Dirk Schulze]

Created attachment 208816 [details]
Patch for filter() image function

[WebKit Commit Bot]

Attachment 208816 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/filter-image/filter-image-expected.html', u'LayoutTests/fast/filter-image/filter-image.html', u'LayoutTests/fast/filter-image/parse-filter-image-expected.txt', u'LayoutTests/fast/filter-image/parse-filter-image.html', u'LayoutTests/fast/filter-image/resources/image.svg', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/GNUmakefile.list.am', u'Source/WebCore/Target.pri', u'Source/WebCore/WebCore.vcxproj/WebCore.vcxproj', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/css/CSSAllInOne.cpp', u'Source/WebCore/css/CSSCrossfadeValue.cpp', u'Source/WebCore/css/CSSFilterImageValue.cpp', u'Source/WebCore/css/CSSFilterImageValue.h', u'Source/WebCore/css/CSSImageGeneratorValue.cpp', u'Source/WebCore/css/CSSImageGeneratorValue.h', u'Source/WebCore/css/CSSParser.cpp', u'Source/WebCore/css/CSSParser.h', u'Source/WebCore/css/CSSValue.cpp', u'Source/WebCore/css/CSSValue.h', u'Source/WebCore/css/StyleResolver.cpp', u'Source/WebCore/css/StyleResolver.h', u'Source/WebCore/rendering/FilterEffectRenderer.cpp', u'Source/WebCore/rendering/FilterEffectRenderer.h']" exit_code: 1
Source/WebCore/css/CSSFilterImageValue.cpp:171:  Could not find a newline character at the end of the file.  [whitespace/ending_newline] [5]
Total errors found: 1 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dirk Schulze]

Created attachment 208818 [details]
Patch for filter() image function

Style fix.

[Dirk Schulze]

Created attachment 208821 [details]
Patch for filter() image function

Looks like gtk-wk2 bot is building with CSS_FILTERS disabled. This patch hopefully fixes the build again.

[Dean Jackson]

Comment on attachment 208821 [details]
Patch for filter() image function

View in context: https://bugs.webkit.org/attachment.cgi?id=208821&action=review

Looks good!

> LayoutTests/fast/filter-image/parse-filter-image.html:10
> +description('Test that clip-path shapes accept different length units');

Wrong description

[Dirk Schulze]

Created attachment 208836 [details]
Patch for landing.

[WebKit Commit Bot]

Comment on attachment 208836 [details]
Patch for landing.

Rejecting attachment 208836 [details] from commit-queue.

New failing tests:
fast/filter-image/parse-filter-image.html
Full output: http://webkit-queues.appspot.com/results/1470285

[WebKit Commit Bot]

Created attachment 208841 [details]
Archive of layout-test-results from webkit-cq-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.4

[Dirk Schulze]

Created attachment 208844 [details]
Patch for landing.

Forgot to update expected file.

[WebKit Commit Bot]

Comment on attachment 208844 [details]
Patch for landing.

Clearing flags on attachment: 208844

Committed r154133: <http://trac.webkit.org/changeset/154133>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brent Fulgham]

Comment on attachment 208844 [details]
Patch for landing.

View in context: https://bugs.webkit.org/attachment.cgi?id=208844&action=review

> Source/WebCore/css/CSSParser.cpp:9279
> +    for (CSSParserValue* value = valueList->current(); value; value = valueList->next()) {

I know you didn't write this logic, but it generates a warning when building because 'value' shadows the second argument passed to this method.

Can you double-check that the use of 'value' inside this loop is unrelated to the return value argument?  I think it would be better to rename this inner loop variable to avoid confusion.

[Dirk Schulze]

(In reply to comment #13)
> (From update of attachment 208844 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=208844&action=review
> 
> > Source/WebCore/css/CSSParser.cpp:9279
> > +    for (CSSParserValue* value = valueList->current(); value; value = valueList->next()) {
> 
> I know you didn't write this logic, but it generates a warning when building because 'value' shadows the second argument passed to this method.
> 
> Can you double-check that the use of 'value' inside this loop is unrelated to the return value argument?  I think it would be better to rename this inner loop variable to avoid confusion.

Oh, that is my fault :P I wonder why it didn't throw a warning on Mac.


After checking the code, the shadowing doesn't influence the logic. But it should certainly be fixed.