96563 2012-09-12 14:42:23 -0700 [CSS Shaders] Cached validated programs are destroyed and recreated when there is only one custom filter animating 2012-09-12 17:17:05 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 71392 1 mvujovic mvujovic achicu dino eric jacobg webkit.review.bot oldest_to_newest 718746 0 mvujovic 2012-09-12 14:42:23 -0700 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. 718811 1 163721 mvujovic 2012-09-12 15:33:48 -0700 Created attachment 163721 Patch 718825 2 163721 achicu 2012-09-12 15:43:44 -0700 Comment on attachment 163721 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); 718827 3 mvujovic 2012-09-12 15:44:54 -0700 (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. 718838 4 163724 mvujovic 2012-09-12 15:57:14 -0700 Created attachment 163724 Patch Updated patch based on Alex's informal review. 718905 5 163724 webkit.review.bot 2012-09-12 17:17:02 -0700 Comment on attachment 163724 Patch Clearing flags on attachment: 163724 Committed r128387: <http://trac.webkit.org/changeset/128387> 718906 6 webkit.review.bot 2012-09-12 17:17:05 -0700 All reviewed patches have been landed. Closing bug. 163721 2012-09-12 15:33:48 -0700 2012-09-12 15:57:14 -0700 Patch 96563-1.patch text/plain 2970 mvujovic ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAxN2I5YjliLi45NTFjNjRjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjcg QEAKKzIwMTItMDktMTIgIE1heCBWdWpvdmljICA8bXZ1am92aWNAYWRvYmUuY29tPgorCisgICAg ICAgIFtDU1MgU2hhZGVyc10gQ2FjaGVkIHZhbGlkYXRlZCBwcm9ncmFtcyBhcmUgZGVzdHJveWVk IGFuZCByZWNyZWF0ZWQgd2hlbiB0aGVyZSBpcyBvbmx5IG9uZSBjdXN0b20gZmlsdGVyIGFuaW1h dGluZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTY1 NjMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBCZWZv cmUgdGhpcyBwYXRjaCwgaW4gRmlsdGVyRWZmZWN0UmVuZGVyZXI6OmJ1aWxkLCB3ZSB3b3VsZCBm aXJzdCBjbGVhciB0aGUgb2xkIGVmZmVjdHMgYW5kCisgICAgICAgIHRoZW4gY3JlYXRlIHRoZSBu ZXcgZWZmZWN0cy4KKworICAgICAgICBTdXBwb3NlIHdlIGhhdmUgb25lIEZFQ3VzdG9tRmlsdGVy IGFuaW1hdGluZyBvbiB0aGUgcGFnZS4gVGhpcyBGRUN1c3RvbUZpbHRlciBob2xkcyB0aGUgbGFz dAorICAgICAgICByZWZlcmVuY2UgdG8gdGhlIGNhY2hlZCB2YWxpZGF0ZWQgcHJvZ3JhbS4gQmVm b3JlIHRoaXMgcGF0Y2gsIHdlIHdvdWxkIGZpcnN0IGRlc3Ryb3kgdGhlIG9sZAorICAgICAgICBG RUN1c3RvbUZpbHRlciwgd2hpY2ggd291bGQgZGVzdHJveSBpdHMgY2FjaGVkIHZhbGlkYXRlZCBw cm9ncmFtLiBUaGVuLCB3ZSB3b3VsZCBjcmVhdGUgdGhlCisgICAgICAgIG5ldyBGRUN1c3RvbUZp bHRlciBhbmQgaGF2ZSB0byByZWNyZWF0ZSB0aGUgdmFsaWRhdGVkIHByb2dyYW0uCisKKyAgICAg ICAgVGhpcyBwYXRjaCBtYWtlcyBGaWx0ZXJFZmZlY3RSZW5kZXJlcjo6YnVpbGQga2VlcCB0aGUg b2xkIGVmZmVjdHMgYXJvdW5kIHVudGlsIHdlJ3JlCisgICAgICAgIGRvbmUgY3JlYXRpbmcgdGhl IG5ldyBlZmZlY3RzLiBUaGlzIHdheSwgd2Ugd29uJ3QgcmVjcmVhdGUgdmFsaWRhdGVkIHByb2dy YW1zLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gV2UgY2FuJ3QgdGVzdCB0aGlzIGJlY2F1c2Ug dGhlIHZhbGlkYXRlZCBwcm9ncmFtIGNhY2hlIGlzIG5vdCBleHBvc2VkIHRvIHdlYgorICAgICAg ICBwYWdlcy4KKworICAgICAgICAqIHJlbmRlcmluZy9GaWx0ZXJFZmZlY3RSZW5kZXJlci5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpGaWx0ZXJFZmZlY3RSZW5kZXJlcjo6YnVpbGQpOgorCiAyMDEy LTA5LTEyICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAg ICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTI4MzE4IGFuZCByMTI4MzMyLgpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0ZpbHRlckVmZmVjdFJlbmRlcmVyLmNwcCBi L1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9GaWx0ZXJFZmZlY3RSZW5kZXJlci5jcHAKaW5kZXgg NzhlZDI0OC4uZGZlMjQwZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0Zp bHRlckVmZmVjdFJlbmRlcmVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvRmls dGVyRWZmZWN0UmVuZGVyZXIuY3BwCkBAIC0yMDIsNyArMjAyLDEwIEBAIGJvb2wgRmlsdGVyRWZm ZWN0UmVuZGVyZXI6OmJ1aWxkKERvY3VtZW50KiBkb2N1bWVudCwgY29uc3QgRmlsdGVyT3BlcmF0 aW9ucyYgb3BlCiAgICAgbV9oYXNGaWx0ZXJUaGF0TW92ZXNQaXhlbHMgPSBvcGVyYXRpb25zLmhh c0ZpbHRlclRoYXRNb3Zlc1BpeGVscygpOwogICAgIGlmIChtX2hhc0ZpbHRlclRoYXRNb3Zlc1Bp eGVscykKICAgICAgICAgb3BlcmF0aW9ucy5nZXRPdXRzZXRzKG1fdG9wT3V0c2V0LCBtX3JpZ2h0 T3V0c2V0LCBtX2JvdHRvbU91dHNldCwgbV9sZWZ0T3V0c2V0KTsKLSAgICBtX2VmZmVjdHMuY2xl YXIoKTsKKyAgICAKKyAgICAvLyBEb24ndCBjbGVhciBtX2VmZmVjdHMgdW50aWwgd2UndmUgY3Jl YXRlZCB0aGUgbmV3IGVmZmVjdHMuCisgICAgLy8gTmV3IEZFQ3VzdG9tRmlsdGVycyBjYW4gcmV1 c2UgY2FjaGVkIHJlc291cmNlcyBmcm9tIG9sZCBGRUN1c3RvbUZpbHRlcnMuCisgICAgRmlsdGVy RWZmZWN0TGlzdCBuZXdFZmZlY3RzOwogCiAgICAgUmVmUHRyPEZpbHRlckVmZmVjdD4gcHJldmlv dXNFZmZlY3QgPSBtX3NvdXJjZUdyYXBoaWM7CiAgICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBv cGVyYXRpb25zLm9wZXJhdGlvbnMoKS5zaXplKCk7ICsraSkgewpAQCAtMzY0LDEyICszNjcsMTQg QEAgYm9vbCBGaWx0ZXJFZmZlY3RSZW5kZXJlcjo6YnVpbGQoRG9jdW1lbnQqIGRvY3VtZW50LCBj b25zdCBGaWx0ZXJPcGVyYXRpb25zJiBvcGUKICAgICAgICAgICAgIAogICAgICAgICAgICAgaWYg KGZpbHRlck9wZXJhdGlvbi0+Z2V0T3BlcmF0aW9uVHlwZSgpICE9IEZpbHRlck9wZXJhdGlvbjo6 UkVGRVJFTkNFKSB7CiAgICAgICAgICAgICAgICAgZWZmZWN0LT5pbnB1dEVmZmVjdHMoKS5hcHBl bmQocHJldmlvdXNFZmZlY3QpOwotICAgICAgICAgICAgICAgIG1fZWZmZWN0cy5hcHBlbmQoZWZm ZWN0KTsKKyAgICAgICAgICAgICAgICBuZXdFZmZlY3RzLmFwcGVuZChlZmZlY3QpOwogICAgICAg ICAgICAgfQogICAgICAgICAgICAgcHJldmlvdXNFZmZlY3QgPSBlZmZlY3QucmVsZWFzZSgpOwog ICAgICAgICB9CiAgICAgfQogCisgICAgbV9lZmZlY3RzLnN3YXAobmV3RWZmZWN0cyk7CisKICAg ICAvLyBJZiB3ZSBkaWRuJ3QgbWFrZSBhbnkgZWZmZWN0cywgdGVsbCBvdXIgY2FsbGVyIHdlIGFy ZSBub3QgdmFsaWQKICAgICBpZiAoIW1fZWZmZWN0cy5zaXplKCkpCiAgICAgICAgIHJldHVybiBm YWxzZTsK 163724 2012-09-12 15:57:14 -0700 2012-09-12 17:17:01 -0700 Patch 96563-2.patch text/plain 2420 mvujovic ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAxN2I5YjliLi45NTFjNjRjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjcg QEAKKzIwMTItMDktMTIgIE1heCBWdWpvdmljICA8bXZ1am92aWNAYWRvYmUuY29tPgorCisgICAg ICAgIFtDU1MgU2hhZGVyc10gQ2FjaGVkIHZhbGlkYXRlZCBwcm9ncmFtcyBhcmUgZGVzdHJveWVk IGFuZCByZWNyZWF0ZWQgd2hlbiB0aGVyZSBpcyBvbmx5IG9uZSBjdXN0b20gZmlsdGVyIGFuaW1h dGluZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTY1 NjMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBCZWZv cmUgdGhpcyBwYXRjaCwgaW4gRmlsdGVyRWZmZWN0UmVuZGVyZXI6OmJ1aWxkLCB3ZSB3b3VsZCBm aXJzdCBjbGVhciB0aGUgb2xkIGVmZmVjdHMgYW5kCisgICAgICAgIHRoZW4gY3JlYXRlIHRoZSBu ZXcgZWZmZWN0cy4KKworICAgICAgICBTdXBwb3NlIHdlIGhhdmUgb25lIEZFQ3VzdG9tRmlsdGVy IGFuaW1hdGluZyBvbiB0aGUgcGFnZS4gVGhpcyBGRUN1c3RvbUZpbHRlciBob2xkcyB0aGUgbGFz dAorICAgICAgICByZWZlcmVuY2UgdG8gdGhlIGNhY2hlZCB2YWxpZGF0ZWQgcHJvZ3JhbS4gQmVm b3JlIHRoaXMgcGF0Y2gsIHdlIHdvdWxkIGZpcnN0IGRlc3Ryb3kgdGhlIG9sZAorICAgICAgICBG RUN1c3RvbUZpbHRlciwgd2hpY2ggd291bGQgZGVzdHJveSBpdHMgY2FjaGVkIHZhbGlkYXRlZCBw cm9ncmFtLiBUaGVuLCB3ZSB3b3VsZCBjcmVhdGUgdGhlCisgICAgICAgIG5ldyBGRUN1c3RvbUZp bHRlciBhbmQgaGF2ZSB0byByZWNyZWF0ZSB0aGUgdmFsaWRhdGVkIHByb2dyYW0uCisKKyAgICAg ICAgVGhpcyBwYXRjaCBtYWtlcyBGaWx0ZXJFZmZlY3RSZW5kZXJlcjo6YnVpbGQga2VlcCB0aGUg b2xkIGVmZmVjdHMgYXJvdW5kIHVudGlsIHdlJ3JlCisgICAgICAgIGRvbmUgY3JlYXRpbmcgdGhl IG5ldyBlZmZlY3RzLiBUaGlzIHdheSwgd2Ugd29uJ3QgcmVjcmVhdGUgdmFsaWRhdGVkIHByb2dy YW1zLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gV2UgY2FuJ3QgdGVzdCB0aGlzIGJlY2F1c2Ug dGhlIHZhbGlkYXRlZCBwcm9ncmFtIGNhY2hlIGlzIG5vdCBleHBvc2VkIHRvIHdlYgorICAgICAg ICBwYWdlcy4KKworICAgICAgICAqIHJlbmRlcmluZy9GaWx0ZXJFZmZlY3RSZW5kZXJlci5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpGaWx0ZXJFZmZlY3RSZW5kZXJlcjo6YnVpbGQpOgorCiAyMDEy LTA5LTEyICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAg ICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTI4MzE4IGFuZCByMTI4MzMyLgpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0ZpbHRlckVmZmVjdFJlbmRlcmVyLmNwcCBi L1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9GaWx0ZXJFZmZlY3RSZW5kZXJlci5jcHAKaW5kZXgg NzhlZDI0OC4uOWNiMjk0NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVuZGVyaW5nL0Zp bHRlckVmZmVjdFJlbmRlcmVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvRmls dGVyRWZmZWN0UmVuZGVyZXIuY3BwCkBAIC0yMDIsNyArMjAyLDExIEBAIGJvb2wgRmlsdGVyRWZm ZWN0UmVuZGVyZXI6OmJ1aWxkKERvY3VtZW50KiBkb2N1bWVudCwgY29uc3QgRmlsdGVyT3BlcmF0 aW9ucyYgb3BlCiAgICAgbV9oYXNGaWx0ZXJUaGF0TW92ZXNQaXhlbHMgPSBvcGVyYXRpb25zLmhh c0ZpbHRlclRoYXRNb3Zlc1BpeGVscygpOwogICAgIGlmIChtX2hhc0ZpbHRlclRoYXRNb3Zlc1Bp eGVscykKICAgICAgICAgb3BlcmF0aW9ucy5nZXRPdXRzZXRzKG1fdG9wT3V0c2V0LCBtX3JpZ2h0 T3V0c2V0LCBtX2JvdHRvbU91dHNldCwgbV9sZWZ0T3V0c2V0KTsKLSAgICBtX2VmZmVjdHMuY2xl YXIoKTsKKyAgICAKKyAgICAvLyBLZWVwIHRoZSBvbGQgZWZmZWN0cyBvbiB0aGUgc3RhY2sgdW50 aWwgd2UndmUgY3JlYXRlZCB0aGUgbmV3IGVmZmVjdHMuCisgICAgLy8gTmV3IEZFQ3VzdG9tRmls dGVycyBjYW4gcmV1c2UgY2FjaGVkIHJlc291cmNlcyBmcm9tIG9sZCBGRUN1c3RvbUZpbHRlcnMu CisgICAgRmlsdGVyRWZmZWN0TGlzdCBvbGRFZmZlY3RzOworICAgIG1fZWZmZWN0cy5zd2FwKG9s ZEVmZmVjdHMpOwogCiAgICAgUmVmUHRyPEZpbHRlckVmZmVjdD4gcHJldmlvdXNFZmZlY3QgPSBt X3NvdXJjZUdyYXBoaWM7CiAgICAgZm9yIChzaXplX3QgaSA9IDA7IGkgPCBvcGVyYXRpb25zLm9w ZXJhdGlvbnMoKS5zaXplKCk7ICsraSkgewo=