Bug 102414

Summary: [Texmap][CSS Shaders] Make the CustomFilterValidatedProgram maintain the platform compiled program
Product: WebKit Reporter: Alexandru Chiculita <achicu>
Component: WebKit QtAssignee: Alexandru Chiculita <achicu>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, abecsi, cmarcelo, dino, gtk-ews, gyuyoung.kim, jamesr, kenneth, menard, noam, rakuco, tony, webkit.review.bot, xan.lopez, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 101801    
Attachments:
Description Flags
Patch V1
none
Patch V2
kenneth: review-
Patch V3 none

Alexandru Chiculita
Reported 2012-11-15 11:23:32 PST
ValidatedCustomFilterOperation already contains a reference to a CustomFilterValidatedProgram which is cached and reused across multiple repaints and RenderLayers. It also has a mechanism to keep a reference to a platform compiled shader. We will reuse this to maintain the "compiled shaders" in the WK2 LayerTreeCoordinator.cpp.
Attachments
Patch V1 (32.70 KB, patch)
2012-11-15 11:50 PST, Alexandru Chiculita 		no flags
DetailsFormatted DiffDiff
Patch V2 (32.74 KB, patch)
2012-11-15 13:03 PST, Alexandru Chiculita 		kenneth: review-
DetailsFormatted DiffDiff
Patch V3 (31.45 KB, patch)
2012-11-15 14:00 PST, Alexandru Chiculita 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexandru Chiculita
Comment 1 2012-11-15 11:50:27 PST
Created attachment 174496 [details] Patch V1
Alexandru Chiculita
Comment 2 2012-11-15 13:03:14 PST
Created attachment 174503 [details] Patch V2 Rebased.
Kenneth Rohde Christiansen
Comment 3 2012-11-15 13:41:08 PST
Comment on attachment 174503 [details] Patch V2 View in context: https://bugs.webkit.org/attachment.cgi?id=174503&action=review > Source/WebCore/WebCore.gyp/WebCore.gyp:118 > '../platform/graphics/filters', > + '../platform/graphics/filters/texmap', > '../platform/graphics/filters/skia', Google doesn't use texture mapper, why is this needed, seems wrong > Source/WebCore/WebCore.gypi:4955 > 'platform/graphics/efl/IntRectEfl.cpp', > + 'platform/graphics/filters/texmap/CustomFilterValidatedProgramTextureMapper.cpp', > + 'platform/graphics/filters/texmap/TextureMapperPlatformCompiledProgram.h', > 'platform/graphics/filters/CustomFilterArrayParameter.h', > 'platform/graphics/filters/CustomFilterConstants.h', Same here
Alexandru Chiculita
Comment 4 2012-11-15 13:43:11 PST
(In reply to comment #3) > (From update of attachment 174503 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=174503&action=review > > > Source/WebCore/WebCore.gyp/WebCore.gyp:118 > > '../platform/graphics/filters', > > + '../platform/graphics/filters/texmap', > > '../platform/graphics/filters/skia', > > Google doesn't use texture mapper, why is this needed, seems wrong > > > Source/WebCore/WebCore.gypi:4955 > > 'platform/graphics/efl/IntRectEfl.cpp', > > + 'platform/graphics/filters/texmap/CustomFilterValidatedProgramTextureMapper.cpp', > > + 'platform/graphics/filters/texmap/TextureMapperPlatformCompiledProgram.h', > > 'platform/graphics/filters/CustomFilterArrayParameter.h', > > 'platform/graphics/filters/CustomFilterConstants.h', > > Same here Sorry about that. I've searched for TextureMapper related files and found them in the GYP project file too. I thought that there may be projects based on that. I will remove them.
Alexandru Chiculita
Comment 5 2012-11-15 13:44:41 PST
(In reply to comment #4) For example in http://svn.webkit.org/repository/webkit/trunk/Source/WebCore/WebCore.gypi search for platform/graphics/texmap/
Kenneth Rohde Christiansen
Comment 6 2012-11-15 13:45:23 PST
I suggest you git blame that line, it might be a mistake
Kenneth Rohde Christiansen
Comment 7 2012-11-15 13:48:24 PST
b372bde3 Source/WebCore/WebCore.gypi (rniwa@webkit.org 2011-07-22 21:31:04 +0000 3986) 'platform/graphics/texmap/GraphicsLayerTextureMapper.cpp', b372bde3 Source/WebCore/WebCore.gypi (rniwa@webkit.org 2011-07-22 21:31:04 +0000 3987) 'platform/graphics/texmap/GraphicsLayerTextureMapper.h', ecf8da49 Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-01-09 11:54:06 +0000 3988) 'platform/graphics/texmap/TextureMapper.cpp', b372bde3 Source/WebCore/WebCore.gypi (rniwa@webkit.org 2011-07-22 21:31:04 +0000 3989) 'platform/graphics/texmap/TextureMapper.h', 1571338c Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-02-14 12:53:30 +0000 3990) 'platform/graphics/texmap/TextureMapperBackingStore.cpp', 1571338c Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-02-14 12:53:30 +0000 3991) 'platform/graphics/texmap/TextureMapperBackingStore.h', 5ee5803c Source/WebCore/WebCore.gypi (yael.aharon@nokia.com 2012-02-22 04:02:26 +0000 3992) 'platform/graphics/texmap/TextureMapperGL.cpp', 5ee5803c Source/WebCore/WebCore.gypi (yael.aharon@nokia.com 2012-02-22 04:02:26 +0000 3993) 'platform/graphics/texmap/TextureMapperGL.h', 1ddb153e Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-02-03 14:59:31 +0000 3994) 'platform/graphics/texmap/TextureMapperImageBuffer.cpp', 1ddb153e Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-02-03 14:59:31 +0000 3995) 'platform/graphics/texmap/TextureMapperImageBuffer.h', a3e29880 Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-02-15 08:26:30 +0000 3996) 'platform/graphics/texmap/TextureMapperLayer.cpp', a3e29880 Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-02-15 08:26:30 +0000 3997) 'platform/graphics/texmap/TextureMapperLayer.h', b372bde3 Source/WebCore/WebCore.gypi (rniwa@webkit.org 2011-07-22 21:31:04 +0000 3998) 'platform/graphics/texmap/TextureMapperPlatformLayer.h', 5ee5803c Source/WebCore/WebCore.gypi (yael.aharon@nokia.com 2012-02-22 04:02:26 +0000 3999) 'platform/graphics/texmap/TextureMapperShaderManager.cpp', 5ee5803c Source/WebCore/WebCore.gypi (yael.aharon@nokia.com 2012-02-22 04:02:26 +0000 4000) 'platform/graphics/texmap/TextureMapperShaderManager.h',
Kenneth Rohde Christiansen
Comment 8 2012-11-15 13:50:33 PST
> ecf8da49 Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-01-09 11:54:06 +0000 3988) 'platform/graphics/texmap/TextureMapper.cpp', Noam, I don't think these should be added to chrome right?
Noam Rosenthal
Comment 9 2012-11-15 13:51:49 PST
(In reply to comment #8) > > ecf8da49 Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-01-09 11:54:06 +0000 3988) 'platform/graphics/texmap/TextureMapper.cpp', > > Noam, I don't think these should be added to chrome right? Probably not :) Let's remove it...
Alexandru Chiculita
Comment 10 2012-11-15 13:56:48 PST
(In reply to comment #9) > (In reply to comment #8) > > > ecf8da49 Source/WebCore/WebCore.gypi (noam.rosenthal@nokia.com 2012-01-09 11:54:06 +0000 3988) 'platform/graphics/texmap/TextureMapper.cpp', > > > > Noam, I don't think these should be added to chrome right? > Probably not :) Let's remove it... Added a bug for that: https://bugs.webkit.org/show_bug.cgi?id=102425
Alexandru Chiculita
Comment 11 2012-11-15 14:00:07 PST
Created attachment 174509 [details] Patch V3
Noam Rosenthal
Comment 12 2012-11-15 14:04:17 PST
Comment on attachment 174509 [details] Patch V3 Looks like a step in the right direction. Can't find anything here that I would do different.
Alexandru Chiculita
Comment 13 2012-11-15 14:06:01 PST
(In reply to comment #12) > (From update of attachment 174509 [details]) > Looks like a step in the right direction. Can't find anything here that I would do different. Thanks, Noam! Looking in the gypi project file I've found lots of other unrelated files like for example the whole platform/graphics/qt/ is in there. Do you know what's the policy?
Tony Chang
Comment 14 2012-11-15 14:23:46 PST
(In reply to comment #13) > (In reply to comment #12) > > (From update of attachment 174509 [details] [details]) > > Looks like a step in the right direction. Can't find anything here that I would do different. > > Thanks, Noam! > > Looking in the gypi project file I've found lots of other unrelated files like for example the whole platform/graphics/qt/ is in there. > > Do you know what's the policy? Normally, we list every file and directory in the .gypi files. In WebCore.gyp, we exclude files that aren't used: http://trac.webkit.org/browser/trunk/Source/WebCore/WebCore.gyp/WebCore.gyp#L1612 You can see that texmap is already excluded so adding new files to WebCore.gypi is fine. In theory, the purpose is to make things easy for people not working on Chromium. You don't have to think about whether to add a file or not, you just add everything.
kov's GTK+ EWS bot
Comment 15 2012-11-15 14:48:49 PST
Comment on attachment 174509 [details] Patch V3 Attachment 174509 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/14848530
Alexandru Chiculita
Comment 16 2012-11-15 23:00:20 PST
Just finished a local build on Gtk and it didn't fail.
Build Bot
Comment 17 2012-11-16 05:44:32 PST
Comment on attachment 174509 [details] Patch V3 Attachment 174509 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14846815 New failing tests: inspector-protocol/nmi-webaudio.html
Noam Rosenthal
Comment 18 2012-11-16 06:49:19 PST
(In reply to comment #16) > Just finished a local build on Gtk and it didn't fail. It's a flaky test, feel free to land when you're ready.
Alexandru Chiculita
Comment 19 2012-11-16 06:56:12 PST
Comment on attachment 174509 [details] Patch V3 Added cq+ to land the patch.
WebKit Review Bot
Comment 20 2012-11-16 07:39:48 PST
Comment on attachment 174509 [details] Patch V3 Clearing flags on attachment: 174509 Committed r134948: <http://trac.webkit.org/changeset/134948>
WebKit Review Bot
Comment 21 2012-11-16 07:39:55 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.