Check for varying packing restrictions per program instead of per shader.
authorroger_fong@apple.com <roger_fong@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Sep 2014 19:22:06 +0000 (19:22 +0000)
committerroger_fong@apple.com <roger_fong@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Sep 2014 19:22:06 +0000 (19:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136585.
<rdar://problem/16308409>.

Reviewed by Dean Jackson.

Covered by webgl/1.0.2/conformance/ogles/GL/build/build_009_to_016.html.

* html/canvas/WebGLRenderingContext.cpp:
(WebCore::WebGLRenderingContext::linkProgram):
Check for varying packing restrictions when linking the program.

* platform/graphics/GraphicsContext3D.h:
* platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:
(WebCore::GraphicsContext3D::checkVaryingsPacking):
Checks varyings shared by both vertex and fragment shaders and makes sure
they satisfy packing restrictions.

Remove varying packing restrictions checks from ANGLE.
* src/compiler/translator/Compiler.cpp:
(TCompiler::compile):
(TCompiler::enforcePackingRestrictions):
* src/compiler/translator/ShHandle.h:

* platform/mac/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@173527 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac/TestExpectations
Source/ThirdParty/ANGLE/ChangeLog
Source/ThirdParty/ANGLE/src/compiler/translator/Compiler.cpp
Source/ThirdParty/ANGLE/src/compiler/translator/ShHandle.h
Source/WebCore/ChangeLog
Source/WebCore/html/canvas/WebGLRenderingContext.cpp
Source/WebCore/platform/graphics/GraphicsContext3D.h
Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp

index 2f8b404adadb41b58405dab522d57f350199b15f..e32de26e9109c05690a741efb71a8e35df2c8248 100644 (file)
@@ -1,3 +1,13 @@
+2014-09-10  Roger Fong  <roger_fong@apple.com>
+
+        Check for varying packing restrictions per program instead of per shader.
+        https://bugs.webkit.org/show_bug.cgi?id=136585.
+        <rdar://problem/16308409>.
+
+        Reviewed by Dean Jackson.
+
+        * platform/mac/TestExpectations:
+
 2014-09-11  Beth Dakin  <bdakin@apple.com>
 
         Support rubber-banding in sub-frames
index 59bbf4e51b37823f48cd9a666c1f4427189dd160..2420ff3171c0c567929d7e0af682b72adb35b362 100755 (executable)
@@ -550,7 +550,6 @@ webgl/1.0.1/conformance/glsl/samplers/glsl-function-texture2dlod.html [ Failure
 webgl/1.0.1/conformance/glsl/samplers/glsl-function-texture2dproj.html [ Failure ]
 webgl/1.0.1/conformance/textures/origin-clean-conformance.html [ Skip ]
 webgl/1.0.1/conformance/context/context-lost-restored.html [ Failure ]
-webgl/1.0.2/conformance/ogles/GL/build/build_009_to_016.html [ Failure ]
 webgl/1.0.2/conformance/context/context-creation-and-destruction.html [ Failure Timeout ]
 webgl/1.0.2/conformance/rendering/multisample-corruption.html [ Failure Timeout ]
 http/tests/webgl/1.0.2/origin-clean-conformance.html [ Skip ]
index cecd116073c1114af182a5b93e628103274710ce..fcd029eb69de483b22f07970c53768673c97cf7d 100644 (file)
@@ -1,3 +1,17 @@
+2014-09-10  Roger Fong  <roger_fong@apple.com>
+
+        Check for varying packing restrictions per program instead of per shader.
+        https://bugs.webkit.org/show_bug.cgi?id=136585.
+        <rdar://problem/16308409>.
+
+        Reviewed by Dean Jackson.
+
+        Remove varying packing restrictions checks from ANGLE.
+        * src/compiler/translator/Compiler.cpp:
+        (TCompiler::compile):
+        (TCompiler::enforcePackingRestrictions):
+        * src/compiler/translator/ShHandle.h:
+
 2014-09-06  Darin Adler  <darin@apple.com>
 
         Make updates suggested by new version of Xcode
index c3111c12e12fd12c22bdb4d2eff61f3e3384a86d..3a5b3f2936eb85db17f321baf54d335f4f2acad9 100644 (file)
@@ -239,8 +239,15 @@ bool TCompiler::compile(const char* const shaderStrings[],
         if (success && (compileOptions & SH_VARIABLES))
         {
             collectVariables(root);
-            if (compileOptions & SH_ENFORCE_PACKING_RESTRICTIONS)
+            if (compileOptions & SH_ENFORCE_PACKING_RESTRICTIONS) {
                 success = enforcePackingRestrictions();
+                if (!success) {
+                    infoSink.info.prefix(EPrefixError);
+                    infoSink.info << "too many uniforms";
+                    return false;
+                }
+            }
+
             if (success && shaderType == SH_VERTEX_SHADER &&
                 (compileOptions & SH_INIT_VARYINGS_WITHOUT_STATIC_USE))
                 initializeVaryingsWithoutStaticUse(root);
@@ -453,21 +460,7 @@ void TCompiler::collectVariables(TIntermNode* root)
 bool TCompiler::enforcePackingRestrictions()
 {
     VariablePacker packer;
-    bool success = packer.CheckVariablesWithinPackingLimits(maxUniformVectors, uniforms);
-    if (!success) {
-        infoSink.info.prefix(EPrefixError);
-        infoSink.info << "too many uniforms";
-        return false;
-    }
-
-    success = packer.CheckVariablesWithinPackingLimits(maxVaryingVectors, varyings);
-
-    if (!success) {
-        infoSink.info.prefix(EPrefixError);
-        infoSink.info << "too many varyings";
-        return false;
-    }
-    return true;
+    return packer.CheckVariablesWithinPackingLimits(maxUniformVectors, uniforms);
 }
 
 void TCompiler::initializeGLPosition(TIntermNode* root)
index 687ff634ab50c1204f3290a02b137ba3d577e846..c44696ab71af32498c3a1a67bdd6a2f9c8230cf5 100644 (file)
@@ -95,7 +95,7 @@ protected:
     // Translate to object code.
     virtual void translate(TIntermNode* root) = 0;
     // Returns true if, after applying the packing rules in the GLSL 1.017 spec
-    // Appendix A, section 7, the shader does not use too many uniforms or varyings.
+    // Appendix A, section 7, the shader does not use too many uniforms
     bool enforcePackingRestrictions();
     // Insert statements to initialize varyings without static use in the beginning
     // of main(). It is to work around a Mac driver where such varyings in a vertex
index 07f48116b47cab50932b1115bfa6ca1353ab4672..d0b10f92a8484945d274c394bf9e4e0d77e5528b 100644 (file)
@@ -1,3 +1,23 @@
+2014-09-10  Roger Fong  <roger_fong@apple.com>
+
+        Check for varying packing restrictions per program instead of per shader.
+        https://bugs.webkit.org/show_bug.cgi?id=136585.
+        <rdar://problem/16308409>.
+
+        Reviewed by Dean Jackson.
+
+        Covered by webgl/1.0.2/conformance/ogles/GL/build/build_009_to_016.html.
+
+        * html/canvas/WebGLRenderingContext.cpp:
+        (WebCore::WebGLRenderingContext::linkProgram): 
+        Check for varying packing restrictions when linking the program.
+
+        * platform/graphics/GraphicsContext3D.h:
+        * platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:
+        (WebCore::GraphicsContext3D::checkVaryingsPacking): 
+        Checks varyings shared by both vertex and fragment shaders and makes sure
+        they satisfy packing restrictions.
+
 2014-09-11  Beth Dakin  <bdakin@apple.com>
 
         Support rubber-banding in sub-frames
index e5bdca3e9f90a4562abb39c893dd180a4eb05d0c..71818d19d63935b0556d3b182662eb209fae42e0 100644 (file)
@@ -3453,7 +3453,7 @@ void WebGLRenderingContext::linkProgram(WebGLProgram* program, ExceptionCode& ec
     if (!isGLES2Compliant()) {
         WebGLShader* vertexShader = program->getAttachedShader(GraphicsContext3D::VERTEX_SHADER);
         WebGLShader* fragmentShader = program->getAttachedShader(GraphicsContext3D::FRAGMENT_SHADER);
-        if (!vertexShader || !vertexShader->isValid() || !fragmentShader || !fragmentShader->isValid() || !m_context->precisionsMatch(objectOrZero(vertexShader), objectOrZero(fragmentShader))) {
+        if (!vertexShader || !vertexShader->isValid() || !fragmentShader || !fragmentShader->isValid() || !m_context->precisionsMatch(objectOrZero(vertexShader), objectOrZero(fragmentShader)) || !m_context->checkVaryingsPacking(objectOrZero(vertexShader), objectOrZero(fragmentShader))) {
             program->setLinkStatus(false);
             return;
         }
index 591faa2153ce8e1df20248cd029a1f743b768e87..fa800eff55309c353c2aefdec3131e7cf0ffd43d 100644 (file)
@@ -809,6 +809,7 @@ public:
 
     void useProgram(Platform3DObject);
     void validateProgram(Platform3DObject);
+    bool checkVaryingsPacking(Platform3DObject vertexShader, Platform3DObject fragmentShader) const;
     bool precisionsMatch(Platform3DObject vertexShader, Platform3DObject fragmentShader) const;
 
     void vertexAttrib1f(GC3Duint index, GC3Dfloat x);
index 608706e218419934ad16088a0a605de97e69869d..51432644fd3ff2fe487d719c7b8346802a3ac2ee 100644 (file)
@@ -40,6 +40,7 @@
 #else
 #include "Extensions3DOpenGL.h"
 #endif
+#include "ANGLEWebKitBridge.h"
 #include "GraphicsContext.h"
 #include "ImageBuffer.h"
 #include "ImageData.h"
@@ -334,6 +335,53 @@ void GraphicsContext3D::reshape(int width, int height)
     ::glFlush();
 }
 
+bool GraphicsContext3D::checkVaryingsPacking(Platform3DObject vertexShader, Platform3DObject fragmentShader) const
+{
+    ASSERT(m_shaderSourceMap.contains(vertexShader));
+    ASSERT(m_shaderSourceMap.contains(fragmentShader));
+    const auto& vertexEntry = m_shaderSourceMap.find(vertexShader)->value;
+    const auto& fragmentEntry = m_shaderSourceMap.find(fragmentShader)->value;
+
+    HashMap<String, ShVariableInfo> combinedVaryings;
+    for (const auto& vertexSymbol : vertexEntry.varyingMap) {
+        const String& symbolName = vertexSymbol.key;
+        // The varying map includes variables for each index of an array variable.
+        // We only want a single variable to represent the array.
+        if (symbolName.endsWith("]"))
+            continue;
+
+        // Don't count built in varyings.
+        if (symbolName == "gl_FragCoord" || symbolName == "gl_FrontFacing" || symbolName == "gl_PointCoord")
+            continue;
+
+        const auto& fragmentSymbol = fragmentEntry.varyingMap.find(symbolName);
+        if (fragmentSymbol != fragmentEntry.varyingMap.end()) {
+            ShVariableInfo symbolInfo;
+            symbolInfo.type = static_cast<ShDataType>((fragmentSymbol->value).type);
+            // The arrays are already split up.
+            symbolInfo.size = (fragmentSymbol->value).size;
+            combinedVaryings.add(symbolName, symbolInfo);
+        }
+    }
+
+    size_t numVaryings = combinedVaryings.size();
+    if (!numVaryings)
+        return true;
+
+    ShVariableInfo* variables = new ShVariableInfo[numVaryings];
+    int index = 0;
+    for (const auto& varyingSymbol : combinedVaryings) {
+        variables[index] = varyingSymbol.value;
+        index++;
+    }
+    
+    GC3Dint maxVaryingFloats = 0;
+    ::glGetIntegerv(GL_MAX_VARYING_FLOATS, &maxVaryingFloats);
+    int result = ShCheckVariablesWithinPackingLimits(maxVaryingFloats / 4, variables, numVaryings);
+
+    delete[] variables;
+    return result;
+}
 
 bool GraphicsContext3D::precisionsMatch(Platform3DObject vertexShader, Platform3DObject fragmentShader) const
 {