Bug 78626 – Incorrect results/offset image in CSS filters (non-composited path)

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

[Stephen White]

In the non-composited path, when two or more filters are used in -webkit-filter, the results are often incorrect.  See the attached test case.

[Stephen White]

Created attachment 127024 [details]
Test case

[Stephen White]

Created attachment 127052 [details]
Patch

[Stephen White]

Created attachment 127069 [details]
Patch

[Stephen White]

Created attachment 127189 [details]
Updated test

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2

Updating OpenSource
git.webkit.org[0: 17.254.20.231]: errno=Connection refused
fatal: unable to connect a socket (Connection refused)
Died at Tools/Scripts/update-webkit line 162.


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

[Stephen White]

(In reply to comment #4)
> Created an attachment (id=127189) [details]
> Updated test

I realized that the layout test in the earlier patch was passing with or without the code change, I think due to the mouse position already being at 0,0 when the tests starts (so the initial draw is correct, even before the mouse pos change).  This one at least fails on Chrome DRT without the code change, although it still passes on Mac DRT for unknown reasons.  Apparently I still haven't quite got the hang of repaint tests.

[Stephen White]

(In reply to comment #5)
> Attachment 127189 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/update-webkit']" exit_code: 2
> 
> Updating OpenSource
> git.webkit.org[0: 17.254.20.231]: errno=Connection refused
> fatal: unable to connect a socket (Connection refused)
> Died at Tools/Scripts/update-webkit line 162.
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

check-webkit-style passes locally; seems the bot is unhappy.

[Darin Adler]

Comment on attachment 127189 [details]
Updated test

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

> Source/WebCore/rendering/FilterEffectRenderer.cpp:332
> +    for (size_t i = 0; i < m_effects.size(); ++i) {
> +        RefPtr<FilterEffect> effect = m_effects.at(i);
> +        effect->clearResult();
> +    }

I think this would read better without the local variable:

    for (size_t i = 0; i < m_effects.size(); ++i)
        m_effects[i]->clearResult();

[Stephen White]

Comment on attachment 127189 [details]
Updated test

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

>> Source/WebCore/rendering/FilterEffectRenderer.cpp:332
>> +    }
> 
> I think this would read better without the local variable:
> 
>     for (size_t i = 0; i < m_effects.size(); ++i)
>         m_effects[i]->clearResult();

Thanks; will make that change on landing.

[Stephen White]

Committed r107822: <http://trac.webkit.org/changeset/107822>

[Stephen White]

Comment on attachment 127189 [details]
Updated test

All patches landed; removing r+ (I guess webkit-patch forgot).

[Alexandru Chiculita]

*** Bug 77623 has been marked as a duplicate of this bug. ***