RESOLVED FIXED 104815
[Texmap] Instead of having multiple shaders sources with lots of duplication, we should have one shader source with MACRO variants
https://bugs.webkit.org/show_bug.cgi?id=104815
Summary [Texmap] Instead of having multiple shaders sources with lots of duplication,...
Noam Rosenthal
Reported 2012-12-12 08:26:00 PST
[Texmap] Instead of having multiple shaders sources with lots of duplication, we should have one shader source with MACRO variants
Attachments
Patch (47.22 KB, patch)
2012-12-12 11:37 PST, Noam Rosenthal
no flags
Patch (15.86 KB, patch)
2012-12-12 12:20 PST, Noam Rosenthal
no flags
Patch (47.33 KB, patch)
2012-12-18 18:37 PST, Noam Rosenthal
no flags
Patch (47.33 KB, patch)
2012-12-18 21:27 PST, Noam Rosenthal
no flags
Patch (47.33 KB, patch)
2012-12-18 21:30 PST, Noam Rosenthal
no flags
Patch (47.60 KB, patch)
2012-12-19 06:12 PST, Noam Rosenthal
no flags
Patch for landing (47.51 KB, patch)
2012-12-26 17:33 PST, Noam Rosenthal
no flags
Noam Rosenthal
Comment 1 2012-12-12 11:37:47 PST
Noam Rosenthal
Comment 2 2012-12-12 12:20:23 PST
Noam Rosenthal
Comment 3 2012-12-18 18:37:32 PST
WebKit Review Bot
Comment 4 2012-12-18 18:40:26 PST
Attachment 180074 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:111: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 5 2012-12-18 18:48:07 PST
Early Warning System Bot
Comment 6 2012-12-18 18:52:07 PST
Noam Rosenthal
Comment 7 2012-12-18 21:27:36 PST
WebKit Review Bot
Comment 8 2012-12-18 21:30:21 PST
Attachment 180085 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:111: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Noam Rosenthal
Comment 9 2012-12-18 21:30:32 PST
WebKit Review Bot
Comment 10 2012-12-18 21:35:37 PST
Attachment 180086 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:111: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kenneth Rohde Christiansen
Comment 11 2012-12-19 00:56:44 PST
Comment on attachment 180086 [details] Patch LGTM
EFL EWS Bot
Comment 12 2012-12-19 01:44:17 PST
Noam Rosenthal
Comment 13 2012-12-19 06:12:01 PST
Noam Rosenthal
Comment 14 2012-12-19 06:13:10 PST
Submitted style-checker false positive bug, https://bugs.webkit.org/show_bug.cgi?id=105427
WebKit Review Bot
Comment 15 2012-12-19 06:14:51 PST
Attachment 180150 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:112: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 16 2012-12-19 06:53:19 PST
Comment on attachment 180150 [details] Patch Clearing flags on attachment: 180150 Committed r138157: <http://trac.webkit.org/changeset/138157>
WebKit Review Bot
Comment 17 2012-12-19 06:53:24 PST
All reviewed patches have been landed. Closing bug.
Dominik Röttsches (drott)
Comment 18 2012-12-19 08:31:21 PST
(In reply to comment #17) > All reviewed patches have been landed. Closing bug. Looks like this is breaking API tests. http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/7337/steps/API%20tests/logs/stdio /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp(479) : void WebCore::GraphicsContext3D::compileShader(Platform3DObject) 1 0x2afabf27da69 WebCore::GraphicsContext3D::compileShader(unsigned int) 2 0x2afabf2906c1 WebCore::TextureMapperShaderProgram::TextureMapperShaderProgram(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&) 3 0x2afabf291253 WebCore::TextureMapperShaderProgram::create(WTF::PassRefPtr<WebCore::GraphicsContext3D>, WTF::String const&, WTF::String const&) 4 0x2afabf291123 WebCore::TextureMapperShaderManager::getShaderProgram(unsigned int) 5 0x2afabf288ef9 WebCore::TextureMapperGL::drawTexture(unsigned int, int, WebCore::IntSize const&, WebCore::FloatRect const&, WebCore::TransformationMatrix const&, float, WebCore::BitmapTexture const*, unsigned int) 6 0x2afabf288d79 WebCore::TextureMapperGL::drawTexture(WebCore::BitmapTexture const&, WebCore::FloatRect const&, WebCore::TransformationMatrix const&, float, WebCore::BitmapTexture const*, unsigned int) 7 0x2afabe839d3c WebCore::TextureMapperTile::paint(WebCore::TextureMapper*, WebCore::TransformationMatrix const&, float, WebCore::BitmapTexture*, unsigned int) 8 0x2afab7b5b26e WebKit::CoordinatedBackingStore::paintTilesToTextureMapper(WTF::Vector<WebCore::TextureMapperTile*, 0ul>&, WebCore::TextureMapper*, WebCore::TransformationMatrix const&, float, WebCore::BitmapTexture*, WebCore::FloatRect const&) 9 0x2afab7b5b7fc WebKit::CoordinatedBackingStore::paintToTextureMapper(WebCore::TextureMapper*, WebCore::FloatRect const&, WebCore::TransformationMatrix const&, float, WebCore::BitmapTexture*) 10 0x2afabe83d582 WebCore::TextureMapperLayer::paintSelf(WebCore::TextureMapperPaintOptions const&) 11 0x2afabe83d783 WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) 12 0x2afabe83e0c5 WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) 13 0x2afabe83e1f7 WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) 14 0x2afabe83d8ef WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) 15 0x2afabe83e0c5 WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) 16 0x2afabe83e1f7 WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) 17 0x2afabe83d8ef WebCore::TextureMapperLayer::paintSelfAndChildren(WebCore::TextureMapperPaintOptions const&) 18 0x2afabe83e0c5 WebCore::TextureMapperLayer::paintSelfAndChildrenWithReplica(WebCore::TextureMapperPaintOptions const&) 19 0x2afabe83e1f7 WebCore::TextureMapperLayer::paintRecursive(WebCore::TextureMapperPaintOptions const&) 20 0x2afabe83d119 WebCore::TextureMapperLayer::paint() 21 0x2afab7b69ab3 WebKit::LayerTreeRenderer::paintToCurrentGLContext(WebCore::TransformationMatrix const&, float, WebCore::FloatRect const&, unsigned int) 22 0x2afab7cadeb7 EwkViewImpl::displayTimerFired(WebCore::Timer<EwkViewImpl>*) 23 0x2afab7cb554c WebCore::Timer<EwkViewImpl>::fired() 24 0x2afabe7b0f3e WebCore::ThreadTimers::sharedTimerFiredInternal() 25 0x2afabe7b0e5f WebCore::ThreadTimers::sharedTimerFired() 26 0x2afabf21206d 27 0x2afab9e9a33e _ecore_timer_expired_call 28 0x2afab9e9a50b _ecore_timer_expired_timers_call 29 0x2afab9e97421 30 0x2afab9e97a15 ecore_main_loop_iterate 31 0x43721b TestWebKitAPI::Util::run(bool*)
Dominik Röttsches (drott)
Comment 19 2012-12-19 08:32:06 PST
(In reply to comment #18) > (In reply to comment #17) > > All reviewed patches have been landed. Closing bug. > > Looks like this is breaking API tests. > http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug%20WK2/builds/7337/steps/API%20tests/logs/stdio Sorry, I missed the important first line: ASSERTION FAILED: GLCompileSuccess == GL_TRUE
WebKit Review Bot
Comment 20 2012-12-19 08:38:45 PST
Re-opened since this is blocked by bug 105439
Csaba Osztrogonác
Comment 21 2012-12-19 09:05:54 PST
Additionally it broke all pixel tests on the Qt-WK2 pixel tester bot: http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Release%20WebKit2%20%28Pixel%20Tests%29/builds/1023/steps/layout-test/logs/stdio All of the png became empty.
Noam Rosenthal
Comment 22 2012-12-19 09:14:33 PST
Hmm, all worked on my machine with no asserts; I will try on a different computer and try to reproduce. Either way thanks for rolling out. Btw would be great if this kind of thing wad caught by ews or cq...
Dominik Röttsches (drott)
Comment 23 2012-12-19 09:20:42 PST
(In reply to comment #22) > Hmm, all worked on my machine with no asserts; I will try on a different computer and try to reproduce. Either way thanks for rolling out. > Btw would be great if this kind of thing wad caught by ews or cq... It would be good, but we're not confident enough yet wrt to gardening and stability to let our EFL EWS cq- patches due to test failures. Sorry for the trouble in this case - you can always ask someone in #webkit-efl to pre-run this patch, we should be able to help with that.
Noam Rosenthal
Comment 24 2012-12-19 09:25:03 PST
(In reply to comment #23) > (In reply to comment #22) > > Hmm, all worked on my machine with no asserts; I will try on a different computer and try to reproduce. Either way thanks for rolling out. > > Btw would be great if this kind of thing wad caught by ews or cq... > > It would be good, but we're not confident enough yet wrt to gardening and stability to let our EFL EWS cq- patches due to test failures. Sorry for the trouble in this case - you can always ask someone in #webkit-efl to pre-run this patch, we should be able to help with that. can we have something where the ews bot posts information potential test failures without cq-? This way the info would be there and the contributor can decide whether to submit or not...
Dominik Röttsches (drott)
Comment 25 2012-12-20 06:24:36 PST
(In reply to comment #24) > can we have something where the ews bot posts information potential test failures without cq-? This way the info would be there and the contributor can decide whether to submit or not... Thanks for the suggestion - I'll add it to my list to see if that's possible.
Csaba Osztrogonác
Comment 26 2012-12-20 07:13:01 PST
I filed a new bug report for the EWS discussion: https://bugs.webkit.org/show_bug.cgi?id=105534
Noam Rosenthal
Comment 27 2012-12-26 17:33:51 PST
Created attachment 180769 [details] Patch for landing
Noam Rosenthal
Comment 28 2012-12-26 17:35:38 PST
OK, I was able to reproduce the issue, it has to do with different GL versions... New patch should "work just fine"(tm) but if something fails when I'm unavailable and it's not trivial feel free to re-rollout.
WebKit Review Bot
Comment 29 2012-12-26 17:38:13 PST
Attachment 180769 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/texmap/TextureMapperShaderManager.cpp:112: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 30 2012-12-26 18:30:34 PST
Comment on attachment 180769 [details] Patch for landing Clearing flags on attachment: 180769 Committed r138489: <http://trac.webkit.org/changeset/138489>
WebKit Review Bot
Comment 31 2012-12-26 18:30:40 PST
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 32 2012-12-28 00:31:33 PST
(In reply to comment #30) > (From update of attachment 180769 [details]) > Clearing flags on attachment: 180769 > > Committed r138489: <http://trac.webkit.org/changeset/138489> It caused a regression on Qt - https://bugs.webkit.org/show_bug.cgi?id=105820
Note You need to log in before you can comment on or make changes to this bug.