Web Inspector: Canvas: support editing WebGL shaders
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Aug 2017 01:55:53 +0000 (01:55 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Aug 2017 01:55:53 +0000 (01:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=124211
<rdar://problem/15448958>

Reviewed by Matt Baker.

Source/JavaScriptCore:

* 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:

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/inspector/canvas/updateShader-expected.txt [new file with mode: 0644]
LayoutTests/inspector/canvas/updateShader.html [new file with mode: 0644]
LayoutTests/platform/win/TestExpectations
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/inspector/protocol/Canvas.json
Source/WebCore/ChangeLog
Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp
Source/WebCore/html/canvas/WebGLRenderingContextBase.h
Source/WebCore/inspector/InspectorCanvasAgent.cpp
Source/WebCore/inspector/InspectorCanvasAgent.h
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Models/ShaderProgram.js
Source/WebInspectorUI/UserInterface/Views/ShaderProgramContentView.js

index 937be1029f1fc65d371010769d0806e97b94c857..d068e21899d0c5ef393b23cefdcf3e76f46b023b 100644 (file)
@@ -1,3 +1,16 @@
+2017-08-08  Devin Rousso  <drousso@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  <ryanhaddad@apple.com>
 
         Mark media/modern-media-controls/css/webkit-cursor-visibility-auto-hide.html as flaky.
diff --git a/LayoutTests/inspector/canvas/updateShader-expected.txt b/LayoutTests/inspector/canvas/updateShader-expected.txt
new file mode 100644 (file)
index 0000000..deb171f
--- /dev/null
@@ -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.
+
diff --git a/LayoutTests/inspector/canvas/updateShader.html b/LayoutTests/inspector/canvas/updateShader.html
new file mode 100644 (file)
index 0000000..e49ad2b
--- /dev/null
@@ -0,0 +1,144 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script src="resources/shaderProgram-utilities.js"></script>
+<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>
index 8e65fe911f15998efef5d1b2cbe15118268d8576..81e25aceb873cf94f260a06101e223ebd6d04cdf 100644 (file)
@@ -1916,6 +1916,7 @@ inspector/canvas/resolveCanvasContext-webgl.html [ Skip ]
 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              #######################
 ################################################################################
index 4d61639120047adeff15caa1f87b894ea8427205..8fd51fc4f9794b52a038bf5d8234ca8979f6921b 100644 (file)
@@ -1,3 +1,16 @@
+2017-08-08  Devin Rousso  <drousso@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  <rmorisset@apple.com>
 
         Make JSC_validateExceptionChecks=1 succeed on JSTests/slowMicrobenchmarks/spread-small-array.js.
index bccf3fc78e2b2d8eb4dd3ad97e945feebd8d18f8..b01b833180a83b151ded7c193f8d508d14d1eb37 100644 (file)
         },
         {
             "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" }
             "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": [
index 5b444070347ea006bb25659b771de938054b589d..b8f5d0dbb943712d381bda2e6637fdcc04735dda 100644 (file)
@@ -1,3 +1,27 @@
+2017-08-08  Devin Rousso  <drousso@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  <sam@webkit.org>
 
         [WebIDL] Add support for Promise<> attributes
index 0fc71e27e0dac4c96a7b47649aebc7319bab51aa..16964280145093205343bccd3d7e0a4ad9503786 100644 (file)
@@ -2836,17 +2836,26 @@ void WebGLRenderingContextBase::lineWidth(GC3Dfloat width)
 
 void WebGLRenderingContextBase::linkProgram(WebGLProgram* program)
 {
-    if (isContextLostOrPending() || !validateWebGLObject("linkProgram", program))
+    if (!linkProgramWithoutInvalidatingAttribLocations(program))
         return;
+
+    program->increaseLinkCount();
+}
+
+bool WebGLRenderingContextBase::linkProgramWithoutInvalidatingAttribLocations(WebGLProgram* program)
+{
+    if (isContextLostOrPending() || !validateWebGLObject("linkProgram", program))
+        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)
index 70f76ce9db4cbab6abfeabe77c8e9e157eb3f9f8..2b134c22bbf5b541616e6dda6088cb8580cdd0e4 100644 (file)
@@ -228,6 +228,7 @@ public:
 
     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);
index 26ef7824faa211ac4d23ea94ab297097ead13f0e..2b0ee71fa80ab1c2ce4588b6820b905cde96adfa 100644 (file)
@@ -293,6 +293,37 @@ void InspectorCanvasAgent::requestShaderSource(ErrorString& errorString, const S
 #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()) {
index f1ccf7b49075d88ee368d764c707247bb886a1dc..aa10042cb779d7c9d2289963308dabc5cbe6a8c1 100644 (file)
@@ -76,6 +76,7 @@ public:
     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&);
index 2149c5330f1d96bae7cdde1f00c8165dae0ab48e..b161ffe5e6df628a8feb55d28bdc53c6b1ed87ed 100644 (file)
@@ -1,3 +1,20 @@
+2017-08-08  Devin Rousso  <drousso@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  <drousso@apple.com>
 
         Web Inspector: Preview Canvas path when viewing a recording
index 7ee7a8f8ed65d8c16de278ff9c9609b51834e8e1..fdca5883065ab444ecc21cdc755076ce5160468d 100644 (file)
@@ -57,6 +57,16 @@ WI.ShaderProgram = class ShaderProgram extends WI.Object
         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 @@ WI.ShaderProgram = class ShaderProgram extends WI.Object
             callback(content);
         });
     }
+
+    _updateShader(shaderType, source)
+    {
+        CanvasAgent.updateShader(this._identifier, shaderType, source, (error) => {
+            console.assert(!error, error);
+        });
+    }
 };
 
 WI.ShaderProgram.ShaderType = {
index 4c80ebdab98aea546587f92ed370e0861bd7ad42..167969095defb7ee569249f7e7ba5d7450f2f83f 100644 (file)
@@ -35,8 +35,10 @@ WI.ShaderProgramContentView = class ShaderProgramContentView extends WI.ContentV
 
         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 @@ WI.ShaderProgramContentView = class ShaderProgramContentView extends WI.ContentV
     {
         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);
+    }
 };