Title: [220436] trunk
Revision
220436
Author
drou...@apple.com
Date
2017-08-08 18:55:53 -0700 (Tue, 08 Aug 2017)

Log Message

Web Inspector: Canvas: support editing WebGL shaders
https://bugs.webkit.org/show_bug.cgi?id=124211
<rdar://problem/15448958>

Reviewed by Matt Baker.

Source/_javascript_Core:

* inspector/protocol/Canvas.json:
Add `updateShader` command that will change the given shader's source to the provided string,
recompile, and relink it to its associated program.
Drive-by: add description to `requestShaderSource` command.

Source/WebCore:

Test: inspector/canvas/updateShader.html

* inspector/InspectorCanvasAgent.h:
* inspector/InspectorCanvasAgent.cpp:
(WebCore::InspectorCanvasAgent::updateShader):

* html/canvas/WebGLRenderingContextBase.h:
* html/canvas/WebGLRenderingContextBase.cpp:
(WebCore::WebGLRenderingContextBase::linkProgram):
(WebCore::WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations):
Normally, when a program is linked, it invalidates any WebGLUniformLocation associated with
the program by incrementing its `linkCount`. In order to allow live editing of shaders, we
need to be able to compile and link a shader without invalidating these locations. This
patch moves the shader linking logic to its own function that is called by `linkProgram` so
that InspectorCanvasAgent can compile and link without invalidation.

Source/WebInspectorUI:

* UserInterface/Models/ShaderProgram.js:
(WI.ShaderProgram.prototype.updateVertexShader):
(WI.ShaderProgram.prototype.updateFragmentShader):
(WI.ShaderProgram.prototype._updateShader):

* UserInterface/Views/ShaderProgramContentView.js:
(WI.ShaderProgramContentView):
(WI.ShaderProgramContentView.prototype._contentDidChange):

LayoutTests:

* inspector/canvas/updateShader-expected.txt: Added.
* inspector/canvas/updateShader.html: Added.

* platform/win/TestExpectations:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (220435 => 220436)


--- trunk/LayoutTests/ChangeLog	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/LayoutTests/ChangeLog	2017-08-09 01:55:53 UTC (rev 220436)
@@ -1,3 +1,16 @@
+2017-08-08  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Canvas: support editing WebGL shaders
+        https://bugs.webkit.org/show_bug.cgi?id=124211
+        <rdar://problem/15448958>
+
+        Reviewed by Matt Baker.
+
+        * inspector/canvas/updateShader-expected.txt: Added.
+        * inspector/canvas/updateShader.html: Added.
+
+        * platform/win/TestExpectations:
+
 2017-08-08  Ryan Haddad  <ryanhad...@apple.com>
 
         Mark media/modern-media-controls/css/webkit-cursor-visibility-auto-hide.html as flaky.

Added: trunk/LayoutTests/inspector/canvas/updateShader-expected.txt (0 => 220436)


--- trunk/LayoutTests/inspector/canvas/updateShader-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/inspector/canvas/updateShader-expected.txt	2017-08-09 01:55:53 UTC (rev 220436)
@@ -0,0 +1,38 @@
+CONSOLE MESSAGE: WebGL: ERROR: 0:1: 'INVALID' : syntax error
+CONSOLE MESSAGE: WebGL: ERROR: 0:1: 'INVALID' : syntax error
+Test compilation of shaders after being attached to a program, with and without syntax errors.
+
+
+== Running test suite: Canvas.updateShader
+-- Running test case: Canvas.updateShader.vertexShaderValid
+
+    void main(void) {
+        gl_Position = vec4(1, 2, 3, 4);
+    }
+
+
+-- Running test case: Canvas.updateShader.fragmentShaderValid
+
+    precision mediump float;
+
+    void main(void) {
+        gl_FragColor = vec4(0.1, 0.2, 0.3, 0.4);
+    }
+
+
+-- Running test case: Canvas.updateShader.invalidProgramId
+PASS: Should produce an error.
+Error: No shader program for given identifier.
+
+-- Running test case: Canvas.updateShader.invalidShaderType
+PASS: Should produce an error.
+Error: No shader for given type.
+
+-- Running test case: Canvas.updateShader.invalidVertexShaderSource
+PASS: Should produce error.
+Error: Shader compilation failed.
+
+-- Running test case: Canvas.updateShader.invalidFragmentShaderSource
+PASS: Should produce error.
+Error: Shader compilation failed.
+

