Bug 104815 – [Texmap] Instead of having multiple shaders sources with lots of duplication, we should have one shader source with MACRO variants

• Component: New Bugs
• Status: RESOLVED FIXED

[Noam Rosenthal]

[Texmap] Instead of having multiple shaders sources with lots of duplication, we should have one shader source with MACRO variants

[Noam Rosenthal]

Created attachment 179092 [details]
Patch

[Noam Rosenthal]

Created attachment 179100 [details]
Patch

[Noam Rosenthal]

Created attachment 180074 [details]
Patch

[WebKit Review Bot]

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 on attachment 180074 [details]
Patch

Attachment 180074 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15413394

[Early Warning System Bot]

Comment on attachment 180074 [details]
Patch

Attachment 180074 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15400451

[Noam Rosenthal]

Created attachment 180085 [details]
Patch

[WebKit Review Bot]

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]

Created attachment 180086 [details]
Patch

[WebKit Review Bot]

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 on attachment 180086 [details]
Patch

LGTM

[EFL EWS Bot]

Comment on attachment 180086 [details]
Patch

Attachment 180086 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/15404601

[Noam Rosenthal]

Created attachment 180150 [details]
Patch

[Noam Rosenthal]

Submitted style-checker false positive bug, https://bugs.webkit.org/show_bug.cgi?id=105427

[WebKit Review Bot]

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 on attachment 180150 [details]
Patch

Clearing flags on attachment: 180150

Committed r138157: <http://trac.webkit.org/changeset/138157>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Dominik Röttsches (drott)]

(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)]

(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]

Re-opened since this is blocked by bug 105439

[Csaba Osztrogonác]

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]

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)]

(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]

(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)]

(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]

I filed a new bug report for the EWS discussion: https://bugs.webkit.org/show_bug.cgi?id=105534

[Noam Rosenthal]

Created attachment 180769 [details]
Patch for landing

[Noam Rosenthal]

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]

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 on attachment 180769 [details]
Patch for landing

Clearing flags on attachment: 180769

Committed r138489: <http://trac.webkit.org/changeset/138489>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(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