Diff
Modified: branches/safari-604-branch/LayoutTests/ChangeLog (223179 => 223180)
--- branches/safari-604-branch/LayoutTests/ChangeLog 2017-10-11 15:53:21 UTC (rev 223179)
+++ branches/safari-604-branch/LayoutTests/ChangeLog 2017-10-11 16:09:19 UTC (rev 223180)
@@ -1,5 +1,23 @@
2017-10-10 Jason Marcell <[email protected]>
+ Cherry-pick r221831. rdar://problem/34891283
+
+ 2017-09-08 Dean Jackson <[email protected]>
+
+ gl.detachShader breaks shader program
+ https://bugs.webkit.org/show_bug.cgi?id=137689
+ <rdar://problem/34025056>
+
+ Reviewed by Sam Weinig.
+
+ Test that detaches and deletes shaders after linking, but before
+ asking for uniform locations.
+
+ * fast/canvas/webgl/detachShader-before-accessing-uniform-expected.txt: Added.
+ * fast/canvas/webgl/detachShader-before-accessing-uniform.html: Added.
+
+2017-10-10 Jason Marcell <[email protected]>
+
Cherry-pick r222450. rdar://problem/34920294
2017-09-25 Ryan Haddad <[email protected]>
Added: branches/safari-604-branch/LayoutTests/fast/canvas/webgl/detachShader-before-accessing-uniform-expected.txt (0 => 223180)
--- branches/safari-604-branch/LayoutTests/fast/canvas/webgl/detachShader-before-accessing-uniform-expected.txt (rev 0)
+++ branches/safari-604-branch/LayoutTests/fast/canvas/webgl/detachShader-before-accessing-uniform-expected.txt 2017-10-11 16:09:19 UTC (rev 223180)
@@ -0,0 +1,7 @@
+Vertex Shader compiled.
+Fragment Shader compiled.
+Program linked. Detaching and deleting shaders.
+Color uniform location was identified.
+Position attribute location was identified.
+Drawn.
+
Added: branches/safari-604-branch/LayoutTests/fast/canvas/webgl/detachShader-before-accessing-uniform.html (0 => 223180)
--- branches/safari-604-branch/LayoutTests/fast/canvas/webgl/detachShader-before-accessing-uniform.html (rev 0)
+++ branches/safari-604-branch/LayoutTests/fast/canvas/webgl/detachShader-before-accessing-uniform.html 2017-10-11 16:09:19 UTC (rev 223180)
@@ -0,0 +1,122 @@
+<style>
+canvas {
+ width: 200px;
+ height: 200px;
+}
+</style>
+</head>
+<script id="vertexShaderSource" type="text/glsl">
+attribute vec4 position;
+void main() {
+ gl_Position = position;
+}
+</script>
+<script id="fragmentShaderSource" type="text/glsl">
+precision mediump float;
+uniform vec4 color;
+void main() {
+ gl_FragColor = color;
+}
+</script>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+function output(msg) {
+ let d = document.querySelector("div");
+ d.innerHTML += `${msg}<br>`;
+}
+
+function drawTriangle() {
+
+ let canvas = document.querySelector("canvas");
+ canvas.width = 200;
+ canvas.height = 200;
+ let gl = canvas.getContext("webgl");
+
+ let vertexShader = gl.createShader(gl.VERTEX_SHADER);
+ gl.shaderSource(vertexShader, document.getElementById("vertexShaderSource").textContent);
+ gl.compileShader(vertexShader);
+ if (!gl.getShaderParameter(vertexShader, gl.COMPILE_STATUS)) {
+ output("Vertex Shader failed to compile.");
+ console.log(gl.getShaderInfoLog(vertexShader));
+ return;
+ }
+ output("Vertex Shader compiled.");
+
+ let fragmentShader = gl.createShader(gl.FRAGMENT_SHADER);
+ gl.shaderSource(fragmentShader, document.getElementById("fragmentShaderSource").textContent);
+ gl.compileShader(fragmentShader);
+ if (!gl.getShaderParameter(fragmentShader, gl.COMPILE_STATUS)) {
+ output("Fragment Shader failed to compile.");
+ console.log(gl.getShaderInfoLog(fragmentShader));
+ return;
+ }
+ output("Fragment Shader compiled.");
+
+ let program = gl.createProgram();
+ gl.attachShader(program, vertexShader);
+ gl.attachShader(program, fragmentShader);
+ gl.linkProgram(program);
+
+ if (!gl.getProgramParameter(program, gl.LINK_STATUS)) {
+ output("Unable to link shaders into program.");
+ return;
+ }
+ output("Program linked. Detaching and deleting shaders.");
+
+ gl.detachShader(program, vertexShader);
+ gl.detachShader(program, fragmentShader);
+ gl.deleteShader(vertexShader);
+ gl.deleteShader(fragmentShader);
+
+ gl.useProgram(program);
+
+ let colorUniform = gl.getUniformLocation(program, "color");
+ if (colorUniform)
+ output("Color uniform location was identified.");
+ else {
+ output("FAIL: Color uniform location was not found.");
+ return;
+ }
+
+ let positionAttribute = gl.getAttribLocation(program, "position");
+ if (positionAttribute >= 0)
+ output("Position attribute location was identified.");
+ else {
+ output("FAIL: Position attribute location was not found.");
+ return;
+ }
+
+ gl.enableVertexAttribArray(positionAttribute);
+
+ let vertices = new Float32Array([
+ -0.8, -0.3,
+ 0.7, -0.8,
+ 0.55, 0.75
+ ]);
+
+ let triangleBuffer = gl.createBuffer();
+
+ gl.bindBuffer(gl.ARRAY_BUFFER, triangleBuffer);
+ gl.bufferData(gl.ARRAY_BUFFER, vertices, gl.STATIC_DRAW);
+
+ gl.clearColor(0, 0, 0, 1);
+ gl.clear(gl.COLOR_BUFFER_BIT);
+
+ let now = Date.now();
+ gl.uniform4fv(colorUniform, [1.0, 0.0, 0.0, 1.0]);
+
+ gl.bindBuffer(gl.ARRAY_BUFFER, triangleBuffer);
+ gl.vertexAttribPointer(positionAttribute, 2, gl.FLOAT, false, 0, 0);
+
+ gl.drawArrays(gl.TRIANGLES, 0, 3);
+ output("Drawn.");
+}
+
+window.addEventListener("load", drawTriangle, false);
+</script>
+<body>
+ <canvas></canvas>
+ <div></div>
+</body>
\ No newline at end of file
Modified: branches/safari-604-branch/Source/WebCore/ChangeLog (223179 => 223180)
--- branches/safari-604-branch/Source/WebCore/ChangeLog 2017-10-11 15:53:21 UTC (rev 223179)
+++ branches/safari-604-branch/Source/WebCore/ChangeLog 2017-10-11 16:09:19 UTC (rev 223180)
@@ -1,5 +1,47 @@
2017-10-10 Jason Marcell <[email protected]>
+ Cherry-pick r221831. rdar://problem/34891283
+
+ 2017-09-08 Dean Jackson <[email protected]>
+
+ gl.detachShader breaks shader program
+ https://bugs.webkit.org/show_bug.cgi?id=137689
+ <rdar://problem/34025056>
+
+ Reviewed by Sam Weinig.
+
+ It should be possible to compile shaders, attach them to a program,
+ link the program, detach the shaders, delete the shaders, and then
+ ask for the uniform and attribute locations. That is, once you've
+ linked, the shaders can be thrown away.
+
+ We were using the attached shaders to look up uniform locations, so
+ we now keep around a separate map that remembers what shaders were
+ attached when the program links.
+
+ This fixes the bug, but the whole area is still a bit messy. For one,
+ we're keeping around all the shader information even after it is
+ no longer used.
+ See https://bugs.webkit.org/show_bug.cgi?id=98204
+
+ Test: fast/canvas/webgl/detachShader-before-accessing-uniform.html
+
+ * platform/graphics/GraphicsContext3D.h: Add another map to remember
+ what shaders were used when a program was linked.
+ * platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:
+ (WebCore::GraphicsContext3D::mappedSymbolInShaderSourceMap): New helper
+ to look up a name in our source maps.
+ (WebCore::GraphicsContext3D::mappedSymbolName): Use the helper, and look
+ at linked shaders if there are no attached shaders.
+ (WebCore::GraphicsContext3D::originalSymbolInShaderSourceMap): Does the
+ reverse of the above.
+ (WebCore::GraphicsContext3D::originalSymbolName):
+ (WebCore::GraphicsContext3D::linkProgram): Add to the new map.
+ (WebCore::GraphicsContext3D::deleteProgram): Delete the program from
+ our shader entries.
+
+2017-10-10 Jason Marcell <[email protected]>
+
Cherry-pick r222478. rdar://problem/34771020
2017-09-25 Youenn Fablet <[email protected]>
Modified: branches/safari-604-branch/Source/WebCore/platform/graphics/GraphicsContext3D.h (223179 => 223180)
--- branches/safari-604-branch/Source/WebCore/platform/graphics/GraphicsContext3D.h 2017-10-11 15:53:21 UTC (rev 223179)
+++ branches/safari-604-branch/Source/WebCore/platform/graphics/GraphicsContext3D.h 2017-10-11 16:09:19 UTC (rev 223180)
@@ -1340,9 +1340,14 @@
}
};
+ // FIXME: Shaders are never removed from this map, even if they and their program are deleted.
+ // This is bad, and it also relies on the fact we never reuse Platform3DObject numbers.
typedef HashMap<Platform3DObject, ShaderSourceEntry> ShaderSourceMap;
ShaderSourceMap m_shaderSourceMap;
+ typedef HashMap<Platform3DObject, std::pair<Platform3DObject, Platform3DObject>> LinkedShaderMap;
+ LinkedShaderMap m_linkedShaderMap;
+
struct ActiveShaderSymbolCounts {
Vector<GC3Dint> filteredToActualAttributeIndexMap;
Vector<GC3Dint> filteredToActualUniformIndexMap;
@@ -1369,6 +1374,8 @@
String mappedSymbolName(Platform3DObject program, ANGLEShaderSymbolType, const String& name);
String mappedSymbolName(Platform3DObject shaders[2], size_t count, const String& name);
String originalSymbolName(Platform3DObject program, ANGLEShaderSymbolType, const String& name);
+ std::optional<String> mappedSymbolInShaderSourceMap(Platform3DObject shader, ANGLEShaderSymbolType, const String& name);
+ std::optional<String> originalSymbolInShaderSourceMap(Platform3DObject shader, ANGLEShaderSymbolType, const String& name);
std::unique_ptr<ShaderNameHash> nameHashMapForShaders;
Modified: branches/safari-604-branch/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp (223179 => 223180)
--- branches/safari-604-branch/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp 2017-10-11 15:53:21 UTC (rev 223179)
+++ branches/safari-604-branch/Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp 2017-10-11 16:09:19 UTC (rev 223180)
@@ -922,6 +922,21 @@
return builder.toString();
}
+std::optional<String> GraphicsContext3D::mappedSymbolInShaderSourceMap(Platform3DObject shader, ANGLEShaderSymbolType symbolType, const String& name)
+{
+ auto result = m_shaderSourceMap.find(shader);
+ if (result == m_shaderSourceMap.end())
+ return std::nullopt;
+
+ const auto& symbolMap = result->value.symbolMap(symbolType);
+ auto symbolEntry = symbolMap.find(name);
+ if (symbolEntry == symbolMap.end())
+ return std::nullopt;
+
+ auto& mappedName = symbolEntry->value.mappedName;
+ return String(mappedName.c_str(), mappedName.length());
+}
+
String GraphicsContext3D::mappedSymbolName(Platform3DObject program, ANGLEShaderSymbolType symbolType, const String& name)
{
GC3Dsizei count = 0;
@@ -929,16 +944,21 @@
getAttachedShaders(program, 2, &count, shaders);
for (GC3Dsizei i = 0; i < count; ++i) {
- ShaderSourceMap::iterator result = m_shaderSourceMap.find(shaders[i]);
- if (result == m_shaderSourceMap.end())
- continue;
+ auto mappedName = mappedSymbolInShaderSourceMap(shaders[i], symbolType, name);
+ if (mappedName)
+ return mappedName.value();
+ }
- const ShaderSymbolMap& symbolMap = result->value.symbolMap(symbolType);
- ShaderSymbolMap::const_iterator symbolEntry = symbolMap.find(name);
- if (symbolEntry != symbolMap.end()) {
- const std::string& mappedName = symbolEntry->value.mappedName;
- return String(mappedName.c_str(), mappedName.length());
- }
+ // We might have detached or deleted the shaders after linking.
+ auto result = m_linkedShaderMap.find(program);
+ if (result != m_linkedShaderMap.end()) {
+ auto linkedShaders = result->value;
+ auto mappedName = mappedSymbolInShaderSourceMap(linkedShaders.first, symbolType, name);
+ if (mappedName)
+ return mappedName.value();
+ mappedName = mappedSymbolInShaderSourceMap(linkedShaders.second, symbolType, name);
+ if (mappedName)
+ return mappedName.value();
}
if (symbolType == SHADER_SYMBOL_TYPE_ATTRIBUTE && !name.isEmpty()) {
@@ -948,7 +968,7 @@
nameHashMapForShaders = std::make_unique<ShaderNameHash>();
setCurrentNameHashMapForShader(nameHashMapForShaders.get());
- String generatedName = generateHashedName(name);
+ auto generatedName = generateHashedName(name);
setCurrentNameHashMapForShader(nullptr);
@@ -959,7 +979,21 @@
return name;
}
-
+
+std::optional<String> GraphicsContext3D::originalSymbolInShaderSourceMap(Platform3DObject shader, ANGLEShaderSymbolType symbolType, const String& name)
+{
+ auto result = m_shaderSourceMap.find(shader);
+ if (result == m_shaderSourceMap.end())
+ return std::nullopt;
+
+ const auto& symbolMap = result->value.symbolMap(symbolType);
+ for (const auto& symbolEntry : symbolMap) {
+ if (name == symbolEntry.value.mappedName.c_str())
+ return symbolEntry.key;
+ }
+ return std::nullopt;
+}
+
String GraphicsContext3D::originalSymbolName(Platform3DObject program, ANGLEShaderSymbolType symbolType, const String& name)
{
GC3Dsizei count;
@@ -967,17 +1001,23 @@
getAttachedShaders(program, 2, &count, shaders);
for (GC3Dsizei i = 0; i < count; ++i) {
- ShaderSourceMap::iterator result = m_shaderSourceMap.find(shaders[i]);
- if (result == m_shaderSourceMap.end())
- continue;
-
- const ShaderSymbolMap& symbolMap = result->value.symbolMap(symbolType);
- for (const auto& symbolEntry : symbolMap) {
- if (name == symbolEntry.value.mappedName.c_str())
- return symbolEntry.key;
- }
+ auto originalName = originalSymbolInShaderSourceMap(shaders[i], symbolType, name);
+ if (originalName)
+ return originalName.value();
}
+ // We might have detached or deleted the shaders after linking.
+ auto result = m_linkedShaderMap.find(program);
+ if (result != m_linkedShaderMap.end()) {
+ auto linkedShaders = result->value;
+ auto originalName = originalSymbolInShaderSourceMap(linkedShaders.first, symbolType, name);
+ if (originalName)
+ return originalName.value();
+ originalName = originalSymbolInShaderSourceMap(linkedShaders.second, symbolType, name);
+ if (originalName)
+ return originalName.value();
+ }
+
if (symbolType == SHADER_SYMBOL_TYPE_ATTRIBUTE && !name.isEmpty()) {
// Attributes are a special case: they may be requested before any shaders have been compiled,
// and aren't even required to be used in any shader program.
@@ -1140,6 +1180,14 @@
{
ASSERT(program);
makeContextCurrent();
+
+ GC3Dsizei count = 0;
+ Platform3DObject shaders[2] = { };
+ getAttachedShaders(program, 2, &count, shaders);
+
+ if (count == 2)
+ m_linkedShaderMap.set(program, std::make_pair(shaders[0], shaders[1]));
+
::glLinkProgram(program);
}
@@ -1845,6 +1893,7 @@
void GraphicsContext3D::deleteProgram(Platform3DObject program)
{
makeContextCurrent();
+ m_shaderProgramSymbolCountMap.remove(program);
glDeleteProgram(program);
}
