SVGFilterBuilder should drive the builtin sourceAlpha from the passed sourceGraphic

• Component: SVG
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

Created attachment 251526 [details]
test case

Open the attached test case.

Result: A black square is displayed.
Expected: A lime square should be displayed.

When creating the SVGFilterBuilder, we create a SourceAlpha object from the current SourceGraphic. We should create the SourceAlpha from the previousEffect. If there is no effect applied before, the previousEffect will be the sourceGraphic.

In the attached test case. Here is what is happening:

1. A lime <div> is created.
2. On top of it, a red <div> is created. But two filter are applied to this <div>
   a. The first filter is css opacity(0) filter, which results a transparent square (the alpha bytes are all zeros)
   b. The second filter is an SVG filter which merges the SourceAlpha of the filter with the existing drawing.

The bug happens because we create the SourceAlpha of the SVGFilterBuilder from the current SourceGraphic. And since the current SourceGraphic is a lime square, the sourceAlpha will be black square (the r, g, b are zeros but a is 1). So when it is merged with the existing drawing it draws a black square.

[Said Abou-Hallawa]

Created attachment 251565 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.cpp:29:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.cpp:36:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.cpp:39:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.cpp:55:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.h:37:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.h:54:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.h:40:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.h:42:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 8 in 87 files


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

[Darin Adler]

Comment on attachment 251565 [details]
Patch

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

> Source/WebCore/platform/graphics/filters/SourceAlpha.cpp:32
> +Ref<SourceAlpha> SourceAlpha::create(FilterEffect* sourceEffect)

Should be a reference, since the code below assumes it’s non-null. Could do that in a later patch, though.

> Source/WebCore/platform/graphics/filters/SourceAlpha.cpp:75
> +SourceAlpha::SourceAlpha(FilterEffect* sourceEffect)

Should be a reference, since the code below assumes it’s non-null. Could do that in a later patch, though.

[Said Abou-Hallawa]

Created attachment 251717 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.cpp:29:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.cpp:36:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.cpp:39:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.cpp:55:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.h:37:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFEImage.h:54:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.h:40:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
ERROR: Source/WebCore/svg/graphics/filters/SVGFilterBuilder.h:42:  The parameter type should use PassRefPtr instead of RefPtr.  [readability/pass_ptr] [5]
Total errors found: 8 in 87 files


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

[Said Abou-Hallawa]

(In reply to comment #3)
> Comment on attachment 251565 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251565&action=review
> 
> > Source/WebCore/platform/graphics/filters/SourceAlpha.cpp:32
> > +Ref<SourceAlpha> SourceAlpha::create(FilterEffect* sourceEffect)
> 
> Should be a reference, since the code below assumes it’s non-null. Could do
> that in a later patch, though.
> 

Done.

> > Source/WebCore/platform/graphics/filters/SourceAlpha.cpp:75
> > +SourceAlpha::SourceAlpha(FilterEffect* sourceEffect)
> 
> Should be a reference, since the code below assumes it’s non-null. Could do
> that in a later patch, though.

Done.

[WebKit Commit Bot]

Comment on attachment 251717 [details]
Patch

Clearing flags on attachment: 251717

Committed r183381: <http://trac.webkit.org/changeset/183381>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/20704289>

[Alexey Proskuryakov]

The new test fails on Windows every time: https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=svg%2Ffilters%2FsourceAlpha-input-filter-effect.html