| Summary: | Must resolve multisampled back buffer during copyTexImage2D and copyTexSubImage2D | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kenneth Russell <kbr> | ||||||
| Component: | WebGL | Assignee: | Zhenyao Mo <zmo> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cmarrin, commit-queue, dglazkov, fishd, oliver | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Kenneth Russell
2010-04-06 15:39:47 PDT
Created attachment 52690 [details]
patch
Comment on attachment 52690 [details] patch The code looks good. One comment on the test case. > Index: LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d.html > =================================================================== > --- LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d.html (revision 0) > +++ LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d.html (revision 0) > @@ -0,0 +1,185 @@ > +<html> > +<head> > +<link rel="stylesheet" href="../../js/resources/js-test-style.css"/> > +<script src="../../js/resources/js-test-pre.js"></script> > +<script src="resources/webgl-test.js"></script> > +<script src="resources/utils3d.js"> </script> > +<script id="vshader" type="x-shader/x-vertex"> > +attribute vec3 g_Position; > +attribute vec2 g_TexCoord0; > + > +varying vec2 texCoord; > + > +void main() > +{ > + gl_Position = vec4(g_Position.x, g_Position.y, g_Position.z, 1.0); > + texCoord = g_TexCoord0; > +} > +</script> > + > +<script id="fshader" type="x-shader/x-fragment"> > +uniform sampler2D tex; > +varying vec2 texCoord; > + > +void main() > +{ > + gl_FragColor = texture2D(tex, texCoord); > +} > +</script> > + > +<script> > +var successfullyParsed = false; > + > +function init() > +{ > + if (window.layoutTestController) { > + layoutTestController.overridePreference("WebKitWebGLEnabled", "1"); > + layoutTestController.dumpAsText(); > + layoutTestController.waitUntilDone(); > + } > + > + description('Verify copyTexImage2D and copyTexSubImage2D'); > + > + runTest(); > +} > + > +// These two declarations need to be global for "shouldBe" to see them > +var pixel = [0, 0, 0]; > +var correctColor = null; > + > +function runTestIteration(antialias) > +{ > + var gl = null; > + if (antialias) > + gl = initWebGL("antialiasOn", "vshader", "fshader", [ "g_Position", "g_TexCoord0" ], [ 0, 0, 0, 1 ], 1); > + else > + gl = initWebGL("antialiasOff", "vshader", "fshader", [ "g_Position", "g_TexCoord0" ], [ 0, 0, 0, 1 ], 1, { antialias: false }); > + > + var textureLoc = gl.getUniformLocation(gl.program, "tex"); > + > + var vertices = new WebGLFloatArray([ > + 1.0, 1.0, 0.0, > + -1.0, 1.0, 0.0, > + -1.0, -1.0, 0.0, > + 1.0, 1.0, 0.0, > + -1.0, -1.0, 0.0, > + 1.0, -1.0, 0.0]); > + var texCoords = new WebGLFloatArray([ > + 1.0, 1.0, > + 0.0, 1.0, > + 0.0, 0.0, > + 1.0, 1.0, > + 0.0, 0.0, > + 1.0, 0.0]); > + var texCoordOffset = vertices.byteLength; > + > + var vbo = gl.createBuffer(); > + gl.bindBuffer(gl.ARRAY_BUFFER, vbo); > + gl.bufferData(gl.ARRAY_BUFFER, > + texCoordOffset + texCoords.byteLength, > + gl.STATIC_DRAW); > + gl.bufferSubData(gl.ARRAY_BUFFER, 0, vertices); > + gl.bufferSubData(gl.ARRAY_BUFFER, texCoordOffset, texCoords); > + > + gl.enableVertexAttribArray(0); > + gl.vertexAttribPointer(0, 3, gl.FLOAT, false, 0, 0); > + gl.enableVertexAttribArray(1); > + gl.vertexAttribPointer(1, 2, gl.FLOAT, gl.FALSE, 0, texCoordOffset); > + > + gl.colorMask(1, 1, 1, 0); > + gl.disable(gl.BLEND); > + debug('Testing copyTexImage2D'); > + > + // Red canvas > + gl.clearColor(1, 0, 0, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); > + > + var texture = gl.createTexture(); > + // Bind the texture to texture unit 0 > + gl.bindTexture(gl.TEXTURE_2D, texture); > + // Set up texture > + gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 2, 2, 0, gl.RGBA, gl.UNSIGNED_BYTE, null); > + gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST); > + gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST); I think there should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. > + gl.copyTexImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 0, 0, 2, 2, 0); There should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. Previously this was generating INVALID_OPERATION. > + > + // Green canvas > + gl.clearColor(0, 1, 0, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); > + > + // Point the uniform sampler to texture unit 0 > + gl.uniform1i(textureLoc, 0); > + // Draw the triangles > + gl.drawArrays(gl.TRIANGLES, 0, 6); > + > + // Read back the rendering results, should be red > + var buf = gl.readPixels(0, 0, 2, 2, gl.RGBA, gl.UNSIGNED_BYTE); > + var idx = 0; > + correctColor = [255, 0, 0]; > + for (var y = 0; y < 2; y++) { > + for (var x = 0; x < 2; x++) { > + idx = (y * 2 + x) * 4; > + pixel[0] = buf[idx]; > + pixel[1] = buf[idx + 1]; > + pixel[2] = buf[idx + 2]; > + shouldBe("pixel", "correctColor"); > + } > + } > + > + debug('Testing copyTexSubImage2D'); > + > + // Green canvas > + gl.clearColor(0, 1, 0, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); I think there should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. > + > + gl.copyTexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, 0, 0, 2, 2); There should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. Previously this was generating INVALID_OPERATION. > + > + // Blue canvas > + gl.clearColor(0, 0, 1, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); > + > + // Draw the triangles > + gl.drawArrays(gl.TRIANGLES, 0, 6); > + > + // Read back the rendering results, should be green > + buf = gl.readPixels(0, 0, 2, 2, gl.RGBA, gl.UNSIGNED_BYTE); > + correctColor = [0, 255, 0]; > + for (var y = 0; y < 2; y++) { > + for (var x = 0; x < 2; x++) { > + idx = (y * 2 + x) * 4; > + pixel[0] = buf[idx]; > + pixel[1] = buf[idx + 1]; > + pixel[2] = buf[idx + 2]; > + shouldBe("pixel", "correctColor"); > + } > + } > +} > + > +function runTest(antialias) > +{ > + debug("Testing with antialias on"); > + runTestIteration(true); > + debug("Testing with antialias off"); > + runTestIteration(false); > + > + successfullyParsed = true; > + var epilogue = document.createElement("script"); > + epilogue.onload = finish; > + epilogue.src = "../../js/resources/js-test-post.js"; > + document.body.appendChild(epilogue); > +} > + > +function finish() { > + if (window.layoutTestController) { > + layoutTestController.notifyDone(); > + } > +} > +</script> > +</head> > +<body onload="init()"> > +<canvas id="antialiasOn" width="2px" height="2px"></canvas> > +<canvas id="antialiasOff" width="2px" height="2px"></canvas> > +<div id="description"></div> > +<div id="console"></div> > +</body> > +</html> > Index: LayoutTests/fast/canvas/webgl/resources/utils3d.js > =================================================================== > --- LayoutTests/fast/canvas/webgl/resources/utils3d.js (revision 57160) > +++ LayoutTests/fast/canvas/webgl/resources/utils3d.js (working copy) > @@ -17,10 +17,10 @@ > // by the caller. By default, it maps to the window.console() function on WebKit and to > // an empty function on other browsers. > // > -function initWebGL(canvasName, vshader, fshader, attribs, clearColor, clearDepth) > +function initWebGL(canvasName, vshader, fshader, attribs, clearColor, clearDepth, contextAttribs) > { > var canvas = document.getElementById(canvasName); > - var gl = canvas.getContext("experimental-webgl"); > + var gl = canvas.getContext("experimental-webgl", contextAttribs); > if (!gl) { > alert("No WebGL context found"); > return null; Created attachment 52782 [details]
revised patch: responding to Ken Russell's review
Comment on attachment 52782 [details]
revised patch: responding to Ken Russell's review
Looks good to me.
Comment on attachment 52782 [details]
revised patch: responding to Ken Russell's review
kbr's word is good as gold.
Comment on attachment 52782 [details] revised patch: responding to Ken Russell's review Clearing flags on attachment: 52782 Committed r57322: <http://trac.webkit.org/changeset/57322> All reviewed patches have been landed. Closing bug. |