Bug 96563 - [CSS Shaders] Cached validated programs are destroyed and recreated when there is only one custom filter animating
Summary: [CSS Shaders] Cached validated programs are destroyed and recreated when ther...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Vujovic
URL:
Keywords:
Depends on:
Blocks: 71392
  Show dependency treegraph
 
Reported: 2012-09-12 14:42 PDT by Max Vujovic
Modified: 2012-09-12 17:17 PDT (History)
5 users (show)

See Also:


Attachments
Patch (2.90 KB, patch)
2012-09-12 15:33 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (2.36 KB, patch)
2012-09-12 15:57 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Max Vujovic 2012-09-12 14:42:23 PDT
In FilterEffectRenderer::build, we first clear the old effects and then create the new effects.

Suppose we have one FECustomFilter animating on our page. In FilterEffectRenderer::build, we first destroy the old FECustomFilter. This removes its validated program from the cache since it was the last FECustomFilter using it. Then, we create the new FECustomFilter, and we have to create a new validated program.

We need to keep the old effects around until we're done creating the new effects so that we can reuse validated programs.
Comment 1 Max Vujovic 2012-09-12 15:33:48 PDT
Created attachment 163721 [details]
Patch
Comment 2 Alexandru Chiculita 2012-09-12 15:43:44 PDT
Comment on attachment 163721 [details]
Patch

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

> Source/WebCore/rendering/FilterEffectRenderer.cpp:207
> +    // New FECustomFilters can reuse cached resources from old FECustomFilters.

I think it would be easy to just create a new list and swap it here. It would be similar to clearing m_effects, while still keeping the old ones alive.

FilterEffectList oldEffects;
m_effects.swap(oldEffects);
Comment 3 Max Vujovic 2012-09-12 15:44:54 PDT
(In reply to comment #2)
> (From update of attachment 163721 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=163721&action=review
> 
> > Source/WebCore/rendering/FilterEffectRenderer.cpp:207
> > +    // New FECustomFilters can reuse cached resources from old FECustomFilters.
> 
> I think it would be easy to just create a new list and swap it here. It would be similar to clearing m_effects, while still keeping the old ones alive.
> 
> FilterEffectList oldEffects;
> m_effects.swap(oldEffects);

Sure, I can do that.
Comment 4 Max Vujovic 2012-09-12 15:57:14 PDT
Created attachment 163724 [details]
Patch

Updated patch based on Alex's informal review.
Comment 5 WebKit Review Bot 2012-09-12 17:17:02 PDT
Comment on attachment 163724 [details]
Patch

Clearing flags on attachment: 163724

Committed r128387: <http://trac.webkit.org/changeset/128387>
Comment 6 WebKit Review Bot 2012-09-12 17:17:05 PDT
All reviewed patches have been landed.  Closing bug.