Added: trunk/LayoutTests/inspector/canvas/updateShader.html (0 => 220436)


--- trunk/LayoutTests/inspector/canvas/updateShader.html	                        (rev 0)
+++ trunk/LayoutTests/inspector/canvas/updateShader.html	2017-08-09 01:55:53 UTC (rev 220436)
@@ -0,0 +1,144 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<script src=""
+<script id="vertex-shader" type="x-shader/x-vertex">
+    void main(void) {
+        gl_Position = vec4(0.0, 0.0, 0.0, 1.0);
+    }
+</script>
+<script id="fragment-shader" type="x-shader/x-fragment">
+    precision mediump float;
+
+    void main(void) {
+        gl_FragColor = vec4(1.0, 1.0, 1.0, 1.0);
+    }
+</script>
+<script>
+function load() {
+    createProgram("webgl");
+    linkProgram("vertex-shader", "fragment-shader");
+
+    runTest();
+}
+
+function test() {
+    let suite = InspectorTest.createAsyncSuite("Canvas.updateShader");
+
+    function validSourceTest({name, shaderType, source}) {
+        suite.addTestCase({
+            name,
+            test(resolve, reject) {
+                let shaderProgram = WI.canvasManager.shaderPrograms[0];
+                if (!shaderProgram) {
+                    reject("Missing shader program")
+                    return;
+                }
+
+                let programId = shaderProgram.identifier;
+
+                CanvasAgent.updateShader(programId, shaderType, source)
+                .then(() => CanvasAgent.requestShaderSource(programId, shaderType))
+                .then(({content}) => InspectorTest.log(content))
+                .then(resolve, reject);
+            }
+        });
+    }
+
+    validSourceTest({
+        name: "Canvas.updateShader.vertexShaderValid",
+        shaderType: CanvasAgent.ShaderType.Vertex,
+        source: `
+    void main(void) {
+        gl_Position = vec4(1, 2, 3, 4);
+    }
+`,
+    });
+
+    validSourceTest({
+        name: "Canvas.updateShader.fragmentShaderValid",
+        shaderType: CanvasAgent.ShaderType.Fragment,
+        source: `
+    precision mediump float;
+
+    void main(void) {
+        gl_FragColor = vec4(0.1, 0.2, 0.3, 0.4);
+    }
+`,
+    });
+
+    suite.addTestCase({
+        name: "Canvas.updateShader.invalidProgramId",
+        description: "Invalid program identifiers should cause an error.",
+        test(resolve, reject) {
+            const programId = "INVALID_PROGRAM_ID";
+            const shaderType = "INVALID_SHADER_TYPE";
+            const source = "INVALID_SOURCE";
+            CanvasAgent.updateShader(programId, shaderType, source, (error) => {
+                InspectorTest.expectThat(error, "Should produce an error.");
+                InspectorTest.log("Error: " + error);
+                resolve();
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "Canvas.updateShader.invalidShaderType",
+        description: "Invalid shader types should cause an error.",
+        test(resolve, reject) {
+            let shaderProgram = WI.canvasManager.shaderPrograms[0];
+            if (!shaderProgram) {
+                reject("Missing shader program")
+                return;
+            }
+
+            const shaderType = "INVALID_SHADER_TYPE";
+            const source = "INVALID_SOURCE";
+            CanvasAgent.updateShader(shaderProgram.identifier, shaderType, source, (error) => {
+                InspectorTest.expectThat(error, "Should produce an error.");
+                InspectorTest.log("Error: " + error);
+                resolve();
+            });
+        }
+    });
+
+    function invalidSourceTest({name, shaderType, source}) {
+        suite.addTestCase({
+            name,
+            test(resolve, reject) {
+                let shaderProgram = WI.canvasManager.shaderPrograms[0];
+                if (!shaderProgram) {
+                    reject("Missing shader program")
+                    return;
+                }
+
+                CanvasAgent.updateShader(shaderProgram.identifier, shaderType, source, (error) => {
+                    InspectorTest.expectThat(error, "Should produce error.");
+                    InspectorTest.log("Error: " + error);
+                    resolve();
+                });
+            }
+        });
+    }
+
+    invalidSourceTest({
+        name: "Canvas.updateShader.invalidVertexShaderSource",
+        shaderType: CanvasAgent.ShaderType.Vertex,
+        source: "INVALID",
+    });
+
+    invalidSourceTest({
+        name: "Canvas.updateShader.invalidFragmentShaderSource",
+        shaderType: CanvasAgent.ShaderType.Fragment,
+        source: "INVALID",
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body _onload_="load()">
+    <p>Test compilation of shaders after being attached to a program, with and without syntax errors.</p>
+</body>
+</html>

Modified: trunk/LayoutTests/platform/win/TestExpectations (220435 => 220436)


--- trunk/LayoutTests/platform/win/TestExpectations	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/LayoutTests/platform/win/TestExpectations	2017-08-09 01:55:53 UTC (rev 220436)
@@ -1916,6 +1916,7 @@
 inspector/canvas/resolveCanvasContext-webgl2.html [ Skip ]
 inspector/canvas/shaderProgram-add-remove-webgl.html [ Skip ]
 inspector/canvas/shaderProgram-add-remove-webgl2.html [ Skip ]
+inspector/canvas/updateShader.html [ Skip ]
 ################################################################################
 #################          End WebGL Issues              #######################
 ################################################################################

Modified: trunk/Source/_javascript_Core/ChangeLog (220435 => 220436)


--- trunk/Source/_javascript_Core/ChangeLog	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-08-09 01:55:53 UTC (rev 220436)
@@ -1,3 +1,16 @@
+2017-08-08  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Canvas: support editing WebGL shaders
+        https://bugs.webkit.org/show_bug.cgi?id=124211
+        <rdar://problem/15448958>
+
+        Reviewed by Matt Baker.
+
+        * inspector/protocol/Canvas.json:
+        Add `updateShader` command that will change the given shader's source to the provided string,
+        recompile, and relink it to its associated program.
+        Drive-by: add description to `requestShaderSource` command.
+
 2017-08-08  Robin Morisset  <rmoris...@apple.com>
 
         Make JSC_validateExceptionChecks=1 succeed on JSTests/slowMicrobenchmarks/spread-small-array.js.

Modified: trunk/Source/_javascript_Core/inspector/protocol/Canvas.json (220435 => 220436)


--- trunk/Source/_javascript_Core/inspector/protocol/Canvas.json	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/_javascript_Core/inspector/protocol/Canvas.json	2017-08-09 01:55:53 UTC (rev 220436)
@@ -122,7 +122,7 @@
         },
         {
             "name": "requestShaderSource",
-            "description": "",
+            "description": "Requests the source of the shader of the given type from the program with the given id.",
             "parameters": [
                 { "name": "programId", "$ref": "ProgramId" },
                 { "name": "shaderType", "$ref": "ShaderType" }
@@ -130,6 +130,15 @@
             "returns": [
                 { "name": "content", "type": "string" }
             ]
+        },
+        {
+            "name": "updateShader",
+            "description": "Compiles and links the shader with identifier and type with the given source code.",
+            "parameters": [
+                { "name": "programId", "$ref": "ProgramId" },
+                { "name": "shaderType", "$ref": "ShaderType" },
+                { "name": "source", "type": "string" }
+            ]
         }
     ],
     "events": [

Modified: trunk/Source/WebCore/ChangeLog (220435 => 220436)


--- trunk/Source/WebCore/ChangeLog	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebCore/ChangeLog	2017-08-09 01:55:53 UTC (rev 220436)
@@ -1,3 +1,27 @@
+2017-08-08  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Canvas: support editing WebGL shaders
+        https://bugs.webkit.org/show_bug.cgi?id=124211
+        <rdar://problem/15448958>
+
+        Reviewed by Matt Baker.
+
+        Test: inspector/canvas/updateShader.html
+
+        * inspector/InspectorCanvasAgent.h:
+        * inspector/InspectorCanvasAgent.cpp:
+        (WebCore::InspectorCanvasAgent::updateShader):
+
+        * html/canvas/WebGLRenderingContextBase.h:
+        * html/canvas/WebGLRenderingContextBase.cpp:
+        (WebCore::WebGLRenderingContextBase::linkProgram):
+        (WebCore::WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations):
+        Normally, when a program is linked, it invalidates any WebGLUniformLocation associated with
+        the program by incrementing its `linkCount`. In order to allow live editing of shaders, we
+        need to be able to compile and link a shader without invalidating these locations. This
+        patch moves the shader linking logic to its own function that is called by `linkProgram` so
+        that InspectorCanvasAgent can compile and link without invalidation.
+
 2017-08-08  Sam Weinig  <s...@webkit.org>
 
         [WebIDL] Add support for Promise<> attributes

Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp (220435 => 220436)


--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp	2017-08-09 01:55:53 UTC (rev 220436)
@@ -2836,17 +2836,26 @@
 
 void WebGLRenderingContextBase::linkProgram(WebGLProgram* program)
 {
+    if (!linkProgramWithoutInvalidatingAttribLocations(program))
+        return;
+
+    program->increaseLinkCount();
+}
+
+bool WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations(WebGLProgram* program)
+{
     if (isContextLostOrPending() || !validateWebGLObject("linkProgram", program))
-        return;
+        return false;
+
     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)) || !m_context->checkVaryingsPacking(objectOrZero(vertexShader), objectOrZero(fragmentShader))) {
         program->setLinkStatus(false);
-        return;
+        return false;
     }
 
     m_context->linkProgram(objectOrZero(program));
-    program->increaseLinkCount();
+    return true;
 }
 
 void WebGLRenderingContextBase::pixelStorei(GC3Denum pname, GC3Dint param)

Modified: trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h (220435 => 220436)


--- trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebCore/html/canvas/WebGLRenderingContextBase.h	2017-08-09 01:55:53 UTC (rev 220436)
@@ -228,6 +228,7 @@
 
     void lineWidth(GC3Dfloat);
     void linkProgram(WebGLProgram*);
+    bool linkProgramWithoutInvalidatingAttribLocations(WebGLProgram*);
     void pixelStorei(GC3Denum pname, GC3Dint param);
     void polygonOffset(GC3Dfloat factor, GC3Dfloat units);
     void readPixels(GC3Dint x, GC3Dint y, GC3Dsizei width, GC3Dsizei height, GC3Denum format, GC3Denum type, ArrayBufferView& pixels);

Modified: trunk/Source/WebCore/inspector/InspectorCanvasAgent.cpp (220435 => 220436)


--- trunk/Source/WebCore/inspector/InspectorCanvasAgent.cpp	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebCore/inspector/InspectorCanvasAgent.cpp	2017-08-09 01:55:53 UTC (rev 220436)
@@ -293,6 +293,37 @@
 #endif
 }
 
+void InspectorCanvasAgent::updateShader(ErrorString& errorString, const String& programId, const String& shaderType, const String& source)
+{
+#if ENABLE(WEBGL)
+    auto* inspectorProgram = assertInspectorProgram(errorString, programId);
+    if (!inspectorProgram)
+        return;
+
+    auto* shader = inspectorProgram->shaderForType(shaderType);
+    if (!shader) {
+        errorString = ASCIILiteral("No shader for given type.");
+        return;
+    }
+
+    WebGLRenderingContextBase* contextWebGL = inspectorProgram->context();
+    contextWebGL->shaderSource(shader, source);
+    contextWebGL->compileShader(shader);
+
+    if (!shader->isValid()) {
+        errorString = ASCIILiteral("Shader compilation failed.");
+        return;
+    }
+
+    contextWebGL->linkProgramWithoutInvalidatingAttribLocations(&inspectorProgram->program());
+#else
+    UNUSED_PARAM(programId);
+    UNUSED_PARAM(shaderType);
+    UNUSED_PARAM(source);
+    errorString = ASCIILiteral("WebGL is not supported.");
+#endif
+}
+
 void InspectorCanvasAgent::frameNavigated(Frame& frame)
 {
     if (frame.isMainFrame()) {

Modified: trunk/Source/WebCore/inspector/InspectorCanvasAgent.h (220435 => 220436)


--- trunk/Source/WebCore/inspector/InspectorCanvasAgent.h	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebCore/inspector/InspectorCanvasAgent.h	2017-08-09 01:55:53 UTC (rev 220436)
@@ -76,6 +76,7 @@
     void requestRecording(ErrorString&, const String& canvasId, const bool* const singleFrame, const int* const memoryLimit) override;
     void cancelRecording(ErrorString&, const String& canvasId) override;
     void requestShaderSource(ErrorString&, const String& programId, const String& shaderType, String*) override;
+    void updateShader(ErrorString&, const String& programId, const String& shaderType, const String& source) override;
 
     // InspectorInstrumentation
     void frameNavigated(Frame&);

Modified: trunk/Source/WebInspectorUI/ChangeLog (220435 => 220436)


--- trunk/Source/WebInspectorUI/ChangeLog	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebInspectorUI/ChangeLog	2017-08-09 01:55:53 UTC (rev 220436)
@@ -1,3 +1,20 @@
+2017-08-08  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Canvas: support editing WebGL shaders
+        https://bugs.webkit.org/show_bug.cgi?id=124211
+        <rdar://problem/15448958>
+
+        Reviewed by Matt Baker.
+
+        * UserInterface/Models/ShaderProgram.js:
+        (WI.ShaderProgram.prototype.updateVertexShader):
+        (WI.ShaderProgram.prototype.updateFragmentShader):
+        (WI.ShaderProgram.prototype._updateShader):
+
+        * UserInterface/Views/ShaderProgramContentView.js:
+        (WI.ShaderProgramContentView):
+        (WI.ShaderProgramContentView.prototype._contentDidChange):
+
 2017-08-07  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: Preview Canvas path when viewing a recording

Modified: trunk/Source/WebInspectorUI/UserInterface/Models/ShaderProgram.js (220435 => 220436)


--- trunk/Source/WebInspectorUI/UserInterface/Models/ShaderProgram.js	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebInspectorUI/UserInterface/Models/ShaderProgram.js	2017-08-09 01:55:53 UTC (rev 220436)
@@ -57,6 +57,16 @@
         this._requestShaderSource(CanvasAgent.ShaderType.Fragment, callback);
     }
 
+    updateVertexShader(source)
+    {
+        this._updateShader(CanvasAgent.ShaderType.Vertex, source);
+    }
+
+    updateFragmentShader(source)
+    {
+        this._updateShader(CanvasAgent.ShaderType.Fragment, source);
+    }
+
     // Private
 
     _requestShaderSource(shaderType, callback)
@@ -70,6 +80,13 @@
             callback(content);
         });
     }
