Changeset 173527 in webkit

Timestamp:

Sep 11, 2014, 12:22:06 PM (12 years ago)

Author:

roger_fong@apple.com

Message:

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.

Remove varying packing restrictions checks from ANGLE.

• src/compiler/translator/Compiler.cpp:

(TCompiler::compile):
(TCompiler::enforcePackingRestrictions):

• src/compiler/translator/ShHandle.h:

• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/ThirdParty/ANGLE/ChangeLog (modified) (1 diff)
• Source/ThirdParty/ANGLE/src/compiler/translator/Compiler.cpp (modified) (2 diffs)
• Source/ThirdParty/ANGLE/src/compiler/translator/ShHandle.h (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/canvas/WebGLRenderingContext.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/GraphicsContext3D.h (modified) (1 diff)
• Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -551 +551 @@
 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 ]

trunk/Source/ThirdParty/ANGLE/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/ThirdParty/ANGLE/src/compiler/translator/Compiler.cpp

@@ -240 +240 @@
         {
             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))
@@ -454 +461 @@
 {
     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);
 }
 

trunk/Source/ThirdParty/ANGLE/src/compiler/translator/ShHandle.h

@@ -96 +96 @@
     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

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/html/canvas/WebGLRenderingContext.cpp

@@ -3454 +3454 @@
         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;

trunk/Source/WebCore/platform/graphics/GraphicsContext3D.h

@@ -810 +810 @@
     void useProgram(Platform3DObject);
     void validateProgram(Platform3DObject);
+    bool checkVaryingsPacking(Platform3DObject vertexShader, Platform3DObject fragmentShader) const;
     bool precisionsMatch(Platform3DObject vertexShader, Platform3DObject fragmentShader) const;
 

trunk/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp

@@ -41 +41 @@
 #include "Extensions3DOpenGL.h"
 #endif
+#include "ANGLEWebKitBridge.h"
 #include "GraphicsContext.h"
 #include "ImageBuffer.h"
@@ -335 +336 @@
 }
 
+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