Bug 96563

Summary: [CSS Shaders] Cached validated programs are destroyed and recreated when there is only one custom filter animating
Product: WebKit Reporter: Max Vujovic <mvujovic>
Component: Layout and RenderingAssignee: Max Vujovic <mvujovic>
Status: RESOLVED FIXED    
Severity: Normal CC: achicu, dino, eric, jacobg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 71392    
Attachments:
Description Flags
Patch
none
Patch none

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.