+
+    _updateShader(shaderType, source)
+    {
+        CanvasAgent.updateShader(this._identifier, shaderType, source, (error) => {
+            console.assert(!error, error);
+        });
+    }
 };
 
 WI.ShaderProgram.ShaderType = {

Modified: trunk/Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.js (220435 => 220436)


--- trunk/Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.js	2017-08-09 01:39:37 UTC (rev 220435)
+++ trunk/Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.js	2017-08-09 01:55:53 UTC (rev 220436)
@@ -35,8 +35,10 @@
 
         let createEditor = (shaderType) => {
             let textEditor = new WI.TextEditor;
+            textEditor.readOnly = false;
             textEditor.addEventListener(WI.TextEditor.Event.Focused, this._editorFocused, this);
             textEditor.addEventListener(WI.TextEditor.Event.NumberOfSearchResultsDidChange, this._numberOfSearchResultsDidChange, this);
+            textEditor.addEventListener(WI.TextEditor.Event.ContentDidChange, this.debounce(250)._contentDidChange, this);
             textEditor.element.classList.add("shader");
 
             let shaderTypeContainer = textEditor.element.insertAdjacentElement("afterbegin", document.createElement("div"));
@@ -194,4 +196,12 @@
     {
         this.dispatchEventToListeners(WI.ContentView.Event.NumberOfSearchResultsDidChange);
     }
+
+    _contentDidChange(event)
+    {
+        if (event.target === this._vertexEditor)
+            this.representedObject.updateVertexShader(this._vertexEditor.string);
+        else if (event.target === this._fragmentEditor)
+            this.representedObject.updateFragmentShader(this._fragmentEditor.string);
+    }
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to