Bug 78626 - Incorrect results/offset image in CSS filters (non-composited path)
Summary: Incorrect results/offset image in CSS filters (non-composited path)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Stephen White
URL:
Keywords:
: 77623 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-14 13:01 PST by Stephen White
Modified: 2012-03-05 11:54 PST (History)
5 users (show)

See Also:


Attachments
Test case (318 bytes, text/html)
2012-02-14 13:02 PST, Stephen White
no flags Details
Patch (18.28 KB, patch)
2012-02-14 14:54 PST, Stephen White
no flags Details | Formatted Diff | Diff
Patch (18.39 KB, patch)
2012-02-14 15:51 PST, Stephen White
no flags Details | Formatted Diff | Diff
Updated test (18.43 KB, patch)
2012-02-15 08:40 PST, Stephen White
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen White 2012-02-14 13:01:41 PST
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.
Comment 1 Stephen White 2012-02-14 13:02:51 PST
Created attachment 127024 [details]
Test case
Comment 2 Stephen White 2012-02-14 14:54:37 PST
Created attachment 127052 [details]
Patch
Comment 3 Stephen White 2012-02-14 15:51:23 PST
Created attachment 127069 [details]
Patch
Comment 4 Stephen White 2012-02-15 08:40:14 PST
Created attachment 127189 [details]
Updated test
Comment 5 WebKit Review Bot 2012-02-15 08:42:12 PST
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.
Comment 6 Stephen White 2012-02-15 08:43:12 PST
(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.
Comment 7 Stephen White 2012-02-15 08:43:57 PST
(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.
Comment 8 Darin Adler 2012-02-15 09:53:41 PST
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();
Comment 9 Stephen White 2012-02-15 10:27:24 PST
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.
Comment 10 Stephen White 2012-02-15 10:44:41 PST
Committed r107822: <http://trac.webkit.org/changeset/107822>
Comment 11 Stephen White 2012-02-16 11:29:07 PST
Comment on attachment 127189 [details]
Updated test

All patches landed; removing r+ (I guess webkit-patch forgot).
Comment 12 Alexandru Chiculita 2012-03-05 11:54:51 PST
*** Bug 77623 has been marked as a duplicate of this bug. ***